I'm working with the Morello board/SoC, and building Debian from the 1.5 release.
It seems dev/net/tun is not supported by default. After some investigation, I have found that it is not configured in linux\arch\arm64\configs\morello_transitional_pcuabi_defconfig. Setting CONFIG_TUN=y in there, I get a compilation error:
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,
Ok, so this is obviously one of the numerous cases where a 128-bit pointer isn't compatible with a 64-bit integer any more, requiring some porting work for Morello.
As a quick hack, I removed the offending entry (.unlocked_ioctl) from the initialisation of tun_fops to make the compilation error go away. The resulting build seems to work fine, and dev/net/tun works OK with my base ABI application.
I'm concerned that this quick hack will not hold up when I start using purecap ABI rather than base ABI.
So, I would like to know: is there a plan to include support for the tun driver?
I can see several 'Add PCuABI support to *** ioctls' issues raised against the code, but none that seems to cover this particular case, which seems to be of the same nature.
Thanks!
David
Enable the required option to have the "Universal TUN/TAP device driver support" working on a Morello SoC in the default defconfig for Morello Transitional PCuABI (morello_transitional_pcuabi_defconfig):
...
- CONFIG_TUN=y
...
A rebased version of the patches on morello/master (morello-last-5.18), to be used for testing purposes, can be found at: https://git.morello-project.org/vincenzo/linux morello/tun/v1
Signed-off-by: Vincenzo Frascino vincenzo.frascino@arm.com
Vincenzo Frascino (2): net: Fix incompatible function pointer types in TUN driver. arm64: morello: Enable TUN in defconfig
.../morello_transitional_pcuabi_defconfig | 1 + drivers/net/tun.c | 35 ++----------------- 2 files changed, 4 insertions(+), 32 deletions(-)
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 --- 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)); -} -#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,
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,
Enable the required option to have the "Universal TUN/TAP device driver support" working on a Morello SoC in the default defconfig for Morello Transitional PCuABI (morello_transitional_pcuabi_defconfig): - CONFIG_TUN=y
Signed-off-by: Vincenzo Frascino vincenzo.frascino@arm.com --- arch/arm64/configs/morello_transitional_pcuabi_defconfig | 1 + 1 file changed, 1 insertion(+)
diff --git a/arch/arm64/configs/morello_transitional_pcuabi_defconfig b/arch/arm64/configs/morello_transitional_pcuabi_defconfig index 8500619dce2f..edc5e3e6def5 100644 --- a/arch/arm64/configs/morello_transitional_pcuabi_defconfig +++ b/arch/arm64/configs/morello_transitional_pcuabi_defconfig @@ -96,6 +96,7 @@ CONFIG_SATA_AHCI=y CONFIG_MD=y CONFIG_BLK_DEV_DM=y CONFIG_NETDEVICES=y +CONFIG_TUN=y CONFIG_VETH=y CONFIG_VIRTIO_NET=y CONFIG_R8169=y
Hi David,
On 2/23/23 10:58, David Horton wrote:
I'm working with the Morello board/SoC, and building Debian from the 1.5 release.
It seems dev/net/tun is not supported by default. After some investigation, I have found that it is not configured in linux\arch\arm64\configs\morello_transitional_pcuabi_defconfig. Setting CONFIG_TUN=y in there, I get a compilation error:
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,
Ok, so this is obviously one of the numerous cases where a 128-bit pointer isn't compatible with a 64-bit integer any more, requiring some porting work for Morello.
As a quick hack, I removed the offending entry (.unlocked_ioctl) from the initialisation of tun_fops to make the compilation error go away. The resulting build seems to work fine, and dev/net/tun works OK with my base ABI application.
I'm concerned that this quick hack will not hold up when I start using purecap ABI rather than base ABI.
So, I would like to know: is there a plan to include support for the tun driver?
I can see several 'Add PCuABI support to *** ioctls' issues raised against the code, but none that seems to cover this particular case, which seems to be of the same nature.
Thank you for reporting the issue with TUN.
I just posted a series of 2 patches in reply to your original email, could you please try them and if they work for you provide your "Tested-by:" tag?
Thank you!
Thanks!
David _______________________________________________ linux-morello mailing list -- linux-morello@op-lists.linaro.org To unsubscribe send an email to linux-morello-leave@op-lists.linaro.org
Wow! Mighty quick work Vincenzo. Thanks for the prompt response. I've done a fairly thorough test of the patches: - builds OK - boots OK - Base ABI application is able to use dev/net/tun for VPN communication, including ioctl calls. - Purecap ABI test application is able to open dev/net/tun and make an ioctl call without error. I don't have my full VPN application running purecap yet, but built a small C test harness for this. - anti-test: Purecap ABI test application running on Debian version WITHOUT the patch (but with my hack described in original post) is able to open dev/net/tun but ioctl returns an error.
On 2/24/23 10:05, David Horton wrote:
Wow! Mighty quick work Vincenzo. Thanks for the prompt response.
Thanks David happy to have helped.
In future if you have more questions or patches to submit please feel free to use the list as a primary mean of communication. ^__^
I've done a fairly thorough test of the patches:
- builds OK
- boots OK
- Base ABI application is able to use dev/net/tun for VPN communication, including ioctl calls.
- Purecap ABI test application is able to open dev/net/tun and make an ioctl call without error. I don't have my full VPN application running purecap yet, but built a small C test harness for this.
- anti-test: Purecap ABI test application running on Debian version WITHOUT the patch (but with my hack described in original post) is able to open dev/net/tun but ioctl returns an error.
linux-morello mailing list -- linux-morello@op-lists.linaro.org To unsubscribe send an email to linux-morello-leave@op-lists.linaro.org
On 24/02/2023 11:05, David Horton wrote:
Wow! Mighty quick work Vincenzo. Thanks for the prompt response. I've done a fairly thorough test of the patches:
- builds OK
- boots OK
- Base ABI application is able to use dev/net/tun for VPN communication, including ioctl calls.
- Purecap ABI test application is able to open dev/net/tun and make an ioctl call without error. I don't have my full VPN application running purecap yet, but built a small C test harness for this.
- anti-test: Purecap ABI test application running on Debian version WITHOUT the patch (but with my hack described in original post) is able to open dev/net/tun but ioctl returns an error.
Thanks for the testing, appreciated! I'll add a Tested-by <you> tag to both patches when merging them, if that's alright with you.
Kevin
linux-morello@op-lists.linaro.org
-
David Horton -
Kevin Brodsky -
Vincenzo Frascino