Re: [PATCH v2 2/4] alloc_tag: Introduce IOCTLs to toggle allocation tracepoints

[sashiko-bot@kernel.org]

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 <abhishekbapat@google.com>

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/cover.1791410885.git.abhishekbapat@google.com?part=2