Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] abi: UAPI struct layout mismatch for 32-bit compat userspace breaks IOCTLs --
commit 8d88f08e2268b0340428c21059f2534d77368ca5 Author: Abhishek Bapat <[email protected]> alloc_tag: Introduce IOCTLs to toggle allocation tracepoints Introduce a new IOCTL (ALLOCINFO_IOC_TOGGLE_TRACE) to selectively toggle tracing on allocation call sites. Userspace tools can use the existing filtering mechanism to specify the set of tags to toggle tracing for. > diff --git a/include/uapi/linux/alloc_tag.h b/include/uapi/linux/alloc_tag.h > index 7d4618bea0434..dafcba2f7e2c1 100644 > --- a/include/uapi/linux/alloc_tag.h > +++ b/include/uapi/linux/alloc_tag.h > @@ -85,9 +85,20 @@ struct allocinfo_get_at { > struct allocinfo_tag_data data; > }; > > +struct allocinfo_toggle_traces { > + /* inputs */ > + /* > + * Tags matching the filter are toggled; an empty mask selects all tags. > + * Size criteria are checked once, against the tag's size at call time. > + */ > + struct allocinfo_filter filter; [Severity: Medium] The new struct allocinfo_toggle_traces embeds struct allocinfo_filter. Does this struct layout introduce a UAPI mismatch regression for 32-bit compat userspace? The struct allocinfo_filter contains struct allocinfo_tag (196 bytes) directly followed by __u64 min_size: include/uapi/linux/alloc_tag.h: struct allocinfo_filter { __u64 mask; struct allocinfo_tag fields; __u64 min_size; ... }; On 32-bit architectures like i386, __u64 has a 4-byte alignment requirement, placing min_size at offset 204. On 64-bit architectures, __u64 has an 8-byte alignment requirement, inserting 4 bytes of padding and placing min_size at offset 208. Because of this size disparity (236 bytes vs 240 bytes for struct allocinfo_toggle_traces), the IOCTL command number generated by _IOW() in userspace will differ between 32-bit and 64-bit builds. Since the pre-existing allocinfo_compat_ioctl() directly passes the 32-bit IOCTL command number to the native allocinfo_ioctl() switch statement without translation: mm/alloc_tag.c:allocinfo_compat_ioctl() { return allocinfo_ioctl(file, cmd, (unsigned long)compat_ptr(arg)); } Won't the new ALLOCINFO_IOC_TOGGLE_TRACE command fall through to default in allocinfo_ioctl() and return -ENOIOCTLCMD, breaking the feature for 32-bit userspace tools on 64-bit kernels? > + __u64 enable; > +}; > + -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=2
