Re: [PATCH][next] fs: Annotate struct file_handle with __counted_by() and use struct_size()

From: Jan Kara
Date: Tue Mar 26 2024 - 04:58:15 EST


On Mon 25-03-24 19:34:01, Gustavo A. R. Silva wrote:
> Prepare for the coming implementation by GCC and Clang of the __counted_by
> attribute. Flexible array members annotated with __counted_by can have
> their accesses bounds-checked at run-time via CONFIG_UBSAN_BOUNDS (for
> array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family
> functions).
>
> While there, use struct_size() helper, instead of the open-coded
> version.
>
> Signed-off-by: Gustavo A. R. Silva <gustavoars@xxxxxxxxxx>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@xxxxxxx>

Honza

> ---
> fs/fhandle.c | 8 ++++----
> include/linux/fs.h | 2 +-
> 2 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/fs/fhandle.c b/fs/fhandle.c
> index 57a12614addf..53ed54711cd2 100644
> --- a/fs/fhandle.c
> +++ b/fs/fhandle.c
> @@ -36,7 +36,7 @@ static long do_sys_name_to_handle(const struct path *path,
> if (f_handle.handle_bytes > MAX_HANDLE_SZ)
> return -EINVAL;
>
> - handle = kzalloc(sizeof(struct file_handle) + f_handle.handle_bytes,
> + handle = kzalloc(struct_size(handle, f_handle, f_handle.handle_bytes),
> GFP_KERNEL);
> if (!handle)
> return -ENOMEM;
> @@ -71,7 +71,7 @@ static long do_sys_name_to_handle(const struct path *path,
> /* copy the mount id */
> if (put_user(real_mount(path->mnt)->mnt_id, mnt_id) ||
> copy_to_user(ufh, handle,
> - sizeof(struct file_handle) + handle_bytes))
> + struct_size(handle, f_handle, handle_bytes)))
> retval = -EFAULT;
> kfree(handle);
> return retval;
> @@ -192,7 +192,7 @@ static int handle_to_path(int mountdirfd, struct file_handle __user *ufh,
> retval = -EINVAL;
> goto out_err;
> }
> - handle = kmalloc(sizeof(struct file_handle) + f_handle.handle_bytes,
> + handle = kmalloc(struct_size(handle, f_handle, f_handle.handle_bytes),
> GFP_KERNEL);
> if (!handle) {
> retval = -ENOMEM;
> @@ -202,7 +202,7 @@ static int handle_to_path(int mountdirfd, struct file_handle __user *ufh,
> *handle = f_handle;
> if (copy_from_user(&handle->f_handle,
> &ufh->f_handle,
> - f_handle.handle_bytes)) {
> + struct_size(ufh, f_handle, f_handle.handle_bytes))) {
> retval = -EFAULT;
> goto out_handle;
> }
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 034f0c918eea..1540e28d10d7 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -1033,7 +1033,7 @@ struct file_handle {
> __u32 handle_bytes;
> int handle_type;
> /* file identifier */
> - unsigned char f_handle[];
> + unsigned char f_handle[] __counted_by(handle_bytes);
> };
>
> static inline struct file *get_file(struct file *f)
> --
> 2.34.1
>
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR