Hi all,
This patch series addresses the issue reported in the following issue: https://git.morello-project.org/morello/kernel/linux/-/issues/42
Changes available at: https://git.morello-project.org/Sevenarth/linux/-/commits/morello/6.1-fix-fc...
Luca Vizzarro (4): fs/fcntl: Fix F_SETLEASE argument type fs/fcntl: Fix F_SETSIG argument type fs/fcntl: Fix F_SETPIPE_SZ argument type fs/fcntl: Fix F_ADD_SEALS argument type
fs/cifs/cifsfs.c | 2 +- fs/fcntl.c | 2 +- fs/libfs.c | 2 +- fs/locks.c | 22 +++++++++++----------- fs/nfs/nfs4file.c | 2 +- fs/nfs/nfs4proc.c | 4 ++-- fs/pipe.c | 6 +++--- include/linux/fs.h | 12 ++++++------ include/linux/memfd.h | 4 ++-- include/linux/pipe_fs_i.h | 4 ++-- mm/memfd.c | 6 +----- 11 files changed, 31 insertions(+), 35 deletions(-)
The interface for fcntl expects the argument passed for the command F_SETLEASE 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/cifs/cifsfs.c | 2 +- fs/libfs.c | 2 +- fs/locks.c | 22 +++++++++++----------- fs/nfs/nfs4file.c | 2 +- fs/nfs/nfs4proc.c | 4 ++-- include/linux/fs.h | 12 ++++++------ 6 files changed, 22 insertions(+), 22 deletions(-)
diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c index 712a43161448..7e519b6a3f6b 100644 --- a/fs/cifs/cifsfs.c +++ b/fs/cifs/cifsfs.c @@ -1065,7 +1065,7 @@ static loff_t cifs_llseek(struct file *file, loff_t offset, int whence) }
static int -cifs_setlease(struct file *file, long arg, struct file_lock **lease, void **priv) +cifs_setlease(struct file *file, int arg, struct file_lock **lease, void **priv) { /* * Note that this is called by vfs setlease with i_lock held to diff --git a/fs/libfs.c b/fs/libfs.c index 682d56345a1c..ee47a22605c0 100644 --- a/fs/libfs.c +++ b/fs/libfs.c @@ -1258,7 +1258,7 @@ EXPORT_SYMBOL(alloc_anon_inode); * All arguments are ignored and it just returns -EINVAL. */ int -simple_nosetlease(struct file *filp, long arg, struct file_lock **flp, +simple_nosetlease(struct file *filp, int arg, struct file_lock **flp, void **priv) { return -EINVAL; diff --git a/fs/locks.c b/fs/locks.c index 607f94a0e789..7405b02db496 100644 --- a/fs/locks.c +++ b/fs/locks.c @@ -437,7 +437,7 @@ static void flock_make_lock(struct file *filp, struct file_lock *fl, int type) fl->fl_end = OFFSET_MAX; }
-static int assign_type(struct file_lock *fl, long type) +static int assign_type(struct file_lock *fl, int type) { switch (type) { case F_RDLCK: @@ -548,7 +548,7 @@ static const struct lock_manager_operations lease_manager_ops = { /* * Initialize a lease, use the default lock manager operations */ -static int lease_init(struct file *filp, long type, struct file_lock *fl) +static int lease_init(struct file *filp, int type, struct file_lock *fl) { if (assign_type(fl, type) != 0) return -EINVAL; @@ -566,7 +566,7 @@ static int lease_init(struct file *filp, long type, struct file_lock *fl) }
/* Allocate a file_lock initialised to this type of lease */ -static struct file_lock *lease_alloc(struct file *filp, long type) +static struct file_lock *lease_alloc(struct file *filp, int type) { struct file_lock *fl = locks_alloc_lock(); int error = -ENOMEM; @@ -1665,7 +1665,7 @@ int fcntl_getlease(struct file *filp) * conflict with the lease we're trying to set. */ static int -check_conflicting_open(struct file *filp, const long arg, int flags) +check_conflicting_open(struct file *filp, const int arg, int flags) { struct inode *inode = locks_inode(filp); int self_wcount = 0, self_rcount = 0; @@ -1700,7 +1700,7 @@ check_conflicting_open(struct file *filp, const long arg, int flags) }
static int -generic_add_lease(struct file *filp, long arg, struct file_lock **flp, void **priv) +generic_add_lease(struct file *filp, int arg, struct file_lock **flp, void **priv) { struct file_lock *fl, *my_fl = NULL, *lease; struct inode *inode = locks_inode(filp); @@ -1858,7 +1858,7 @@ static int generic_delete_lease(struct file *filp, void *owner) * The (input) flp->fl_lmops->lm_break function is required * by break_lease(). */ -int generic_setlease(struct file *filp, long arg, struct file_lock **flp, +int generic_setlease(struct file *filp, int arg, struct file_lock **flp, void **priv) { struct inode *inode = locks_inode(filp); @@ -1905,7 +1905,7 @@ lease_notifier_chain_init(void) }
static inline void -setlease_notifier(long arg, struct file_lock *lease) +setlease_notifier(int arg, struct file_lock *lease) { if (arg != F_UNLCK) srcu_notifier_call_chain(&lease_notifier_chain, arg, lease); @@ -1930,7 +1930,7 @@ lease_notifier_chain_init(void) }
static inline void -setlease_notifier(long arg, struct file_lock *lease) +setlease_notifier(int arg, struct file_lock *lease) { }
@@ -1965,7 +1965,7 @@ EXPORT_SYMBOL_GPL(lease_unregister_notifier); * may be NULL if the lm_setup operation doesn't require it. */ int -vfs_setlease(struct file *filp, long arg, struct file_lock **lease, void **priv) +vfs_setlease(struct file *filp, int arg, struct file_lock **lease, void **priv) { if (lease) setlease_notifier(arg, *lease); @@ -1976,7 +1976,7 @@ vfs_setlease(struct file *filp, long arg, struct file_lock **lease, void **priv) } EXPORT_SYMBOL_GPL(vfs_setlease);
-static int do_fcntl_add_lease(unsigned int fd, struct file *filp, long arg) +static int do_fcntl_add_lease(unsigned int fd, struct file *filp, int arg) { struct file_lock *fl; struct fasync_struct *new; @@ -2011,7 +2011,7 @@ static int do_fcntl_add_lease(unsigned int fd, struct file *filp, long arg) * Note that you also need to call %F_SETSIG to * receive a signal when the lease is broken. */ -int fcntl_setlease(unsigned int fd, struct file *filp, long arg) +int fcntl_setlease(unsigned int fd, struct file *filp, int arg) { if (arg == F_UNLCK) return vfs_setlease(filp, F_UNLCK, NULL, (void **)&filp); diff --git a/fs/nfs/nfs4file.c b/fs/nfs/nfs4file.c index 9eb181287879..1bc7a05e7e7e 100644 --- a/fs/nfs/nfs4file.c +++ b/fs/nfs/nfs4file.c @@ -442,7 +442,7 @@ void nfs42_ssc_unregister_ops(void) } #endif /* CONFIG_NFS_V4_2 */
-static int nfs4_setlease(struct file *file, long arg, struct file_lock **lease, +static int nfs4_setlease(struct file *file, int arg, struct file_lock **lease, void **priv) { return nfs4_proc_setlease(file, arg, lease, priv); diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c index 86ed5c0142c3..d7529f2761bb 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -7553,7 +7553,7 @@ static int nfs4_delete_lease(struct file *file, void **priv) return generic_setlease(file, F_UNLCK, NULL, priv); }
-static int nfs4_add_lease(struct file *file, long arg, struct file_lock **lease, +static int nfs4_add_lease(struct file *file, int arg, struct file_lock **lease, void **priv) { struct inode *inode = file_inode(file); @@ -7571,7 +7571,7 @@ static int nfs4_add_lease(struct file *file, long arg, struct file_lock **lease, return -EAGAIN; }
-int nfs4_proc_setlease(struct file *file, long arg, struct file_lock **lease, +int nfs4_proc_setlease(struct file *file, int arg, struct file_lock **lease, void **priv) { switch (arg) { diff --git a/include/linux/fs.h b/include/linux/fs.h index ded8c97516f3..14ea6adc5e7b 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -1151,7 +1151,7 @@ extern int fcntl_setlk64(unsigned int, struct file *, unsigned int, struct flock64 *); #endif
-extern int fcntl_setlease(unsigned int fd, struct file *filp, long arg); +extern int fcntl_setlease(unsigned int fd, struct file *filp, int arg); extern int fcntl_getlease(struct file *filp);
/* fs/locks.c */ @@ -1173,8 +1173,8 @@ extern int vfs_cancel_lock(struct file *filp, struct file_lock *fl); extern int locks_lock_inode_wait(struct inode *inode, struct file_lock *fl); extern int __break_lease(struct inode *inode, unsigned int flags, unsigned int type); extern void lease_get_mtime(struct inode *, struct timespec64 *time); -extern int generic_setlease(struct file *, long, struct file_lock **, void **priv); -extern int vfs_setlease(struct file *, long, struct file_lock **, void **); +extern int generic_setlease(struct file *, int, struct file_lock **, void **priv); +extern int vfs_setlease(struct file *, int, struct file_lock **, void **); extern int lease_modify(struct file_lock *, int, struct list_head *);
struct notifier_block; @@ -1212,7 +1212,7 @@ static inline int fcntl_setlk64(unsigned int fd, struct file *file, return -EACCES; } #endif -static inline int fcntl_setlease(unsigned int fd, struct file *filp, long arg) +static inline int fcntl_setlease(unsigned int fd, struct file *filp, int arg) { return -EINVAL; } @@ -2131,7 +2131,7 @@ struct file_operations { int (*flock) (struct file *, int, struct file_lock *); ssize_t (*splice_write)(struct pipe_inode_info *, struct file *, loff_t *, size_t, unsigned int); ssize_t (*splice_read)(struct file *, loff_t *, struct pipe_inode_info *, size_t, unsigned int); - int (*setlease)(struct file *, long, struct file_lock **, void **); + int (*setlease)(struct file *, int, struct file_lock **, void **); long (*fallocate)(struct file *file, int mode, loff_t offset, loff_t len); void (*show_fdinfo)(struct seq_file *m, struct file *f); @@ -3350,7 +3350,7 @@ extern int simple_write_begin(struct file *file, struct address_space *mapping, extern const struct address_space_operations ram_aops; extern int always_delete_dentry(const struct dentry *); extern struct inode *alloc_anon_inode(struct super_block *); -extern int simple_nosetlease(struct file *, long, struct file_lock **, void **); +extern int simple_nosetlease(struct file *, int, struct file_lock **, void **); extern const struct dentry_operations simple_dentry_operations;
extern struct dentry *simple_lookup(struct inode *, struct dentry *, unsigned int flags);
On 01-02-2023 16:50, Luca Vizzarro wrote:
diff --git a/include/linux/fs.h b/include/linux/fs.h index ded8c97516f3..14ea6adc5e7b 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -1151,7 +1151,7 @@ extern int fcntl_setlk64(unsigned int, struct file *, unsigned int, struct flock64 *); #endif -extern int fcntl_setlease(unsigned int fd, struct file *filp, long arg); +extern int fcntl_setlease(unsigned int fd, struct file *filp, int arg); extern int fcntl_getlease(struct file *filp); /* fs/locks.c */ @@ -1173,8 +1173,8 @@ extern int vfs_cancel_lock(struct file *filp, struct file_lock *fl); extern int locks_lock_inode_wait(struct inode *inode, struct file_lock *fl); extern int __break_lease(struct inode *inode, unsigned int flags, unsigned int type); extern void lease_get_mtime(struct inode *, struct timespec64 *time); -extern int generic_setlease(struct file *, long, struct file_lock **, void **priv); -extern int vfs_setlease(struct file *, long, struct file_lock **, void **); +extern int generic_setlease(struct file *, int, struct file_lock **, void **priv); +extern int vfs_setlease(struct file *, int, struct file_lock **, void **); extern int lease_modify(struct file_lock *, int, struct list_head *); struct notifier_block; @@ -1212,7 +1212,7 @@ static inline int fcntl_setlk64(unsigned int fd, struct file *file, return -EACCES; } #endif -static inline int fcntl_setlease(unsigned int fd, struct file *filp, long arg) +static inline int fcntl_setlease(unsigned int fd, struct file *filp, int arg) { return -EINVAL; } @@ -2131,7 +2131,7 @@ struct file_operations { int (*flock) (struct file *, int, struct file_lock *); ssize_t (*splice_write)(struct pipe_inode_info *, struct file *, loff_t *, size_t, unsigned int); ssize_t (*splice_read)(struct file *, loff_t *, struct pipe_inode_info *, size_t, unsigned int);
- int (*setlease)(struct file *, long, struct file_lock **, void **);
- int (*setlease)(struct file *, int, struct file_lock **, void **); long (*fallocate)(struct file *file, int mode, loff_t offset, loff_t len); void (*show_fdinfo)(struct seq_file *m, struct file *f);
@@ -3350,7 +3350,7 @@ extern int simple_write_begin(struct file *file, struct address_space *mapping, extern const struct address_space_operations ram_aops; extern int always_delete_dentry(const struct dentry *); extern struct inode *alloc_anon_inode(struct super_block *); -extern int simple_nosetlease(struct file *, long, struct file_lock **, void **); +extern int simple_nosetlease(struct file *, int, struct file_lock **, void **); extern const struct dentry_operations simple_dentry_operations; extern struct dentry *simple_lookup(struct inode *, struct dentry *, unsigned int flags);
Hi Luca,
There are 2 more occurrences of setlease functions in include/linux/fs.h I think:
static inline int generic_setlease(struct file *filp, long arg, struct file_lock **flp, void **priv) { return -EINVAL; }
static inline int vfs_setlease(struct file *filp, long arg, struct file_lock **lease, void **priv) { return -EINVAL; }
Otherwise, looks good!
Thanks, Tudor
The interface for fcntl expects the argument passed for the command F_SETSIG 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 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/fcntl.c b/fs/fcntl.c index 918d0136d12b..22eb0ae23421 100644 --- a/fs/fcntl.c +++ b/fs/fcntl.c @@ -390,7 +390,7 @@ static long do_fcntl(int fd, unsigned int cmd, user_uintptr_t arg, break; case F_SETSIG: /* arg == 0 restores default behaviour. */ - if (!valid_signal(arg)) { + if (!valid_signal((int)arg)) { break; } err = 0;
On 01-02-2023 16:50, Luca Vizzarro wrote:
The interface for fcntl expects the argument passed for the command F_SETSIG 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 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/fcntl.c b/fs/fcntl.c index 918d0136d12b..22eb0ae23421 100644 --- a/fs/fcntl.c +++ b/fs/fcntl.c @@ -390,7 +390,7 @@ static long do_fcntl(int fd, unsigned int cmd, user_uintptr_t arg, break; case F_SETSIG: /* arg == 0 restores default behaviour. */
if (!valid_signal(arg)) {
} err = 0;if (!valid_signal((int)arg)) { break;
Hi Luca,
arg is also used here just below this line:
filp->f_owner.signum = arg;
signum is an int, so the top bytes are discarded, so it's fine, but this could easily be missed when something changes. I suggest having a local int variable, such as:
int argi = (int)arg;
and use this variable in all the cases where int is expected. This is a similar approach to argp, for the cases where a user pointer is expected. This would make the code clearer as well IMO.
Thanks, Tudor
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/pipe.c | 6 +++--- include/linux/pipe_fs_i.h | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-)
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) { 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
The interface for fcntl expects the argument passed for the command F_ADD_SEALS 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 --- include/linux/memfd.h | 4 ++-- mm/memfd.c | 6 +----- 2 files changed, 3 insertions(+), 7 deletions(-)
diff --git a/include/linux/memfd.h b/include/linux/memfd.h index 4f1600413f91..e7abf6fa4c52 100644 --- a/include/linux/memfd.h +++ b/include/linux/memfd.h @@ -5,9 +5,9 @@ #include <linux/file.h>
#ifdef CONFIG_MEMFD_CREATE -extern long memfd_fcntl(struct file *file, unsigned int cmd, unsigned long arg); +extern long memfd_fcntl(struct file *file, unsigned int cmd, unsigned int arg); #else -static inline long memfd_fcntl(struct file *f, unsigned int c, unsigned long a) +static inline long memfd_fcntl(struct file *f, unsigned int c, unsigned int a) { return -EINVAL; } diff --git a/mm/memfd.c b/mm/memfd.c index 08f5f8304746..f97081ff9a0d 100644 --- a/mm/memfd.c +++ b/mm/memfd.c @@ -234,16 +234,12 @@ static int memfd_get_seals(struct file *file) return seals ? *seals : -EINVAL; }
-long memfd_fcntl(struct file *file, unsigned int cmd, unsigned long arg) +long memfd_fcntl(struct file *file, unsigned int cmd, unsigned int arg) { long error;
switch (cmd) { case F_ADD_SEALS: - /* disallow upper 32bit */ - if (arg > UINT_MAX) - return -EINVAL; - error = memfd_add_seals(file, arg); break; case F_GET_SEALS:
linux-morello@op-lists.linaro.org