Re: [RFC PATCH v4 1/4] md: add uapi definitions for the md control device

["Masalkhi, Abd Alrhman" <abd.masalkhi@gatech.edu>]

Hi Kuai,

On Thu, Oct 15, 2026 at 20:55 +0000, sashiko-bot@kernel.org wrote:
> [You don't often get email from sashiko-bot@kernel.org. Learn why this is=mportant at https://aka.ms/LearnAboutSenderIdentification ]
>
> Thank you for your contribution! Sashiko AI review found 1 potential issu=(s) to consider:
> - [Medium] md: implicit 4096-byte buffer contract in new MD_CTL_GET_BITMA=_FILE UAPI
> --
>
> commit 04cabfea66def1d91136adb7afff7c1b2c70d669
> Author: Abd-Alrhman Masalkhi <abd.masalkhi@gatech.edu>
>
> md: add uapi definitions for the md control device
>
> Add the ioctl commands and structures for /dev/md-control, a misc device
> that manages md arrays without opening the md block device. The device
> itself is added in the next patch.
>
>> diff --git a/include/uapi/linux/raid/md_u.h b/include/uapi/linux/raid/md=u.h
>> index a893010735fba..f5a84605b6772 100644
>> --- a/include/uapi/linux/raid/md_u.h
>> +++ b/include/uapi/linux/raid/md_u.h
> [ ... ]
>> @@ -146,4 +174,90 @@ typedef struct mdu_param_s
>>       int                     max_fault;      /* unused for now */
>>  } mdu_param_t;
>>
>> +/* structures for the MD misc control driver */
> [ ... ]
>> +struct mdu_ioctl {
>> +     char name[MD_NAME_LEN];
>> +     char uuid[MD_UUID_LEN];
>> +     __u64 dev;
>> +     __u32 flags;
>> +     __u32 padding;
>> +     union {
>> +             __u64 array;
>> +             __u64 disk;
>> +             __u64 param;
>> +             __u64 bitmap_file;
>
> [Severity: Medium]
> Does this implicit 4096-byte buffer contract risk user-space memory
> corruption?
>
> Looking at the new UAPI struct, it defines a generic pointer for the bitm=p
> file path but lacks a length field or an associated typed payload structu=e.
>
> When a user-space application calls the new MD_CTL_GET_BITMAP_FILE ioctl =ith
> a standard dynamically allocated string buffer, the ioctl handler directly
> passes the generic user pointer to get_bitmap_file():
>
> drivers/md/md.c:mdctl_ioctl() {
>     ...
>         case MD_CTL_GET_BITMAP_FILE:
>                 err = get_bitmap_file(mddev, arg);
>                 goto out;
>     ...
> }
>
> The kernel blindly copies sizeof(mdu_bitmap_file_t) (4096 bytes) to the u=er
> pointer without validating the buffer's capacity:
>
> drivers/md/md.c:get_bitmap_file() {
>     ...
>         if (err == 0 &&
>             copy_to_user(arg, file, sizeof(*file)))
>                 err = -EFAULT;
>     ...
> }
>
> Without an explicit size parameter or a dedicated structure to indicate t=e
> expected 4096-byte size, callers allocating a smaller buffer could face
> memory corruption.

MD_CTL_GET_BITMAP_FILE expects bitmap_file to point to mdu_bitmap_file_t
(4096 bytes), just as the old GET_BITMAP_FILE. A caller that passes a
smaller buffer is misusing the API, and the kernel side is safe either
way.

>
>> +             __u64 arg;
>> +     };
>> +};
>> +
>> +/*
>> + * If set, MD_STOP_ARRAY will switch the array to read-only mode instead
>> + * of fully stopping it.
>> + */
>> +#define MD_CTL_RO_FLAG              (1u << 0)
>> +
>>  #endif /* _UAPI_MD_U_H */
>
> --
> Sashiko AI review · https://nam12.safelinks.protection.outlook.com/?=rl=https%3A%2F%2Fsashiko.dev%2F%23%2Fpatchset%2F20261015204640.1184848-1=abd.masalkhi%40gatech.edu%3Fpart%3D1&data=05%7C02%7Cabd.masalkhi%40gatec=.edu%7C4001a1abeead450e95cc08df2afe9fe4%7C482198bbae7b4b258b7a6d7f32faa083=7C1%7C0%7C639276945233560298%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRyd=UsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0=7C%7C%7C&sdata=wTMjZcauN65ftf9ffrShA6LLEqw14zBQU2nLqpbFTwo%3D&reserved=3D0

-- 
Best Regards,
Abd-Alrhman