[PATCH 0/5] Alter fcntl to handle int arguments correctly

[PATCH 0/5] Alter fcntl to handle int arguments correctly

[Luca Vizzarro]

According to the documentation of fcntl, some commands take an int as
argument. In practice not all of them enforce this behaviour, as they
instead accept a more permissive long and in most cases not even a
range check is performed.

An issue could possibly arise from a combination of the handling of the
varargs in user space and the ABI rules of the target, which may result
in the top bits of an int argument being non-zero.

This issue was originally raised and detailed in the following thread:
  https://lore.kernel.org/linux-api/Y1%2FDS6uoWP7OSkmd@arm.com/

This series modifies the interested commands so that they explicitly
take an int argument. It also propagates this change down to helper and
related functions as necessary.

This series is also available on my fork at:
  https://git.morello-project.org/Sevenarth/linux/-/commits/fcntl-int-handling

Best regards,
Luca Vizzarro

Luca Vizzarro (5):
  fcntl: Cast commands with int args explicitly
  fs: Pass argument to fcntl_setlease as int
  pipe: Pass argument of pipe_fcntl as int
  memfd: Pass argument of memfd_fcntl as int
  dnotify: Pass argument of fcntl_dirnotify as int

 fs/cifs/cifsfs.c            |  2 +-
 fs/fcntl.c                  | 29 +++++++++++++++--------------
 fs/libfs.c                  |  2 +-
 fs/locks.c                  | 20 ++++++++++----------
 fs/nfs/nfs4_fs.h            |  2 +-
 fs/nfs/nfs4file.c           |  2 +-
 fs/nfs/nfs4proc.c           |  4 ++--
 fs/notify/dnotify/dnotify.c |  4 ++--
 fs/pipe.c                   |  6 +++---
 include/linux/dnotify.h     |  4 ++--
 include/linux/filelock.h    | 12 ++++++------
 include/linux/fs.h          |  6 +++---
 include/linux/memfd.h       |  4 ++--
 include/linux/pipe_fs_i.h   |  4 ++--
 mm/memfd.c                  |  6 +-----
 15 files changed, 52 insertions(+), 55 deletions(-)

--
2.34.1

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

[PATCH 1/5] fcntl: Cast commands with int args explicitly

[Luca Vizzarro]

According to the fcntl API specification commands that expect an
integer, hence not a pointer, always take an int and not long. In
order to avoid access to undefined bits, we should explicitly cast
the argument to int.

Cc: Kevin Brodsky <Kevin.Brodsky@arm.com>
Cc: Szabolcs Nagy <Szabolcs.Nagy@arm.com>
Cc: "Theodore Ts'o" <tytso@mit.edu>
Cc: David Laight <David.Laight@ACULAB.com>
Cc: Mark Rutland <Mark.Rutland@arm.com>
Cc: Alexander Viro <viro@zeniv.linux.org.uk>
Cc: Christian Brauner <brauner@kernel.org>
Cc: Jeff Layton <jlayton@kernel.org>
Cc: Chuck Lever <chuck.lever@oracle.com>
Cc: linux-fsdevel@vger.kernel.org
Signed-off-by: Luca Vizzarro <Luca.Vizzarro@arm.com>
---
 fs/fcntl.c         | 29 +++++++++++++++--------------
 include/linux/fs.h |  2 +-
 2 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/fs/fcntl.c b/fs/fcntl.c
index b622be119706..e871009f6c88 100644
--- a/fs/fcntl.c
+++ b/fs/fcntl.c
@@ -34,7 +34,7 @@

 #define SETFL_MASK (O_APPEND | O_NONBLOCK | O_NDELAY | O_DIRECT | O_NOATIME)

