On 23/02/2023 13:56, Vincenzo Frascino wrote:
With the introduction of capabilities, the PCuABI expects a capability when dealing with the user pointers.
When the TUN driver is enabled in a PCuABI compatible kernel, the compilation reports the error below:
drivers/net/tun.c:3478:20: error: incompatible function pointer types initializing 'long (*)(struct file *, unsigned int, user_uintptr_t)' (aka 'long (*)(struct file *, unsigned int, unsigned __intcap)') with an expression of type 'long (struct file *, unsigned int, unsigned long)' [-Werror,-Wincompatible-function-pointer-types] .unlocked_ioctl = tun_chr_ioctl, ^~~~~~~~~~~~~ 1 error generated.
Address a compilation issue using the proper type to represent user pointers (user_uintptr_t) and redirecting the .compat_ioctl.
Signed-off-by: Vincenzo Frascino vincenzo.frascino@arm.com
Thanks for the patch! Regarding the commit title, how about using the "tun:" tag to shorten it, also "function pointer" may be misinterpreted, so maybe something like:
net: tun: Fix ioctl handler argument type
drivers/net/tun.c | 35 +++-------------------------------- 1 file changed, 3 insertions(+), 32 deletions(-)
diff --git a/drivers/net/tun.c b/drivers/net/tun.c index dbe4c0a4be2c..d76131b29bbc 100644 --- a/drivers/net/tun.c +++ b/drivers/net/tun.c @@ -3018,7 +3018,7 @@ static unsigned char tun_get_addr_len(unsigned short type) } static long __tun_chr_ioctl(struct file *file, unsigned int cmd,
unsigned long arg, int ifreq_len)
user_uintptr_t arg, int ifreq_len){ struct tun_file *tfile = file->private_data; struct net *net = sock_net(&tfile->sk); @@ -3349,40 +3349,11 @@ static long __tun_chr_ioctl(struct file *file, unsigned int cmd, } static long tun_chr_ioctl(struct file *file,
unsigned int cmd, unsigned long arg)
unsigned int cmd, user_uintptr_t arg){ return __tun_chr_ioctl(file, cmd, arg, sizeof (struct ifreq)); } -#ifdef CONFIG_COMPAT -static long tun_chr_compat_ioctl(struct file *file,
unsigned int cmd, unsigned long arg)-{
- switch (cmd) {
- case TUNSETIFF:
- case TUNGETIFF:
- case TUNSETTXFILTER:
- case TUNGETSNDBUF:
- case TUNSETSNDBUF:
- case SIOCGIFHWADDR:
- case SIOCSIFHWADDR:
arg = (unsigned long)compat_ptr(arg);break;- default:
arg = (compat_ulong_t)arg;break;- }
- /*
* compat_ifreq is shorter than ifreq, so we must not access beyond* the end of that structure. All fields that are used in this* driver are compatible though, we don't need to convert the* contents.*/- return __tun_chr_ioctl(file, cmd, arg, sizeof(struct compat_ifreq));
I think we still need this special compat handling. struct compat_ifreq is smaller than struct ifreq so we need to pass a different size in compat.
That said the switch above is indeed broken as in fact most commands expect a pointer. Using (user_uintptr_t)compat_ptr(arg) unconditionally would be fine by me.
Happy with the rest and patch 2.
Kevin
-} -#endif /* CONFIG_COMPAT */
static int tun_chr_fasync(int fd, struct file *file, int on) { struct tun_file *tfile = file->private_data; @@ -3477,7 +3448,7 @@ static const struct file_operations tun_fops = { .poll = tun_chr_poll, .unlocked_ioctl = tun_chr_ioctl, #ifdef CONFIG_COMPAT
- .compat_ioctl = tun_chr_compat_ioctl,
- .compat_ioctl = compat_ptr_ioctl,
#endif .open = tun_chr_open, .release = tun_chr_close,