On Wed, Feb 05, 2025 at 10:47:58AM +0100, Jens Remus wrote: > UNSAFE_GET_USER_INC(ra_off, cur, offset_size, Efault); > > With offset_size=1 expands into: > > __UNSAFE_GET_USER_INC(/*to=*/ra_off, /*from=*cur, /*type=*/u8, > /*label=*/Efault); > > Expands into: > > { > u8 __to; > unsafe_get_user(__to, (u8 __user *)cur, Efault); > cur += sizeof(__to); > ra_off = (typeof(ra_off))__to; > } > > The issue is that on the last line __to is u8 instead of s8 and thus > u8 gets casted to s32, which is performed without sign extension. __to > would need to be s8 or get casted to s8 for sign extension to take > place.
Ah, I get it now, thanks for spelling that out for me. Here's what I have at the moment: #define __UNSAFE_GET_USER_INC(to, from, type, label) \ ({ \ type __to; \ unsafe_get_user(__to, (type __user *)from, label); \ from += sizeof(__to); \ to = __to; \ }) #define __UNSAFE_GET_USER_INC(to, from, size, label, u_or_s) \ ({ \ switch (size) { \ case 1: \ __UNSAFE_GET_USER_INC(to, from, u_or_s##8, label); \ break; \ case 2: \ __UNSAFE_GET_USER_INC(to, from, u_or_s##16, label); \ break; \ case 4: \ __UNSAFE_GET_USER_INC(to, from, u_or_s##32, label); \ break; \ default: \ dbg_sec_uaccess("%d: bad unsafe_get_user() size %u\n", \ __LINE__, size); \ return -EFAULT; \ } \ }) #define UNSAFE_GET_USER_UNSIGNED_INC(to, from, size, label) \ __UNSAFE_GET_USER_INC(to, from, size, label, u) #define UNSAFE_GET_USER_SIGNED_INC(to, from, size, label) \ __UNSAFE_GET_USER_INC(to, from, size, label, s) #define UNSAFE_GET_USER_INC(to, from, size, label) \ _Generic(to, \ u8: UNSAFE_GET_USER_UNSIGNED_INC(to, from, size, label), \ u16: UNSAFE_GET_USER_UNSIGNED_INC(to, from, size, label), \ u32: UNSAFE_GET_USER_UNSIGNED_INC(to, from, size, label), \ s8: UNSAFE_GET_USER_SIGNED_INC(to, from, size, label), \ s16: UNSAFE_GET_USER_SIGNED_INC(to, from, size, label), \ s32: UNSAFE_GET_USER_SIGNED_INC(to, from, size, label))