-static int setfl(int fd, struct file * filp, unsigned long arg)
+static int setfl(int fd, struct file * filp, unsigned int arg)
 {
        struct inode * inode = file_inode(filp);
        int error = 0;
@@ -112,11 +112,11 @@ void __f_setown(struct file *filp, struct pid *pid, enum pid_type type,
 }
 EXPORT_SYMBOL(__f_setown);

-int f_setown(struct file *filp, unsigned long arg, int force)
+int f_setown(struct file *filp, int who, int force)
 {
        enum pid_type type;
        struct pid *pid = NULL;
-       int who = arg, ret = 0;
+       int ret = 0;

        type = PIDTYPE_TGID;
        if (who < 0) {
@@ -317,28 +317,29 @@ static long do_fcntl(int fd, unsigned int cmd, unsigned long 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() */
@@ -375,7 +376,7 @@ static long do_fcntl(int fd, unsigned int cmd, unsigned long 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);
@@ -391,28 +392,28 @@ static long do_fcntl(int fd, unsigned int cmd, unsigned long 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:
diff --git a/include/linux/fs.h b/include/linux/fs.h
index c85916e9f7db..8da79822dbba 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1050,7 +1050,7 @@ extern void fasync_free(struct fasync_struct *);
 extern void kill_fasync(struct fasync_struct **, int, int);

 extern void __f_setown(struct file *filp, struct pid *, enum pid_type, int force);
-extern int f_setown(struct file *filp, unsigned long arg, int force);
+extern int f_setown(struct file *filp, int who, int force);
 extern void f_delown(struct file *filp);
 extern pid_t f_getown(struct file *filp);
 extern int send_sigurg(struct fown_struct *fown);
--
2.34.1

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

[PATCH 2/5] fs: Pass argument to fcntl_setlease as int

[Luca Vizzarro]

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. In order to avoid access to undefined bits, we should explicitly
cast the argument to int.

Cc: Kevin Brodsky <Kevin.Brodsky@arm.com>
Cc: Szabolcs Nagy <Szabolcs.Nagy@arm.com>
Cc: "Theodore Ts'o" <tytso@mit.edu>
Cc: David Laight <David.Laight@ACULAB.com>
Cc: Mark Rutland <Mark.Rutland@arm.com>
Cc: Alexander Viro <viro@zeniv.linux.org.uk>
Cc: Christian Brauner <brauner@kernel.org>
Cc: Jeff Layton <jlayton@kernel.org>
Cc: Chuck Lever <chuck.lever@oracle.com>
Cc: linux-fsdevel@vger.kernel.org
Cc: Trond Myklebust <trond.myklebust@hammerspace.com>
Cc: Anna Schumaker <anna@kernel.org>
Cc: linux-cifs@vger.kernel.org
Cc: linux-nfs@vger.kernel.org
Signed-off-by: Luca Vizzarro <Luca.Vizzarro@arm.com>
---
 fs/cifs/cifsfs.c         |  2 +-
 fs/libfs.c               |  2 +-
 fs/locks.c               | 20 ++++++++++----------
 fs/nfs/nfs4_fs.h         |  2 +-
 fs/nfs/nfs4file.c        |  2 +-
 fs/nfs/nfs4proc.c        |  4 ++--
 include/linux/filelock.h | 12 ++++++------
 include/linux/fs.h       |  4 ++--
 8 files changed, 24 insertions(+), 24 deletions(-)

diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index ac9034fce409..ad5b2cfe8320 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -1069,7 +1069,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 4eda519c3002..1c451e76560c 100644
--- a/fs/libfs.c
+++ b/fs/libfs.c
@@ -1274,7 +1274,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 df8b26a42524..265b5190db3e 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -438,7 +438,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:
@@ -549,7 +549,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;
@@ -567,7 +567,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;
@@ -1666,7 +1666,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 = file_inode(filp);
        int self_wcount = 0, self_rcount = 0;
@@ -1701,7 +1701,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 = file_inode(filp);
@@ -1859,7 +1859,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 = file_inode(filp);
@@ -1906,7 +1906,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);
@@ -1942,7 +1942,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);
@@ -1953,7 +1953,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;
@@ -1988,7 +1988,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/nfs4_fs.h b/fs/nfs/nfs4_fs.h
index 4c9f8bd866ab..47c5c1f86d66 100644
--- a/fs/nfs/nfs4_fs.h
+++ b/fs/nfs/nfs4_fs.h
@@ -328,7 +328,7 @@ extern int update_open_stateid(struct nfs4_state *state,
                                const nfs4_stateid *open_stateid,
                                const nfs4_stateid *deleg_stateid,
                                fmode_t fmode);
-extern int nfs4_proc_setlease(struct file *file, long arg,
+extern int nfs4_proc_setlease(struct file *file, int arg,
                              struct file_lock **lease, void **priv);
 extern int nfs4_proc_get_lease_time(struct nfs_client *clp,
                struct nfs_fsinfo *fsinfo);
diff --git a/fs/nfs/nfs4file.c b/fs/nfs/nfs4file.c
index 2563ed8580f3..26c2d3539d75 100644
--- a/fs/nfs/nfs4file.c
+++ b/fs/nfs/nfs4file.c
@@ -438,7 +438,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 5607b1e2b821..ba59ad558209 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -7559,7 +7559,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);
@@ -7577,7 +7577,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/filelock.h b/include/linux/filelock.h
index efcdd1631d9b..95e868e09e29 100644
--- a/include/linux/filelock.h
+++ b/include/linux/filelock.h
@@ -144,7 +144,7 @@ int fcntl_setlk64(unsigned int, struct file *, unsigned int,
                        struct flock64 *);
 #endif

-int fcntl_setlease(unsigned int fd, struct file *filp, long arg);
+int fcntl_setlease(unsigned int fd, struct file *filp, int arg);
 int fcntl_getlease(struct file *filp);

 /* fs/locks.c */
@@ -167,8 +167,8 @@ bool vfs_inode_has_locks(struct inode *inode);
 int locks_lock_inode_wait(struct inode *inode, struct file_lock *fl);
 int __break_lease(struct inode *inode, unsigned int flags, unsigned int type);
 void lease_get_mtime(struct inode *, struct timespec64 *time);
-int generic_setlease(struct file *, long, struct file_lock **, void **priv);
-int vfs_setlease(struct file *, long, struct file_lock **, void **);
+int generic_setlease(struct file *, int, struct file_lock **, void **priv);
+int vfs_setlease(struct file *, int, struct file_lock **, void **);
 int lease_modify(struct file_lock *, int, struct list_head *);

 struct notifier_block;
@@ -213,7 +213,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;
 }
@@ -306,13 +306,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;
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 8da79822dbba..0c9367980636 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1779,7 +1779,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);
@@ -2914,7 +2914,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);
--
2.34.1

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

