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...
v3: - move the explicit cast change before updating the interfaces - fix typos v2: - added a new argi variable which performs explicit int casting - updated all of the commands with int argument to use argi explicitly - updated signatures for the abstract generic_setlease and vfs_setlease with int argument
Luca Vizzarro (4): fs/fcntl: Cast commands with int args explicitly fs/fcntl: Fix F_SETLEASE helper functions fs/fcntl: Fix F_SETPIPE_SZ helper functions fs/fcntl: Fix F_ADD_SEALS helper functions
fs/cifs/cifsfs.c | 2 +- fs/fcntl.c | 23 ++++++++++++----------- 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 | 16 ++++++++-------- include/linux/memfd.h | 4 ++-- include/linux/pipe_fs_i.h | 4 ++-- mm/memfd.c | 6 +----- 11 files changed, 44 insertions(+), 47 deletions(-)
According to the fcntl API specification some commands take an argument of type int and not long. In order to avoid access to undefined bits, we should explicitly cast the argument to int.
This commit creates an explicit cast to int of arg through a new variable called argi, and updates the relevant commands to use it.
Signed-off-by: Luca Vizzarro Luca.Vizzarro@arm.com --- fs/fcntl.c | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-)
diff --git a/fs/fcntl.c b/fs/fcntl.c index 918d0136d12b..520520fa875b 100644 --- a/fs/fcntl.c +++ b/fs/fcntl.c @@ -316,28 +316,29 @@ static long do_fcntl(int fd, unsigned int cmd, user_uintptr_t arg, struct file *filp) { void __user *argp = (void __user *)arg; + int argi = (int)arg; struct flock flock; long err = -EINVAL;
switch (cmd) { case F_DUPFD: - err = f_dupfd(arg, filp, 0); + err = f_dupfd(argi, filp, 0); break; case F_DUPFD_CLOEXEC: - err = f_dupfd(arg, filp, O_CLOEXEC); + err = f_dupfd(argi, filp, O_CLOEXEC); break; case F_GETFD: err = get_close_on_exec(fd) ? FD_CLOEXEC : 0; break; case F_SETFD: err = 0; - set_close_on_exec(fd, arg & FD_CLOEXEC); + set_close_on_exec(fd, argi & FD_CLOEXEC); break; case F_GETFL: err = filp->f_flags; break; case F_SETFL: - err = setfl(fd, filp, arg); + err = setfl(fd, filp, argi); break; #if BITS_PER_LONG != 32 /* 32-bit arches must use fcntl64() */ @@ -374,7 +375,7 @@ static long do_fcntl(int fd, unsigned int cmd, user_uintptr_t arg, force_successful_syscall_return(); break; case F_SETOWN: - err = f_setown(filp, arg, 1); + err = f_setown(filp, argi, 1); break; case F_GETOWN_EX: err = f_getown_ex(filp, arg); @@ -390,28 +391,28 @@ 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(argi)) { break; } err = 0; - filp->f_owner.signum = arg; + filp->f_owner.signum = argi; break; case F_GETLEASE: err = fcntl_getlease(filp); break; case F_SETLEASE: - err = fcntl_setlease(fd, filp, arg); + err = fcntl_setlease(fd, filp, argi); break; case F_NOTIFY: - err = fcntl_dirnotify(fd, filp, arg); + err = fcntl_dirnotify(fd, filp, argi); break; case F_SETPIPE_SZ: case F_GETPIPE_SZ: - err = pipe_fcntl(filp, cmd, arg); + err = pipe_fcntl(filp, cmd, argi); break; case F_ADD_SEALS: case F_GET_SEALS: - err = memfd_fcntl(filp, cmd, arg); + err = memfd_fcntl(filp, cmd, argi); break; case F_GET_RW_HINT: case F_SET_RW_HINT:
On 02/02/2023 17:06, Luca Vizzarro wrote:
According to the fcntl API specification some commands take an argument of type int and not long.
To be slightly more specific, commands that expect an integer (not a pointer) always take an int, not long.
In order to avoid access to undefined bits, we should explicitly cast the argument to int.
This commit creates an explicit cast to int of arg through a new variable called argi, and updates the relevant commands to use it.
Being slightly pickier than usual with the commit message as it's eventually for upstream: I think we can do without this paragraph, in general there isn't much point in paraphrasing what the patch does.
Signed-off-by: Luca Vizzarro Luca.Vizzarro@arm.com
fs/fcntl.c | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-)
diff --git a/fs/fcntl.c b/fs/fcntl.c index 918d0136d12b..520520fa875b 100644 --- a/fs/fcntl.c +++ b/fs/fcntl.c @@ -316,28 +316,29 @@ static long do_fcntl(int fd, unsigned int cmd, user_uintptr_t arg, struct file *filp) { void __user *argp = (void __user *)arg;
- int argi = (int)arg; struct flock flock; long err = -EINVAL;
switch (cmd) { case F_DUPFD:
err = f_dupfd(arg, filp, 0);
break; case F_DUPFD_CLOEXEC:err = f_dupfd(argi, filp, 0);
err = f_dupfd(arg, filp, O_CLOEXEC);
break; case F_GETFD: err = get_close_on_exec(fd) ? FD_CLOEXEC : 0; break; case F_SETFD: err = 0;err = f_dupfd(argi, filp, O_CLOEXEC);
set_close_on_exec(fd, arg & FD_CLOEXEC);
break; case F_GETFL: err = filp->f_flags; break; case F_SETFL:set_close_on_exec(fd, argi & FD_CLOEXEC);
err = setfl(fd, filp, arg);
err = setfl(fd, filp, argi);
While we're at it, we could also change the prototype of setfl() accordingly.
break;
#if BITS_PER_LONG != 32 /* 32-bit arches must use fcntl64() */ @@ -374,7 +375,7 @@ static long do_fcntl(int fd, unsigned int cmd, user_uintptr_t arg, force_successful_syscall_return(); break; case F_SETOWN:
err = f_setown(filp, arg, 1);
err = f_setown(filp, argi, 1);
Same here, in fact we could also simplify f_setown() a bit with s/unsigned long arg/int who/ in the prototype and removing the local variable "who".
break;
case F_GETOWN_EX: err = f_getown_ex(filp, arg); @@ -390,28 +391,28 @@ 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(argi)) { break;
filp->f_owner.signum = arg;
break; case F_GETLEASE: err = fcntl_getlease(filp); break; case F_SETLEASE:filp->f_owner.signum = argi;
err = fcntl_setlease(fd, filp, arg);
break; case F_NOTIFY:err = fcntl_setlease(fd, filp, argi);
err = fcntl_dirnotify(fd, filp, arg);
err = fcntl_dirnotify(fd, filp, argi);
Since you're going for a thorough clean, we could also propagate the type change to fcntl_dirnotify() and convert_arg() that it calls.
Kevin
break;
case F_SETPIPE_SZ: case F_GETPIPE_SZ:
err = pipe_fcntl(filp, cmd, arg);
break; case F_ADD_SEALS: case F_GET_SEALS:err = pipe_fcntl(filp, cmd, argi);
err = memfd_fcntl(filp, cmd, arg);
break; case F_GET_RW_HINT: case F_SET_RW_HINT:err = memfd_fcntl(filp, cmd, argi);
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 | 16 ++++++++-------- 6 files changed, 24 insertions(+), 24 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..d939351461fe 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; } @@ -1300,13 +1300,13 @@ static inline void lease_get_mtime(struct inode *inode, return; }
-static inline int generic_setlease(struct file *filp, long arg, +static inline int generic_setlease(struct file *filp, int arg, struct file_lock **flp, void **priv) { return -EINVAL; }
-static inline int vfs_setlease(struct file *filp, long arg, +static inline int vfs_setlease(struct file *filp, int arg, struct file_lock **lease, void **priv) { 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);
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:
Apologies,
Forgot to add the title to the cover letter, this should be:
[PATCH v3 0/4] Handling of int arguments in fcntl()
Best, Luca
On 02/02/2023 16:06, Luca Vizzarro wrote:
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...
v3:
- move the explicit cast change before updating the interfaces
- fix typos
v2:
- added a new argi variable which performs explicit int casting
- updated all of the commands with int argument to use argi explicitly
- updated signatures for the abstract generic_setlease and vfs_setlease with int argument
Luca Vizzarro (4): fs/fcntl: Cast commands with int args explicitly fs/fcntl: Fix F_SETLEASE helper functions fs/fcntl: Fix F_SETPIPE_SZ helper functions fs/fcntl: Fix F_ADD_SEALS helper functions
fs/cifs/cifsfs.c | 2 +- fs/fcntl.c | 23 ++++++++++++----------- 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 | 16 ++++++++-------- include/linux/memfd.h | 4 ++-- include/linux/pipe_fs_i.h | 4 ++-- mm/memfd.c | 6 +----- 11 files changed, 44 insertions(+), 47 deletions(-)
On 02-02-2023 16:13, Luca Vizzarro wrote:
Apologies,
Forgot to add the title to the cover letter, this should be:
[PATCH v3 0/4] Handling of int arguments in fcntl()
Best, Luca
On 02/02/2023 16:06, Luca Vizzarro wrote:
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...
Hi Luca,
The series looks good to me! Well done!
Thanks, Tudor
v3:
- move the explicit cast change before updating the interfaces
- fix typos
v2:
- added a new argi variable which performs explicit int casting
- updated all of the commands with int argument to use argi explicitly
- updated signatures for the abstract generic_setlease and
vfs_setlease with int argument
Luca Vizzarro (4): fs/fcntl: Cast commands with int args explicitly fs/fcntl: Fix F_SETLEASE helper functions fs/fcntl: Fix F_SETPIPE_SZ helper functions fs/fcntl: Fix F_ADD_SEALS helper functions
fs/cifs/cifsfs.c | 2 +- fs/fcntl.c | 23 ++++++++++++----------- 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 | 16 ++++++++-------- include/linux/memfd.h | 4 ++-- include/linux/pipe_fs_i.h | 4 ++-- mm/memfd.c | 6 +----- 11 files changed, 44 insertions(+), 47 deletions(-)
linux-morello mailing list -- linux-morello@op-lists.linaro.org To unsubscribe send an email to linux-morello-leave@op-lists.linaro.org
On 02/02/2023 17:06, Luca Vizzarro wrote:
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...
v3:
- move the explicit cast change before updating the interfaces
- fix typos
v2:
- added a new argi variable which performs explicit int casting
- updated all of the commands with int argument to use argi explicitly
- updated signatures for the abstract generic_setlease and vfs_setlease with int argument
Luca Vizzarro (4): fs/fcntl: Cast commands with int args explicitly fs/fcntl: Fix F_SETLEASE helper functions fs/fcntl: Fix F_SETPIPE_SZ helper functions fs/fcntl: Fix F_ADD_SEALS helper functions
The tags should correspond to the files they are touching, trying to match commits upstream. I'd suggest respectively "fnctl:", "fs:", "pipe:" and "memfd:".
It would also be nice to be more "functional" in these commit titles, you can't be very specific in so few characters but just "Fix" gives no idea about the nature of the issue. Maybe something along the lines of "interpreting/passing arg as int" would help. Happy to try and give more specific suggestions if needed.
The changes look good to me otherwise (with a few comments on patch 1), this is the most thorough approach possible.
Kevin
fs/cifs/cifsfs.c | 2 +- fs/fcntl.c | 23 ++++++++++++----------- 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 | 16 ++++++++-------- include/linux/memfd.h | 4 ++-- include/linux/pipe_fs_i.h | 4 ++-- mm/memfd.c | 6 +----- 11 files changed, 44 insertions(+), 47 deletions(-)
linux-morello@op-lists.linaro.org