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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] md: implicit 4096-byte buffer contract in new MD_CTL_GET_BITMAP_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 bitmap
file path but lacks a length field or an associated typed payload structure.

When a user-space application calls the new MD_CTL_GET_BITMAP_FILE ioctl with
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 user
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 the
expected 4096-byte size, callers allocating a smaller buffer could face
memory corruption.

> +		__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://sashiko.dev/#/patchset/20261015204640.1184848-1-abd.masalkhi@gatech.edu?part=1