[PATCH 3/5] pipe: Pass argument of pipe_fcntl as int

[Luca Vizzarro]

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. In order to avoid access to undefined bits, we should explicitly
cast the argument to int.

Cc: Kevin Brodsky <Kevin.Brodsky@arm.com>
Cc: Szabolcs Nagy <Szabolcs.Nagy@arm.com>
Cc: "Theodore Ts'o" <tytso@mit.edu>
Cc: David Laight <David.Laight@ACULAB.com>
Cc: Mark Rutland <Mark.Rutland@arm.com>
Cc: Alexander Viro <viro@zeniv.linux.org.uk>
Cc: Christian Brauner <brauner@kernel.org>
Cc: Jeff Layton <jlayton@kernel.org>
Cc: Chuck Lever <chuck.lever@oracle.com>
Cc: linux-fsdevel@vger.kernel.org
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 42c7ff41c2db..5b718342105f 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 d2c3f16cf6b1..033d77f0c568 100644
--- a/include/linux/pipe_fs_i.h
+++ b/include/linux/pipe_fs_i.h
@@ -273,10 +273,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
--
2.34.1

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

[PATCH 4/5] memfd: Pass argument of memfd_fcntl as int

[Luca Vizzarro]

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. In order to avoid access to undefined bits, we should explicitly
cast the argument to int.

This commit changes the signature of all the related and helper
functions so that they treat the argument as int instead of long.

Cc: Kevin Brodsky <Kevin.Brodsky@arm.com>
Cc: Szabolcs Nagy <Szabolcs.Nagy@arm.com>
Cc: "Theodore Ts'o" <tytso@mit.edu>
Cc: David Laight <David.Laight@ACULAB.com>
Cc: Mark Rutland <Mark.Rutland@arm.com>
Cc: Alexander Viro <viro@zeniv.linux.org.uk>
Cc: Christian Brauner <brauner@kernel.org>
Cc: Jeff Layton <jlayton@kernel.org>
Cc: Chuck Lever <chuck.lever@oracle.com>
Cc: linux-fsdevel@vger.kernel.org
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: linux-mm@kvack.org
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 a0a7a37e8177..69b90c31d38c 100644
--- a/mm/memfd.c
+++ b/mm/memfd.c
@@ -243,16 +243,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:
--
2.34.1

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

