Skip to contentSkip to conversation

Potentially Incorrect Wraparound Check in mm/usercopy.c

PATCH [email protected] 1 participant 1 message Nov 9, 2018

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

Hi Kees,

We are seeing the following message and kernel BUG on the 4.14.76 kernel:

[   16.094139] usercopy: kernel memory overwrite attempt detected to fffffffffffff000 (<wrapped address>) (4096 bytes)
[   16.140498] kernel BUG at /local/mnt/workspace/isaacm/hana_workspace/kdev/kernel/mm/usercopy.c:72!

This occurs when a thread tries to write 4 KB to one page, and the virtual address for that page--which was acquired via a call to kmap_atomic()--is 0xfffffffffffff000. Before doing the write, we call check_copy_size(0xfffffffffffff000, SZ_4K, false). It seems like we are seeing this issue because of the first check in check_bogus_address(), which checks to see if reading the 4 KB will cause wraparound. With the following change, we no longer see this issue:

mm/usercopy.c +2 -1
index 852eb4e..0293645 100644
@@ -151,7 +151,7 @@ static inline void check_bogus_address(const unsigned long ptr, unsigned long n,
                                       bool to_user)
 {
        /* Reject if object wraps past end of memory. */
-       if (ptr + n < ptr)
+       if (ptr + (n - 1) < ptr)
                usercopy_abort("wrapped address", NULL, to_user, 0, ptr 
+ n);
 
        /* Reject if NULL or ZERO-allocation. */

Is there a reason why this change to that check would not be valid? If we are checking to see if reading n bytes, starting at ptr, will cause a wraparound, then shouldn't we be checking to ensure that the range of memory that will actually be read from won't cause a wraparound, since we would only be accessing [ptr, ptr + (n - 1)], and not ptr + n?

Thanks,
Isaac Manjarres