On 02-02-2023 12:13, Luca Vizzarro wrote:
The interface for fcntl expects the argument passed for the command F_SETPIPE_SZ to be of type int. The current code wrongly treats it as a long.
This commit changes the signature of all the related and helper functions so that they treat the argument as int instead of long.
Signed-off-by: Luca Vizzarro Luca.Vizzarro@arm.com
fs/fcntl.c | 2 +- fs/pipe.c | 6 +++--- include/linux/pipe_fs_i.h | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/fs/fcntl.c b/fs/fcntl.c index db9f03ab23d3..dc416037d9fc 100644 --- a/fs/fcntl.c +++ b/fs/fcntl.c @@ -408,7 +408,7 @@ static long do_fcntl(int fd, unsigned int cmd, user_uintptr_t arg, break; case F_SETPIPE_SZ: case F_GETPIPE_SZ:
err = pipe_fcntl(filp, cmd, arg);
err = pipe_fcntl(filp, cmd, argi);diff --git a/fs/pipe.c b/fs/pipe.c index 76169e370e64..79f3eb118f11 100644 --- a/fs/pipe.c +++ b/fs/pipe.c @@ -1231,7 +1231,7 @@ const struct file_operations pipefifo_fops = {
- Currently we rely on the pipe array holding a power-of-2 number
- of pages. Returns 0 on error.
*/ -unsigned int round_pipe_size(unsigned long size) +unsigned int round_pipe_size(unsigned int size) { if (size > (1U << 31)) return 0; @@ -1314,7 +1314,7 @@ int pipe_resize_ring(struct pipe_inode_info *pipe, unsigned int nr_slots)
- Allocate a new array of pipe buffers and copy the info over. Returns the
- pipe size if successful, or return -ERROR on error.
*/ -static long pipe_set_size(struct pipe_inode_info *pipe, unsigned long arg) +static long pipe_set_size(struct pipe_inode_info *pipe, unsigned int arg) { unsigned long user_bufs; unsigned int nr_slots, size; @@ -1382,7 +1382,7 @@ struct pipe_inode_info *get_pipe_info(struct file *file, bool for_splice) return pipe; } -long pipe_fcntl(struct file *file, unsigned int cmd, unsigned long arg) +long pipe_fcntl(struct file *file, unsigned int cmd, unsigned int arg)
Hi Luca,
Please, correct me if I'm wrong, but this function seems to be called for both F_SETPIPE_SZ and F_GETPIPE_SZ. The issue is the type of arg expected for F_GETPIPE_SZ is void (no argument). So in the case of F_GETPIPE_SZ, the kernel accesses undefined bytes even with this change. My first idea is to separate the handler into two, one for F_SETPIPE_SZ and one for F_GETPIPE_SZ. I haven't looked in detail to see if that would cause issues, so feel free to explore the issue and other options as well.
Thanks, Tudor
{ struct pipe_inode_info *pipe; long ret; diff --git a/include/linux/pipe_fs_i.h b/include/linux/pipe_fs_i.h index 6cb65df3e3ba..49da593edfed 100644 --- a/include/linux/pipe_fs_i.h +++ b/include/linux/pipe_fs_i.h @@ -253,10 +253,10 @@ bool pipe_is_unprivileged_user(void); #ifdef CONFIG_WATCH_QUEUE int pipe_resize_ring(struct pipe_inode_info *pipe, unsigned int nr_slots); #endif -long pipe_fcntl(struct file *, unsigned int, unsigned long arg); +long pipe_fcntl(struct file *, unsigned int, unsigned int arg); struct pipe_inode_info *get_pipe_info(struct file *file, bool for_splice); int create_pipe_files(struct file **, int); -unsigned int round_pipe_size(unsigned long size); +unsigned int round_pipe_size(unsigned int size); #endif