[PATCH 5/5] dnotify: Pass argument of fcntl_dirnotify as int

[Luca Vizzarro]

The interface for fcntl expects the argument passed for the command
F_DIRNOTIFY to be of type int. The current code wrongly treats it as
a long. In order to avoid access to undefined bits, we should explicitly
cast the argument to int.

Cc: Kevin Brodsky <Kevin.Brodsky@arm.com>
Cc: Szabolcs Nagy <Szabolcs.Nagy@arm.com>
Cc: "Theodore Ts'o" <tytso@mit.edu>
Cc: David Laight <David.Laight@ACULAB.com>
Cc: Mark Rutland <Mark.Rutland@arm.com>
Cc: Alexander Viro <viro@zeniv.linux.org.uk>
Cc: Christian Brauner <brauner@kernel.org>
Cc: Jeff Layton <jlayton@kernel.org>
Cc: Chuck Lever <chuck.lever@oracle.com>
Cc: linux-fsdevel@vger.kernel.org
Cc: Jan Kara <jack@suse.cz>
Cc: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Luca Vizzarro <Luca.Vizzarro@arm.com>
---
 fs/notify/dnotify/dnotify.c | 4 ++--
 include/linux/dnotify.h     | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/fs/notify/dnotify/dnotify.c b/fs/notify/dnotify/dnotify.c
index 190aa717fa32..ebdcc25df0f7 100644
--- a/fs/notify/dnotify/dnotify.c
+++ b/fs/notify/dnotify/dnotify.c
@@ -199,7 +199,7 @@ void dnotify_flush(struct file *filp, fl_owner_t id)
 }

 /* this conversion is done only at watch creation */
