Thank you for your contribution! Sashiko AI review found 1 potential issue(s) 
to consider:
- [High] alloc_tag: UAPI breakage in ALLOCINFO_IOC_GET_AT from growing 
allocinfo_filter
--

commit b1243e6bc8216f5fe9d8786ec8ca3a597c41681f
Author: Abhishek Bapat <[email protected]>

alloc_tag: Extend allocinfo_filter to support tracing queries

This commit extends struct allocinfo_counter and struct allocinfo_filter in
the alloc_tag uapi header to report whether tracing is enabled on a tag and
allow filtering tags by their trace state.

> diff --git a/include/uapi/linux/alloc_tag.h b/include/uapi/linux/alloc_tag.h
> index dafcba2f7e2c1..a96054ac22376 100644
> --- a/include/uapi/linux/alloc_tag.h
> +++ b/include/uapi/linux/alloc_tag.h
> @@ -38,7 +38,8 @@ struct allocinfo_counter {
>       __u64 bytes;
>       __u64 calls;
>       __u8 accurate;
> -     __u8 pad[7];
> +     __u8 trace_on;
> +     __u8 pad[6];
>  } __attribute__((aligned(8)));
>  
>  struct allocinfo_tag_data {
> @@ -54,7 +55,8 @@ enum {
>       ALLOCINFO_FILTER_INACCURATE,
>       ALLOCINFO_FILTER_MIN_SIZE,
>       ALLOCINFO_FILTER_MAX_SIZE,
> -     __ALLOCINFO_FILTER_LAST = ALLOCINFO_FILTER_MAX_SIZE
> +     ALLOCINFO_FILTER_TRACE_ON,
> +     __ALLOCINFO_FILTER_LAST = ALLOCINFO_FILTER_TRACE_ON
>  };
>  
>  #define ALLOCINFO_FILTER_MASK_MODNAME                (1 << 
> ALLOCINFO_FILTER_MODNAME)
> @@ -64,6 +66,7 @@ enum {
>  #define ALLOCINFO_FILTER_MASK_INACCURATE     (1 << 
> ALLOCINFO_FILTER_INACCURATE)
>  #define ALLOCINFO_FILTER_MASK_MIN_SIZE               (1 << 
> ALLOCINFO_FILTER_MIN_SIZE)
>  #define ALLOCINFO_FILTER_MASK_MAX_SIZE               (1 << 
> ALLOCINFO_FILTER_MAX_SIZE)
> +#define ALLOCINFO_FILTER_MASK_TRACE_ON               (1 << 
> ALLOCINFO_FILTER_TRACE_ON)
>  
>  #define ALLOCINFO_FILTER_MASKS \
>       ((1 << (__ALLOCINFO_FILTER_LAST + 1)) - 1)
> @@ -75,6 +78,7 @@ struct allocinfo_filter {
>       __u64 max_size;
>       /* filter criteria only; see allocinfo_counter.accurate for actual 
> accuracy */
>       __u64 inaccurate;
> +     __u64 tracing;
>  };

[Severity: High]
Does appending the tracing field to struct allocinfo_filter in
include/uapi/linux/alloc_tag.h break existing userspace binaries calling
ALLOCINFO_IOC_GET_AT?

While struct allocinfo_counter consumes a byte from pad[] to keep its size
unchanged, growing struct allocinfo_filter by 8 bytes shifts the data field
in struct allocinfo_get_at and changes sizeof(struct allocinfo_get_at):

include/uapi/linux/alloc_tag.h:
struct allocinfo_get_at {
        /* inputs */
        __u64 pos;
        struct allocinfo_filter filter;
        /* output */
        struct allocinfo_tag_data data;
};
...
#define ALLOCINFO_IOC_GET_AT            _IOWR(ALLOCINFO_IOC_BASE, 
_ALLOCINFO_IOC_GET_AT,        \
                                              struct allocinfo_get_at)

Because ALLOCINFO_IOC_GET_AT encodes sizeof(struct allocinfo_get_at) via
_IOWR(), and allocinfo_ioctl() -> allocinfo_ioctl_get_at() in mm/alloc_tag.c
matches only the new command value and copies sizeof(struct allocinfo_get_at),
won't userspace binaries built against the previous uapi header fail with
-ENOIOCTLCMD (-ENOTTY) when invoking ioctl(fd, ALLOCINFO_IOC_GET_AT, &params)?

[ ... ]

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=3

Reply via email to