Hi all,
This patch series addresses the issue reported in the following issue: https://git.morello-project.org/morello/kernel/linux/-/issues/45
Changes available at: https://git.morello-project.org/Sevenarth/linux/-/commits/morello/6.1-ioctl-...
v3: - fixed casting of ptr from unsigned long to int - reworded commits and added Fixes tag where opportune - fixed compat ioctl handler for net - changed copy_from_user to copy_from_user_with_ptr for br_dev_read_uargs, only for non-compat - removed redundant #ifdef for compat mode - rearranged commits order v2: - discarded the posix_clock_compat_ioctl handler and replaced with the compat_ptr_ioctl helper function
Luca Vizzarro (8): ptp: Modify ptp_ioctl to use user_uintptr_t posix_clocks: Use compat_ptr_ioctl helper tty: Modify ioctl to use user_uintptr_t fs: Fix user_uinptr_t behaviour with ioctl net: Modify ioctl to use user_uintptr_t net: Fix compat ioctl handler net: bridge: Modify br_dev_read_uargs to accept user_uintptr_t net: bridge: Support 64-bit pointers in compat
drivers/ptp/ptp_chardev.c | 2 +- drivers/ptp/ptp_private.h | 2 +- drivers/tty/n_tty.c | 2 +- drivers/tty/pty.c | 4 ++-- drivers/tty/serial/serial_core.c | 2 +- drivers/tty/tty_ioctl.c | 2 +- drivers/tty/vt/keyboard.c | 2 +- drivers/tty/vt/vt.c | 2 +- drivers/tty/vt/vt_ioctl.c | 8 ++++---- fs/autofs/root.c | 2 +- fs/ext4/ioctl.c | 4 ++-- include/linux/net.h | 2 +- include/linux/posix-clock.h | 2 +- include/linux/tty.h | 4 ++-- include/linux/tty_driver.h | 2 +- include/linux/tty_ldisc.h | 2 +- include/linux/vt_kern.h | 4 ++-- include/net/inet_common.h | 2 +- include/net/ipv6.h | 2 +- include/net/sock.h | 2 +- include/net/tcp.h | 2 +- include/net/udp.h | 2 +- kernel/time/posix-clock.c | 23 +---------------------- net/bridge/br_ioctl.c | 14 +++++++------- net/ipv4/af_inet.c | 2 +- net/ipv4/raw.c | 2 +- net/ipv4/tcp.c | 2 +- net/ipv4/udp.c | 2 +- net/ipv6/af_inet6.c | 2 +- net/ipv6/raw.c | 2 +- net/netlink/af_netlink.c | 2 +- net/packet/af_packet.c | 2 +- net/socket.c | 6 +++--- net/unix/af_unix.c | 4 ++-- 34 files changed, 50 insertions(+), 71 deletions(-)
The third argument of ioctl handlers is a user input which can contain either a pointer to data in the user space or any number. This commit changes the type of the argument from unsigned long to user_uintptr_t so that we can accommodate the bigger size of capabilities and accept them from user space correctly without discarding their metadata.
Signed-off-by: Luca Vizzarro Luca.Vizzarro@arm.com --- drivers/ptp/ptp_chardev.c | 2 +- drivers/ptp/ptp_private.h | 2 +- include/linux/posix-clock.h | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/ptp/ptp_chardev.c b/drivers/ptp/ptp_chardev.c index af3bc65c4595..397e4ba71d90 100644 --- a/drivers/ptp/ptp_chardev.c +++ b/drivers/ptp/ptp_chardev.c @@ -106,7 +106,7 @@ int ptp_open(struct posix_clock *pc, fmode_t fmode) return 0; }
-long ptp_ioctl(struct posix_clock *pc, unsigned int cmd, unsigned long arg) +long ptp_ioctl(struct posix_clock *pc, unsigned int cmd, user_uintptr_t arg) { struct ptp_clock *ptp = container_of(pc, struct ptp_clock, clock); struct ptp_sys_offset_extended *extoff = NULL; diff --git a/drivers/ptp/ptp_private.h b/drivers/ptp/ptp_private.h index 77918a2c6701..3bd83bc6e209 100644 --- a/drivers/ptp/ptp_private.h +++ b/drivers/ptp/ptp_private.h @@ -118,7 +118,7 @@ int ptp_set_pinfunc(struct ptp_clock *ptp, unsigned int pin, enum ptp_pin_function func, unsigned int chan);
long ptp_ioctl(struct posix_clock *pc, - unsigned int cmd, unsigned long arg); + unsigned int cmd, user_uintptr_t arg);
int ptp_open(struct posix_clock *pc, fmode_t fmode);
diff --git a/include/linux/posix-clock.h b/include/linux/posix-clock.h index 468328b1e1dd..184aaa05d0ff 100644 --- a/include/linux/posix-clock.h +++ b/include/linux/posix-clock.h @@ -51,7 +51,7 @@ struct posix_clock_operations { * Optional character device methods: */ long (*ioctl) (struct posix_clock *pc, - unsigned int cmd, unsigned long arg); + unsigned int cmd, user_uintptr_t arg);
int (*open) (struct posix_clock *pc, fmode_t f_mode);
The posix_clock_ioctl handler mostly takes a user pointer as an argument, and the compat implementation is not PCuABI compatible.
This commit removes the posix_clock_compat_ioctl handler and replaces it with the compat_ptr_ioctl helper function instead.
Signed-off-by: Luca Vizzarro Luca.Vizzarro@arm.com --- kernel/time/posix-clock.c | 23 +---------------------- 1 file changed, 1 insertion(+), 22 deletions(-)
diff --git a/kernel/time/posix-clock.c b/kernel/time/posix-clock.c index 8ecfea067832..4e913d8cbe94 100644 --- a/kernel/time/posix-clock.c +++ b/kernel/time/posix-clock.c @@ -86,25 +86,6 @@ static long posix_clock_ioctl(struct file *fp, return err; }
-#ifdef CONFIG_COMPAT -static long posix_clock_compat_ioctl(struct file *fp, - unsigned int cmd, unsigned long arg) -{ - struct posix_clock *clk = get_posix_clock(fp); - int err = -ENOTTY; - - if (!clk) - return -ENODEV; - - if (clk->ops.ioctl) - err = clk->ops.ioctl(clk, cmd, arg); - - put_posix_clock(clk); - - return err; -} -#endif - static int posix_clock_open(struct inode *inode, struct file *fp) { int err; @@ -154,9 +135,7 @@ static const struct file_operations posix_clock_file_operations = { .unlocked_ioctl = posix_clock_ioctl, .open = posix_clock_open, .release = posix_clock_release, -#ifdef CONFIG_COMPAT - .compat_ioctl = posix_clock_compat_ioctl, -#endif + .compat_ioctl = compat_ptr_ioctl, };
int posix_clock_register(struct posix_clock *clk, struct device *dev)
The third argument of ioctl handlers is a user input which can contain either a pointer to data in the user space or any number. This commit changes the type of the argument from unsigned long to user_uintptr_t so that we can accommodate the bigger size of capabilities and accept them from user space correctly without discarding their metadata. It also updates the signature of child functions and casts the argument to unsigned long when it must be treated as such for size reasons.
Signed-off-by: Luca Vizzarro Luca.Vizzarro@arm.com --- drivers/tty/n_tty.c | 2 +- drivers/tty/pty.c | 4 ++-- drivers/tty/serial/serial_core.c | 2 +- drivers/tty/tty_ioctl.c | 2 +- drivers/tty/vt/keyboard.c | 2 +- drivers/tty/vt/vt.c | 2 +- drivers/tty/vt/vt_ioctl.c | 8 ++++---- include/linux/tty.h | 4 ++-- include/linux/tty_driver.h | 2 +- include/linux/tty_ldisc.h | 2 +- include/linux/vt_kern.h | 4 ++-- 11 files changed, 17 insertions(+), 17 deletions(-)
diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c index 597019690ae6..fd58d19c1dc4 100644 --- a/drivers/tty/n_tty.c +++ b/drivers/tty/n_tty.c @@ -2463,7 +2463,7 @@ static unsigned long inq_canon(struct n_tty_data *ldata) }
static int n_tty_ioctl(struct tty_struct *tty, unsigned int cmd, - unsigned long arg) + user_uintptr_t arg) { struct n_tty_data *ldata = tty->disc_data; int retval; diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c index aa6c5fa3d562..255f91b0d515 100644 --- a/drivers/tty/pty.c +++ b/drivers/tty/pty.c @@ -451,7 +451,7 @@ static void pty_remove(struct tty_driver *driver, struct tty_struct *tty) }
static int pty_bsd_ioctl(struct tty_struct *tty, - unsigned int cmd, unsigned long arg) + unsigned int cmd, user_uintptr_t arg) { switch (cmd) { case TIOCSPTLCK: /* Set PT Lock (disallow slave open) */ @@ -640,7 +640,7 @@ int ptm_open_peer(struct file *master, struct tty_struct *tty, int flags) }
static int pty_unix98_ioctl(struct tty_struct *tty, - unsigned int cmd, unsigned long arg) + unsigned int cmd, user_uintptr_t arg) { switch (cmd) { case TIOCSPTLCK: /* Set PT Lock (disallow slave open) */ diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c index 6b86149cd879..122c768893fd 100644 --- a/drivers/tty/serial/serial_core.c +++ b/drivers/tty/serial/serial_core.c @@ -1511,7 +1511,7 @@ static int uart_set_iso7816_config(struct uart_port *port, * Called via sys_ioctl. We can use spin_lock_irq() here. */ static int -uart_ioctl(struct tty_struct *tty, unsigned int cmd, unsigned long arg) +uart_ioctl(struct tty_struct *tty, unsigned int cmd, user_uintptr_t arg) { struct uart_state *state = tty->driver_data; struct tty_port *port = &state->port; diff --git a/drivers/tty/tty_ioctl.c b/drivers/tty/tty_ioctl.c index ce511557b98b..45a5538cdc4a 100644 --- a/drivers/tty/tty_ioctl.c +++ b/drivers/tty/tty_ioctl.c @@ -760,7 +760,7 @@ static int tty_change_softcar(struct tty_struct *tty, int arg) * consistent mode setting. */
-int tty_mode_ioctl(struct tty_struct *tty, unsigned int cmd, unsigned long arg) +int tty_mode_ioctl(struct tty_struct *tty, unsigned int cmd, user_uintptr_t arg) { struct tty_struct *real_tty; void __user *p = (void __user *)arg; diff --git a/drivers/tty/vt/keyboard.c b/drivers/tty/vt/keyboard.c index be8313cdbac3..e001fc67151c 100644 --- a/drivers/tty/vt/keyboard.c +++ b/drivers/tty/vt/keyboard.c @@ -2109,7 +2109,7 @@ int vt_do_kdgkb_ioctl(int cmd, struct kbsentry __user *user_kdgkb, int perm) return ret; }
-int vt_do_kdskled(unsigned int console, int cmd, unsigned long arg, int perm) +int vt_do_kdskled(unsigned int console, int cmd, user_uintptr_t arg, int perm) { struct kbd_struct *kb = &kbd_table[console]; unsigned long flags; diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c index 981d2bfcf9a5..8f7bc37fab54 100644 --- a/drivers/tty/vt/vt.c +++ b/drivers/tty/vt/vt.c @@ -3181,7 +3181,7 @@ static struct console vt_console_driver = { * set_selection_user has locking, and definitely needs it */
-int tioclinux(struct tty_struct *tty, unsigned long arg) +int tioclinux(struct tty_struct *tty, user_uintptr_t arg) { char type, data; char __user *p = (char __user *)arg; diff --git a/drivers/tty/vt/vt_ioctl.c b/drivers/tty/vt/vt_ioctl.c index e120bd655b0e..1106077f1b0c 100644 --- a/drivers/tty/vt/vt_ioctl.c +++ b/drivers/tty/vt/vt_ioctl.c @@ -281,7 +281,7 @@ static int vt_kdsetmode(struct vc_data *vc, unsigned long mode) }
static int vt_k_ioctl(struct tty_struct *tty, unsigned int cmd, - unsigned long arg, bool perm) + user_uintptr_t arg, bool perm) { struct vc_data *vc = tty->driver_data; void __user *up = (void __user *)arg; @@ -734,7 +734,7 @@ static int vt_resizex(struct vc_data *vc, struct vt_consize __user *cs) * capability to modify any console, not just the fg_console. */ int vt_ioctl(struct tty_struct *tty, - unsigned int cmd, unsigned long arg) + unsigned int cmd, user_uintptr_t arg) { struct vc_data *vc = tty->driver_data; void __user *up = (void __user *)arg; @@ -845,7 +845,7 @@ int vt_ioctl(struct tty_struct *tty, return -ENXIO;
arg--; - arg = array_index_nospec(arg, MAX_NR_CONSOLES); + arg = array_index_nospec((unsigned long)arg, MAX_NR_CONSOLES); console_lock(); ret = vc_allocate(arg); console_unlock(); @@ -903,7 +903,7 @@ int vt_ioctl(struct tty_struct *tty, break; }
- arg = array_index_nospec(arg - 1, MAX_NR_CONSOLES); + arg = array_index_nospec((unsigned long)arg - 1, MAX_NR_CONSOLES); return vt_disallocate(arg);
case VT_RESIZE: diff --git a/include/linux/tty.h b/include/linux/tty.h index 730c3301d710..dd344a6f1bd0 100644 --- a/include/linux/tty.h +++ b/include/linux/tty.h @@ -458,7 +458,7 @@ int tty_set_termios(struct tty_struct *tty, struct ktermios *kt);
void tty_wakeup(struct tty_struct *tty);
-int tty_mode_ioctl(struct tty_struct *tty, unsigned int cmd, unsigned long arg); +int tty_mode_ioctl(struct tty_struct *tty, unsigned int cmd, user_uintptr_t arg); int tty_perform_flush(struct tty_struct *tty, unsigned long arg); struct tty_struct *tty_init_dev(struct tty_driver *driver, int idx); void tty_release_struct(struct tty_struct *tty, int idx); @@ -501,7 +501,7 @@ int n_tty_ioctl_helper(struct tty_struct *tty, unsigned int cmd,
/* vt.c */
-int vt_ioctl(struct tty_struct *tty, unsigned int cmd, unsigned long arg); +int vt_ioctl(struct tty_struct *tty, unsigned int cmd, user_uintptr_t arg);
long vt_compat_ioctl(struct tty_struct *tty, unsigned int cmd, unsigned long arg); diff --git a/include/linux/tty_driver.h b/include/linux/tty_driver.h index e00034118c7b..8301629c5c5d 100644 --- a/include/linux/tty_driver.h +++ b/include/linux/tty_driver.h @@ -363,7 +363,7 @@ struct tty_operations { unsigned int (*write_room)(struct tty_struct *tty); unsigned int (*chars_in_buffer)(struct tty_struct *tty); int (*ioctl)(struct tty_struct *tty, - unsigned int cmd, unsigned long arg); + unsigned int cmd, user_uintptr_t arg); long (*compat_ioctl)(struct tty_struct *tty, unsigned int cmd, unsigned long arg); void (*set_termios)(struct tty_struct *tty, const struct ktermios *old); diff --git a/include/linux/tty_ldisc.h b/include/linux/tty_ldisc.h index dcb61ec11424..b8c340a12bff 100644 --- a/include/linux/tty_ldisc.h +++ b/include/linux/tty_ldisc.h @@ -224,7 +224,7 @@ struct tty_ldisc_ops { ssize_t (*write)(struct tty_struct *tty, struct file *file, const unsigned char *buf, size_t nr); int (*ioctl)(struct tty_struct *tty, unsigned int cmd, - unsigned long arg); + user_uintptr_t arg); int (*compat_ioctl)(struct tty_struct *tty, unsigned int cmd, unsigned long arg); void (*set_termios)(struct tty_struct *tty, const struct ktermios *old); diff --git a/include/linux/vt_kern.h b/include/linux/vt_kern.h index c1f5aebef170..5cca5e106313 100644 --- a/include/linux/vt_kern.h +++ b/include/linux/vt_kern.h @@ -43,7 +43,7 @@ void redraw_screen(struct vc_data *vc, int is_switch); #define switch_screen(x) redraw_screen(x, 1)
struct tty_struct; -int tioclinux(struct tty_struct *tty, unsigned long arg); +int tioclinux(struct tty_struct *tty, user_uintptr_t arg);
#ifdef CONFIG_CONSOLE_TRANSLATIONS /* consolemap.c */ @@ -154,7 +154,7 @@ int vt_do_kbkeycode_ioctl(int cmd, struct kbkeycode __user *user_kbkc, int vt_do_kdsk_ioctl(int cmd, struct kbentry __user *user_kbe, int perm, unsigned int console); int vt_do_kdgkb_ioctl(int cmd, struct kbsentry __user *user_kdgkb, int perm); -int vt_do_kdskled(unsigned int console, int cmd, unsigned long arg, int perm); +int vt_do_kdskled(unsigned int console, int cmd, user_uintptr_t arg, int perm); int vt_do_kdgkbmode(unsigned int console); int vt_do_kdgkbmeta(unsigned int console); void vt_reset_unicode(unsigned int console);
The third argument of ioctl handlers is a user input which can contain either a pointer to data in the user space or any number. This commit updates the signature of ext4_ioctl_checkpoint, which is a child handler of __ext4_ioctl. __ext4_ioctl was updated to correctly accept and supply a user_uintptr_t but the signature of ext4_ioctl_checkpoint was not updated to reflect this change. Moreover the value of the argument is casted to unsigned long when outputted to the debug log to match the expected type.
Signed-off-by: Luca Vizzarro Luca.Vizzarro@arm.com --- fs/autofs/root.c | 2 +- fs/ext4/ioctl.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/fs/autofs/root.c b/fs/autofs/root.c index 554f8fb3947e..1f70683dc362 100644 --- a/fs/autofs/root.c +++ b/fs/autofs/root.c @@ -858,7 +858,7 @@ static int autofs_root_ioctl_unlocked(struct inode *inode, struct file *filp, void __user *p = (void __user *)arg;
pr_debug("cmd = 0x%08x, arg = 0x%08lx, sbi = %p, pgrp = %u\n", - cmd, arg, sbi, task_pgrp_nr(current)); + cmd, (unsigned long)arg, sbi, task_pgrp_nr(current));
if (_IOC_TYPE(cmd) != _IOC_TYPE(AUTOFS_IOC_FIRST) || _IOC_NR(cmd) - _IOC_NR(AUTOFS_IOC_FIRST) >= AUTOFS_IOC_COUNT) diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c index 60437a1572ca..0d3ed9812e2b 100644 --- a/fs/ext4/ioctl.c +++ b/fs/ext4/ioctl.c @@ -1039,7 +1039,7 @@ static int ext4_ioctl_get_es_cache(struct file *filp, user_uintptr_t arg) return error; }
-static int ext4_ioctl_checkpoint(struct file *filp, unsigned long arg) +static int ext4_ioctl_checkpoint(struct file *filp, user_uintptr_t arg) { int err = 0; __u32 flags = 0; @@ -1215,7 +1215,7 @@ static long __ext4_ioctl(struct file *filp, unsigned int cmd, user_uintptr_t arg struct super_block *sb = inode->i_sb; struct user_namespace *mnt_userns = file_mnt_user_ns(filp);
- ext4_debug("cmd = %u, arg = %lu\n", cmd, arg); + ext4_debug("cmd = %u, arg = %lu\n", cmd, (unsigned long)arg);
switch (cmd) { case FS_IOC_GETFSMAP:
The third argument of ioctl handlers is a user input which can contain either a pointer to data in the user space or any number. This commit changes the type of the argument from unsigned long to user_uintptr_t so that we can accommodate the bigger size of capabilities and accept them from user space correctly without discarding their metadata.
Signed-off-by: Luca Vizzarro Luca.Vizzarro@arm.com --- include/linux/net.h | 2 +- include/net/inet_common.h | 2 +- include/net/ipv6.h | 2 +- include/net/sock.h | 2 +- include/net/tcp.h | 2 +- include/net/udp.h | 2 +- net/ipv4/af_inet.c | 2 +- net/ipv4/raw.c | 2 +- net/ipv4/tcp.c | 2 +- net/ipv4/udp.c | 2 +- net/ipv6/af_inet6.c | 2 +- net/ipv6/raw.c | 2 +- net/netlink/af_netlink.c | 2 +- net/packet/af_packet.c | 2 +- net/socket.c | 2 +- net/unix/af_unix.c | 4 ++-- 16 files changed, 17 insertions(+), 17 deletions(-)
diff --git a/include/linux/net.h b/include/linux/net.h index 18d942bbdf6e..43ee08cc57a7 100644 --- a/include/linux/net.h +++ b/include/linux/net.h @@ -176,7 +176,7 @@ struct proto_ops { __poll_t (*poll) (struct file *file, struct socket *sock, struct poll_table_struct *wait); int (*ioctl) (struct socket *sock, unsigned int cmd, - unsigned long arg); + user_uintptr_t arg); #ifdef CONFIG_COMPAT int (*compat_ioctl) (struct socket *sock, unsigned int cmd, unsigned long arg); diff --git a/include/net/inet_common.h b/include/net/inet_common.h index cec453c18f1d..e17bfdb8e5d2 100644 --- a/include/net/inet_common.h +++ b/include/net/inet_common.h @@ -53,7 +53,7 @@ int __inet_bind(struct sock *sk, struct sockaddr *uaddr, int addr_len, u32 flags); int inet_getname(struct socket *sock, struct sockaddr *uaddr, int peer); -int inet_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg); +int inet_ioctl(struct socket *sock, unsigned int cmd, user_uintptr_t arg); int inet_ctl_sock_create(struct sock **sk, unsigned short family, unsigned short type, unsigned char protocol, struct net *net); diff --git a/include/net/ipv6.h b/include/net/ipv6.h index d383c895592a..6d301a05d66d 100644 --- a/include/net/ipv6.h +++ b/include/net/ipv6.h @@ -1188,7 +1188,7 @@ int inet6_release(struct socket *sock); int inet6_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len); int inet6_getname(struct socket *sock, struct sockaddr *uaddr, int peer); -int inet6_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg); +int inet6_ioctl(struct socket *sock, unsigned int cmd, user_uintptr_t arg); int inet6_compat_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg);
diff --git a/include/net/sock.h b/include/net/sock.h index e0517ecc6531..153054819061 100644 --- a/include/net/sock.h +++ b/include/net/sock.h @@ -1242,7 +1242,7 @@ struct proto { bool kern);
int (*ioctl)(struct sock *sk, int cmd, - unsigned long arg); + user_uintptr_t arg); int (*init)(struct sock *sk); void (*destroy)(struct sock *sk); void (*shutdown)(struct sock *sk, int how); diff --git a/include/net/tcp.h b/include/net/tcp.h index 14d45661a84d..6c3498ce906b 100644 --- a/include/net/tcp.h +++ b/include/net/tcp.h @@ -342,7 +342,7 @@ void tcp_release_cb(struct sock *sk); void tcp_wfree(struct sk_buff *skb); void tcp_write_timer_handler(struct sock *sk); void tcp_delack_timer_handler(struct sock *sk); -int tcp_ioctl(struct sock *sk, int cmd, unsigned long arg); +int tcp_ioctl(struct sock *sk, int cmd, user_uintptr_t arg); int tcp_rcv_state_process(struct sock *sk, struct sk_buff *skb); void tcp_rcv_established(struct sock *sk, struct sk_buff *skb); void tcp_rcv_space_adjust(struct sock *sk); diff --git a/include/net/udp.h b/include/net/udp.h index fee053bcd17c..023a11ce421f 100644 --- a/include/net/udp.h +++ b/include/net/udp.h @@ -274,7 +274,7 @@ void udp_flush_pending_frames(struct sock *sk); int udp_cmsg_send(struct sock *sk, struct msghdr *msg, u16 *gso_size); void udp4_hwcsum(struct sk_buff *skb, __be32 src, __be32 dst); int udp_rcv(struct sk_buff *skb); -int udp_ioctl(struct sock *sk, int cmd, unsigned long arg); +int udp_ioctl(struct sock *sk, int cmd, user_uintptr_t arg); int udp_init_sock(struct sock *sk); int udp_pre_connect(struct sock *sk, struct sockaddr *uaddr, int addr_len); int __udp_disconnect(struct sock *sk, int flags); diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c index 0da679411330..3968d555f496 100644 --- a/net/ipv4/af_inet.c +++ b/net/ipv4/af_inet.c @@ -932,7 +932,7 @@ EXPORT_SYMBOL(inet_shutdown); * There's a good 20K of config code hanging around the kernel. */
-int inet_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg) +int inet_ioctl(struct socket *sock, unsigned int cmd, user_uintptr_t arg) { struct sock *sk = sock->sk; int err = 0; diff --git a/net/ipv4/raw.c b/net/ipv4/raw.c index 006c1f0ed8b4..86072fc11ffe 100644 --- a/net/ipv4/raw.c +++ b/net/ipv4/raw.c @@ -854,7 +854,7 @@ static int raw_getsockopt(struct sock *sk, int level, int optname, return do_raw_getsockopt(sk, level, optname, optval, optlen); }
-static int raw_ioctl(struct sock *sk, int cmd, unsigned long arg) +static int raw_ioctl(struct sock *sk, int cmd, user_uintptr_t arg) { switch (cmd) { case SIOCOUTQ: { diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c index 4f2205756cfe..17b213acffc8 100644 --- a/net/ipv4/tcp.c +++ b/net/ipv4/tcp.c @@ -595,7 +595,7 @@ __poll_t tcp_poll(struct file *file, struct socket *sock, poll_table *wait) } EXPORT_SYMBOL(tcp_poll);
-int tcp_ioctl(struct sock *sk, int cmd, unsigned long arg) +int tcp_ioctl(struct sock *sk, int cmd, user_uintptr_t arg) { struct tcp_sock *tp = tcp_sk(sk); int answ; diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c index 6a320a614e54..92d2da292071 100644 --- a/net/ipv4/udp.c +++ b/net/ipv4/udp.c @@ -1708,7 +1708,7 @@ static int first_packet_length(struct sock *sk) * IOCTL requests applicable to the UDP protocol */
-int udp_ioctl(struct sock *sk, int cmd, unsigned long arg) +int udp_ioctl(struct sock *sk, int cmd, user_uintptr_t arg) { switch (cmd) { case SIOCOUTQ: diff --git a/net/ipv6/af_inet6.c b/net/ipv6/af_inet6.c index 024191004982..2a429bb3f84a 100644 --- a/net/ipv6/af_inet6.c +++ b/net/ipv6/af_inet6.c @@ -566,7 +566,7 @@ int inet6_getname(struct socket *sock, struct sockaddr *uaddr, } EXPORT_SYMBOL(inet6_getname);
-int inet6_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg) +int inet6_ioctl(struct socket *sock, unsigned int cmd, user_uintptr_t arg) { void __user *argp = (void __user *)arg; struct sock *sk = sock->sk; diff --git a/net/ipv6/raw.c b/net/ipv6/raw.c index 722de9dd0ff7..80fba26a0f8d 100644 --- a/net/ipv6/raw.c +++ b/net/ipv6/raw.c @@ -1114,7 +1114,7 @@ static int rawv6_getsockopt(struct sock *sk, int level, int optname, return do_rawv6_getsockopt(sk, level, optname, optval, optlen); }
-static int rawv6_ioctl(struct sock *sk, int cmd, unsigned long arg) +static int rawv6_ioctl(struct sock *sk, int cmd, user_uintptr_t arg) { switch (cmd) { case SIOCOUTQ: { diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c index a662e8a5ff84..46ca0da083bc 100644 --- a/net/netlink/af_netlink.c +++ b/net/netlink/af_netlink.c @@ -1139,7 +1139,7 @@ static int netlink_getname(struct socket *sock, struct sockaddr *addr, }
static int netlink_ioctl(struct socket *sock, unsigned int cmd, - unsigned long arg) + user_uintptr_t arg) { /* try to hand this ioctl down to the NIC drivers. */ diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c index 1ab65f7f2a0a..a7e2a3a145b3 100644 --- a/net/packet/af_packet.c +++ b/net/packet/af_packet.c @@ -4188,7 +4188,7 @@ static int packet_notifier(struct notifier_block *this,
static int packet_ioctl(struct socket *sock, unsigned int cmd, - unsigned long arg) + user_uintptr_t arg) { struct sock *sk = sock->sk;
diff --git a/net/socket.c b/net/socket.c index 7279bb1a4c28..1238bd1560ce 100644 --- a/net/socket.c +++ b/net/socket.c @@ -1158,7 +1158,7 @@ void vlan_ioctl_set(int (*hook) (struct net *, void __user *)) EXPORT_SYMBOL(vlan_ioctl_set);
static long sock_do_ioctl(struct net *net, struct socket *sock, - unsigned int cmd, unsigned long arg) + unsigned int cmd, user_uintptr_t arg) { struct ifreq ifr; bool need_copyout; diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c index ac90bba7cbb1..5b4671f503a8 100644 --- a/net/unix/af_unix.c +++ b/net/unix/af_unix.c @@ -750,7 +750,7 @@ static int unix_getname(struct socket *, struct sockaddr *, int); static __poll_t unix_poll(struct file *, struct socket *, poll_table *); static __poll_t unix_dgram_poll(struct file *, struct socket *, poll_table *); -static int unix_ioctl(struct socket *, unsigned int, unsigned long); +static int unix_ioctl(struct socket *, unsigned int, user_uintptr_t); #ifdef CONFIG_COMPAT static int unix_compat_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg); #endif @@ -3094,7 +3094,7 @@ static int unix_open_file(struct sock *sk) return fd; }
-static int unix_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg) +static int unix_ioctl(struct socket *sock, unsigned int cmd, user_uintptr_t arg) { struct sock *sk = sock->sk; long amount = 0;
The compat_sock_ioctl_trans handler currently passes the argument from a compat syscall directly to the generic ioctl handler without changing it. With PCuABI the ioctl handler can either take a capability or a real number value. Given that the compat handler takes a pointer or a number, this needs to be converted correctly when passed along.
This commits introduces changes that pass the argument as a capability where required.
Signed-off-by: Luca Vizzarro Luca.Vizzarro@arm.com --- net/socket.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/socket.c b/net/socket.c index 1238bd1560ce..2bc683259a49 100644 --- a/net/socket.c +++ b/net/socket.c @@ -3363,7 +3363,7 @@ static int compat_sock_ioctl_trans(struct file *file, struct socket *sock, case SIOCGIFCONF: case SIOCSIFBR: case SIOCGIFBR: - return sock_ioctl(file, cmd, arg); + return sock_ioctl(file, cmd, argp);
case SIOCGIFFLAGS: case SIOCSIFFLAGS: @@ -3411,7 +3411,7 @@ static int compat_sock_ioctl_trans(struct file *file, struct socket *sock, case SIOCOUTQ: case SIOCOUTQNSD: case SIOCATMARK: - return sock_do_ioctl(net, sock, cmd, arg); + return sock_do_ioctl(net, sock, cmd, argp); }
return -ENOIOCTLCMD;
I have just realised that this is raising warnings for an implicit cast from void __user * to user_uintptr_t. Will fix it in the next patch series. Apologies!
Luca
On 07/02/2023 14:22, Luca Vizzarro wrote:
The compat_sock_ioctl_trans handler currently passes the argument from a compat syscall directly to the generic ioctl handler without changing it. With PCuABI the ioctl handler can either take a capability or a real number value. Given that the compat handler takes a pointer or a number, this needs to be converted correctly when passed along.
This commits introduces changes that pass the argument as a capability where required.
Signed-off-by: Luca Vizzarro Luca.Vizzarro@arm.com
net/socket.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/socket.c b/net/socket.c index 1238bd1560ce..2bc683259a49 100644 --- a/net/socket.c +++ b/net/socket.c @@ -3363,7 +3363,7 @@ static int compat_sock_ioctl_trans(struct file *file, struct socket *sock, case SIOCGIFCONF: case SIOCSIFBR: case SIOCGIFBR:
return sock_ioctl(file, cmd, arg);
return sock_ioctl(file, cmd, argp);case SIOCGIFFLAGS: case SIOCSIFFLAGS: @@ -3411,7 +3411,7 @@ static int compat_sock_ioctl_trans(struct file *file, struct socket *sock, case SIOCOUTQ: case SIOCOUTQNSD: case SIOCATMARK:
return sock_do_ioctl(net, sock, cmd, arg);
return sock_do_ioctl(net, sock, cmd, argp);return -ENOIOCTLCMD;
The compat_sock_ioctl_trans handler currently passes the argument from a compat syscall directly to the generic ioctl handler without changing it. With PCuABI the ioctl handler can either take a capability or a real number value. Given that the compat handler takes a pointer or a number, this needs to be converted correctly when passed along.
This commits introduces changes that pass the argument as a capability where required.
Signed-off-by: Luca Vizzarro Luca.Vizzarro@arm.com --- net/socket.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/socket.c b/net/socket.c index 1238bd1560ce..741086ceff95 100644 --- a/net/socket.c +++ b/net/socket.c @@ -3363,7 +3363,7 @@ static int compat_sock_ioctl_trans(struct file *file, struct socket *sock, case SIOCGIFCONF: case SIOCSIFBR: case SIOCGIFBR: - return sock_ioctl(file, cmd, arg); + return sock_ioctl(file, cmd, (user_uintptr_t)argp);
case SIOCGIFFLAGS: case SIOCSIFFLAGS: @@ -3411,7 +3411,7 @@ static int compat_sock_ioctl_trans(struct file *file, struct socket *sock, case SIOCOUTQ: case SIOCOUTQNSD: case SIOCATMARK: - return sock_do_ioctl(net, sock, cmd, arg); + return sock_do_ioctl(net, sock, cmd, (user_uintptr_t)argp); }
return -ENOIOCTLCMD;
Nit: the tag could be a bit more specific since it's only touching socket.c, "net: socket:" should do it.
Kevin
On 07/02/2023 16:12, Luca Vizzarro wrote:
The compat_sock_ioctl_trans handler currently passes the argument from a compat syscall directly to the generic ioctl handler without changing it. With PCuABI the ioctl handler can either take a capability or a real number value. Given that the compat handler takes a pointer or a number, this needs to be converted correctly when passed along.
This commits introduces changes that pass the argument as a capability where required.
Signed-off-by: Luca Vizzarro Luca.Vizzarro@arm.com
net/socket.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/socket.c b/net/socket.c index 1238bd1560ce..741086ceff95 100644 --- a/net/socket.c +++ b/net/socket.c @@ -3363,7 +3363,7 @@ static int compat_sock_ioctl_trans(struct file *file, struct socket *sock, case SIOCGIFCONF: case SIOCSIFBR: case SIOCGIFBR:
return sock_ioctl(file, cmd, arg);
return sock_ioctl(file, cmd, (user_uintptr_t)argp);case SIOCGIFFLAGS: case SIOCSIFFLAGS: @@ -3411,7 +3411,7 @@ static int compat_sock_ioctl_trans(struct file *file, struct socket *sock, case SIOCOUTQ: case SIOCOUTQNSD: case SIOCATMARK:
return sock_do_ioctl(net, sock, cmd, arg);
return sock_do_ioctl(net, sock, cmd, (user_uintptr_t)argp);return -ENOIOCTLCMD;
The function br_dev_read_uargs copies a list of arguments from userspace. Normally, they may contain pointers or real numbers. When working with capabilities, the type unsigned long used for this purpose is not big enough to hold them. This commit changes the type of args from unsigned long to user_uintptr_t for this purpose. As a consequence the type of the variables fed to this function is also updated, and casted to unsigned long when treated as a number. It also fixes the type of argp, as it points to an entry of args, which lives in the kernel space, and not in the user space as the type suggests.
Fixes: 337ba1b1cdd8 ("net: Fix the position of __capability in double pointer") Signed-off-by: Luca Vizzarro Luca.Vizzarro@arm.com --- net/bridge/br_ioctl.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/net/bridge/br_ioctl.c b/net/bridge/br_ioctl.c index 9c3030de438c..650b1cb2910d 100644 --- a/net/bridge/br_ioctl.c +++ b/net/bridge/br_ioctl.c @@ -104,9 +104,9 @@ static int add_del_if(struct net_bridge *br, int ifindex, int isadd) }
#define BR_UARGS_MAX 4 -static int br_dev_read_uargs(unsigned long *args, size_t nr_args, +static int br_dev_read_uargs(user_uintptr_t *args, size_t nr_args, #ifdef CONFIG_CHERI_PURECAP_UABI - void * __capability * __capability argp, void __user *data) + void * __capability *argp, void __user *data) #else void __user **argp, void __user *data) #endif @@ -129,7 +129,7 @@ static int br_dev_read_uargs(unsigned long *args, size_t nr_args,
*argp = compat_ptr(args[1]); } else { - ret = copy_from_user(args, data, nr_args * sizeof(*args)); + ret = copy_from_user_with_ptr(args, data, nr_args * sizeof(*args)); if (ret) goto fault; *argp = (void __user *)args[1]; @@ -150,7 +150,7 @@ int br_dev_siocdevprivate(struct net_device *dev, struct ifreq *rq, { struct net_bridge *br = netdev_priv(dev); struct net_bridge_port *p = NULL; - unsigned long args[4]; + user_uintptr_t args[4]; void __user *argp; int ret;
@@ -342,7 +342,7 @@ int br_dev_siocdevprivate(struct net_device *dev, struct ifreq *rq,
static int old_deviceless(struct net *net, void __user *data) { - unsigned long args[3]; + user_uintptr_t args[3]; void __user *argp; int ret;
@@ -369,7 +369,7 @@ static int old_deviceless(struct net *net, void __user *data)
ret = copy_to_user(argp, indices, array_size(args[2], sizeof(int))) - ? -EFAULT : args[2]; + ? -EFAULT : (int)args[2];
kfree(indices); return ret;
On 07/02/2023 15:22, Luca Vizzarro wrote:
|Fixes: 337ba1b1cdd8 ("net: Fix the position of __capability in double pointer")|
Nit: because we rebase regularly, we avoid referring to the hashes of of our own commits, since they change at every rebase. Here we keep the format unchanged, just without the hash. You couldn't guess that, that's really our own convention :)
Both this and the nit on patch 6 are very minor and I'm happy with the series otherwise, so I can tweak those two things myself when merging, if you're happy with that.
Kevin
The br_dev_read_uargs function copies a list of arguments from a user capability. When operating in compat64, pointers are 64-bit long and cannot be represented as unsigned int. In this scenario the function copies a list of compat pointers, therefore the type of the receiver variable, cargs, must be updated to compat_uptr_t from unsigned int.
Signed-off-by: Luca Vizzarro Luca.Vizzarro@arm.com --- net/bridge/br_ioctl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/bridge/br_ioctl.c b/net/bridge/br_ioctl.c index 650b1cb2910d..3dca929e3a24 100644 --- a/net/bridge/br_ioctl.c +++ b/net/bridge/br_ioctl.c @@ -117,7 +117,7 @@ static int br_dev_read_uargs(user_uintptr_t *args, size_t nr_args, return -EINVAL;
if (in_compat_syscall()) { - unsigned int cargs[BR_UARGS_MAX]; + compat_uptr_t cargs[BR_UARGS_MAX]; int i;
ret = copy_from_user(cargs, data, nr_args * sizeof(*cargs));
linux-morello@op-lists.linaro.org
-
Kevin Brodsky -
Luca Vizzarro