-static __u32 convert_arg(unsigned long arg)
+static __u32 convert_arg(unsigned int arg)
 {
        __u32 new_mask = FS_EVENT_ON_CHILD;

@@ -258,7 +258,7 @@ static int attach_dn(struct dnotify_struct *dn, struct dnotify_mark *dn_mark,
  * up here.  Allocate both a mark for fsnotify to add and a dnotify_struct to be
  * attached to the fsnotify_mark.
  */
-int fcntl_dirnotify(int fd, struct file *filp, unsigned long arg)
+int fcntl_dirnotify(int fd, struct file *filp, unsigned int arg)
 {
        struct dnotify_mark *new_dn_mark, *dn_mark;
        struct fsnotify_mark *new_fsn_mark, *fsn_mark;
diff --git a/include/linux/dnotify.h b/include/linux/dnotify.h
index b1d26f9f1c9f..9f183a679277 100644
--- a/include/linux/dnotify.h
+++ b/include/linux/dnotify.h
@@ -30,7 +30,7 @@ struct dnotify_struct {
                            FS_MOVED_FROM | FS_MOVED_TO)

 extern void dnotify_flush(struct file *, fl_owner_t);
-extern int fcntl_dirnotify(int, struct file *, unsigned long);
+extern int fcntl_dirnotify(int, struct file *, unsigned int);

 #else

@@ -38,7 +38,7 @@ static inline void dnotify_flush(struct file *filp, fl_owner_t id)
 {
 }

-static inline int fcntl_dirnotify(int fd, struct file *filp, unsigned long arg)
+static inline int fcntl_dirnotify(int fd, struct file *filp, unsigned int arg)
 {
        return -EINVAL;
 }
--
2.34.1

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

Re: [PATCH 5/5] dnotify: Pass argument of fcntl_dirnotify as int

[Jan Kara]

On Fri 14-04-23 11:02:12, Luca Vizzarro wrote:
> The interface for fcntl expects the argument passed for the command
> F_DIRNOTIFY to be of type int. The current code wrongly treats it as
> a long. In order to avoid access to undefined bits, we should explicitly
> cast the argument to int.
> 
> Cc: Kevin Brodsky <Kevin.Brodsky@arm.com>
> Cc: Szabolcs Nagy <Szabolcs.Nagy@arm.com>
> Cc: "Theodore Ts'o" <tytso@mit.edu>
> Cc: David Laight <David.Laight@ACULAB.com>
> Cc: Mark Rutland <Mark.Rutland@arm.com>
> Cc: Alexander Viro <viro@zeniv.linux.org.uk>
> Cc: Christian Brauner <brauner@kernel.org>
> Cc: Jeff Layton <jlayton@kernel.org>
> Cc: Chuck Lever <chuck.lever@oracle.com>
> Cc: linux-fsdevel@vger.kernel.org
> Cc: Jan Kara <jack@suse.cz>
> Cc: Amir Goldstein <amir73il@gmail.com>
> Signed-off-by: Luca Vizzarro <Luca.Vizzarro@arm.com>

Looks good to me. Do you plan to merge this series together (perhaps
Christian could?) or should I pick up the dnotify patch? In case someone
else will merge the patch feel free to add:

Acked-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/notify/dnotify/dnotify.c | 4 ++--
>  include/linux/dnotify.h     | 4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/notify/dnotify/dnotify.c b/fs/notify/dnotify/dnotify.c
> index 190aa717fa32..ebdcc25df0f7 100644
> --- a/fs/notify/dnotify/dnotify.c
> +++ b/fs/notify/dnotify/dnotify.c
> @@ -199,7 +199,7 @@ void dnotify_flush(struct file *filp, fl_owner_t id)
>  }
> 
>  /* this conversion is done only at watch creation */
> -static __u32 convert_arg(unsigned long arg)
> +static __u32 convert_arg(unsigned int arg)
>  {
>         __u32 new_mask = FS_EVENT_ON_CHILD;
> 
> @@ -258,7 +258,7 @@ static int attach_dn(struct dnotify_struct *dn, struct dnotify_mark *dn_mark,
>   * up here.  Allocate both a mark for fsnotify to add and a dnotify_struct to be
>   * attached to the fsnotify_mark.
>   */
> -int fcntl_dirnotify(int fd, struct file *filp, unsigned long arg)
> +int fcntl_dirnotify(int fd, struct file *filp, unsigned int arg)
>  {
>         struct dnotify_mark *new_dn_mark, *dn_mark;
>         struct fsnotify_mark *new_fsn_mark, *fsn_mark;
> diff --git a/include/linux/dnotify.h b/include/linux/dnotify.h
> index b1d26f9f1c9f..9f183a679277 100644
> --- a/include/linux/dnotify.h
> +++ b/include/linux/dnotify.h
> @@ -30,7 +30,7 @@ struct dnotify_struct {
>                             FS_MOVED_FROM | FS_MOVED_TO)
> 
>  extern void dnotify_flush(struct file *, fl_owner_t);
> -extern int fcntl_dirnotify(int, struct file *, unsigned long);
> +extern int fcntl_dirnotify(int, struct file *, unsigned int);
> 
>  #else
> 
> @@ -38,7 +38,7 @@ static inline void dnotify_flush(struct file *filp, fl_owner_t id)
>  {
>  }
> 
> -static inline int fcntl_dirnotify(int fd, struct file *filp, unsigned long arg)
> +static inline int fcntl_dirnotify(int fd, struct file *filp, unsigned int arg)
>  {
>         return -EINVAL;
>  }
> --
> 2.34.1
> 
> IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 5/5] dnotify: Pass argument of fcntl_dirnotify as int

[Christian Brauner]

On Fri, Apr 14, 2023 at 12:46:25PM +0200, Jan Kara wrote:
> On Fri 14-04-23 11:02:12, Luca Vizzarro wrote:
> > The interface for fcntl expects the argument passed for the command
> > F_DIRNOTIFY to be of type int. The current code wrongly treats it as
> > a long. In order to avoid access to undefined bits, we should explicitly
> > cast the argument to int.
> > 
> > Cc: Kevin Brodsky <Kevin.Brodsky@arm.com>
> > Cc: Szabolcs Nagy <Szabolcs.Nagy@arm.com>
> > Cc: "Theodore Ts'o" <tytso@mit.edu>
> > Cc: David Laight <David.Laight@ACULAB.com>
> > Cc: Mark Rutland <Mark.Rutland@arm.com>
> > Cc: Alexander Viro <viro@zeniv.linux.org.uk>
> > Cc: Christian Brauner <brauner@kernel.org>
> > Cc: Jeff Layton <jlayton@kernel.org>
> > Cc: Chuck Lever <chuck.lever@oracle.com>
> > Cc: linux-fsdevel@vger.kernel.org
> > Cc: Jan Kara <jack@suse.cz>
> > Cc: Amir Goldstein <amir73il@gmail.com>
> > Signed-off-by: Luca Vizzarro <Luca.Vizzarro@arm.com>
> 
> Looks good to me. Do you plan to merge this series together (perhaps
> Christian could?) or should I pick up the dnotify patch? In case someone
> else will merge the patch feel free to add:
> 
> Acked-by: Jan Kara <jack@suse.cz>

Yeah, I'm happy to carry it once it's reviewed if noone objects.

Re: [PATCH 0/5] Alter fcntl to handle int arguments correctly

[Christian Brauner]

On Fri, Apr 14, 2023 at 11:02:07AM +0100, Luca Vizzarro wrote:
> According to the documentation of fcntl, some commands take an int as
> argument. In practice not all of them enforce this behaviour, as they
> instead accept a more permissive long and in most cases not even a
> range check is performed.
> 
> An issue could possibly arise from a combination of the handling of the
> varargs in user space and the ABI rules of the target, which may result
> in the top bits of an int argument being non-zero.
> 
> This issue was originally raised and detailed in the following thread:
>   https://lore.kernel.org/linux-api/Y1%2FDS6uoWP7OSkmd@arm.com/
> 
> This series modifies the interested commands so that they explicitly
> take an int argument. It also propagates this change down to helper and
> related functions as necessary.
> 
> This series is also available on my fork at:
>   https://git.morello-project.org/Sevenarth/linux/-/commits/fcntl-int-handling
> 
> Best regards,
> Luca Vizzarro
> 
> Luca Vizzarro (5):
>   fcntl: Cast commands with int args explicitly
>   fs: Pass argument to fcntl_setlease as int
>   pipe: Pass argument of pipe_fcntl as int
>   memfd: Pass argument of memfd_fcntl as int
>   dnotify: Pass argument of fcntl_dirnotify as int
> 
>  fs/cifs/cifsfs.c            |  2 +-
>  fs/fcntl.c                  | 29 +++++++++++++++--------------
>  fs/libfs.c                  |  2 +-
>  fs/locks.c                  | 20 ++++++++++----------
>  fs/nfs/nfs4_fs.h            |  2 +-
>  fs/nfs/nfs4file.c           |  2 +-
>  fs/nfs/nfs4proc.c           |  4 ++--
>  fs/notify/dnotify/dnotify.c |  4 ++--
>  fs/pipe.c                   |  6 +++---
>  include/linux/dnotify.h     |  4 ++--
>  include/linux/filelock.h    | 12 ++++++------
>  include/linux/fs.h          |  6 +++---
>  include/linux/memfd.h       |  4 ++--
>  include/linux/pipe_fs_i.h   |  4 ++--
>  mm/memfd.c                  |  6 +-----
>  15 files changed, 52 insertions(+), 55 deletions(-)
> 
> --
> 2.34.1
> 
> IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

Fyi, this blurb at the end breaks applying this series.

It means when someone pulls these patches down they get corrupted git
patches. You should fix your setup to not have this nonsense in your
mails. I tried to apply this for review to v6.2 up until v6.3-mainline
until I realized that the patches are corrupted by the blurb at the
end...

Re: [PATCH 5/5] dnotify: Pass argument of fcntl_dirnotify as int

[Luca Vizzarro]

On 14/04/2023 11:46, Jan Kara wrote:
> Looks good to me. Do you plan to merge this series together (perhaps
> Christian could?) or should I pick up the dnotify patch? In case someone
> else will merge the patch feel free to add:
> 
> Acked-by: Jan Kara <jack@suse.cz>

Hi Jan! Thank you for your review! The patches could potentially be 
merged separately, as they should work independently.

Re: [PATCH 0/5] Alter fcntl to handle int arguments correctly

[Luca Vizzarro]

On 14/04/2023 13:37, Christian Brauner wrote
> Fyi, this blurb at the end breaks applying this series.
> 
> It means when someone pulls these patches down they get corrupted git
> patches. You should fix your setup to not have this nonsense in your
> mails. I tried to apply this for review to v6.2 up until v6.3-mainline
> until I realized that the patches are corrupted by the blurb at the
> end...

Apologies! There was an error in my end with the selection of the SMTP 
server. That was definitely not meant to be there! Will make sure to 
repost correctly.