* [PATCH] kni: fix ioctl signature
@ 2021-11-20 13:14 Markus Theil
2021-11-22 13:19 ` Ferruh Yigit
0 siblings, 1 reply; 7+ messages in thread
From: Markus Theil @ 2021-11-20 13:14 UTC (permalink / raw)
To: ferruh.yigit; +Cc: dev, markus.theil, Michael Pfeiffer
From: Markus Theil <markus.theil@secunet.com>
Fix kni's ioctl signature to correctly match the kernel's
structs. This shaves off the (void*) casts and uses struct file*
instead of struct inode*. With the correct signature, control flow
integrity checkers are no longer confused at this point.
Signed-off-by: Markus Theil <markus.theil@secunet.com>
Tested-by: Michael Pfeiffer <michael.pfeiffer@tu-ilmenau.de>
---
kernel/linux/kni/kni_misc.c | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/kernel/linux/kni/kni_misc.c b/kernel/linux/kni/kni_misc.c
index f4944e1ddf..d4e00abc40 100644
--- a/kernel/linux/kni/kni_misc.c
+++ b/kernel/linux/kni/kni_misc.c
@@ -245,7 +245,7 @@ kni_check_param(struct kni_dev *kni, struct rte_kni_device_info *dev)
return 0;
}
-static int
+static long
kni_run_thread(struct kni_net *knet, struct kni_dev *kni, uint8_t force_bind)
{
/**
@@ -286,12 +286,12 @@ kni_run_thread(struct kni_net *knet, struct kni_dev *kni, uint8_t force_bind)
return 0;
}
-static int
+static long
kni_ioctl_create(struct net *net, uint32_t ioctl_num,
unsigned long ioctl_param)
{
struct kni_net *knet = net_generic(net, kni_net_id);
- int ret;
+ long ret;
struct rte_kni_device_info dev_info;
struct net_device *net_dev = NULL;
struct kni_dev *kni, *dev, *n;
@@ -416,7 +416,7 @@ kni_ioctl_create(struct net *net, uint32_t ioctl_num,
ret = register_netdev(net_dev);
if (ret) {
- pr_err("error %i registering device \"%s\"\n",
+ pr_err("error %li registering device \"%s\"\n",
ret, dev_info.name);
kni->net_dev = NULL;
kni_dev_remove(kni);
@@ -437,12 +437,12 @@ kni_ioctl_create(struct net *net, uint32_t ioctl_num,
return 0;
}
-static int
+static long
kni_ioctl_release(struct net *net, uint32_t ioctl_num,
unsigned long ioctl_param)
{
struct kni_net *knet = net_generic(net, kni_net_id);
- int ret = -EINVAL;
+ long ret = -EINVAL;
struct kni_dev *dev, *n;
struct rte_kni_device_info dev_info;
@@ -478,8 +478,8 @@ kni_ioctl_release(struct net *net, uint32_t ioctl_num,
return ret;
}
-static int
-kni_ioctl(struct inode *inode, uint32_t ioctl_num, unsigned long ioctl_param)
+static long
+kni_ioctl(struct file *file, uint32_t ioctl_num, unsigned long ioctl_param)
{
int ret = -EINVAL;
struct net *net = current->nsproxy->net_ns;
@@ -507,8 +507,8 @@ kni_ioctl(struct inode *inode, uint32_t ioctl_num, unsigned long ioctl_param)
return ret;
}
-static int
-kni_compat_ioctl(struct inode *inode, uint32_t ioctl_num,
+static long
+kni_compat_ioctl(struct file *file, uint32_t ioctl_num,
unsigned long ioctl_param)
{
/* 32 bits app on 64 bits OS to be supported later */
@@ -521,8 +521,8 @@ static const struct file_operations kni_fops = {
.owner = THIS_MODULE,
.open = kni_open,
.release = kni_release,
- .unlocked_ioctl = (void *)kni_ioctl,
- .compat_ioctl = (void *)kni_compat_ioctl,
+ .unlocked_ioctl = kni_ioctl,
+ .compat_ioctl = kni_compat_ioctl,
};
static struct miscdevice kni_misc = {
--
2.33.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] kni: fix ioctl signature
2021-11-20 13:14 [PATCH] kni: fix ioctl signature Markus Theil
@ 2021-11-22 13:19 ` Ferruh Yigit
2021-11-28 13:14 ` [PATCH v2] " Markus Theil
0 siblings, 1 reply; 7+ messages in thread
From: Ferruh Yigit @ 2021-11-22 13:19 UTC (permalink / raw)
To: Markus Theil; +Cc: dev, markus.theil, Michael Pfeiffer
On 11/20/2021 1:14 PM, Markus Theil wrote:
> From: Markus Theil <markus.theil@secunet.com>
>
> Fix kni's ioctl signature to correctly match the kernel's
> structs. This shaves off the (void*) casts and uses struct file*
> instead of struct inode*. With the correct signature, control flow
> integrity checkers are no longer confused at this point.
>
+1 to update the signature, the code is like this from the initial release,
not sure why.
My concern was if this cause build issues with old kernel versions, but as
far as I can see the signature is like this at least since 2005, so that
should be OK.
A few minor comments below.
And overall for the patch, I suggest postponing this to next release, since
we are just about the release v21.11.
Although change looks safe and trivial, still I think it doesn't worth the risk,
taking account that current code is at it is for years now, it can wait a
little more.
> Signed-off-by: Markus Theil <markus.theil@secunet.com>
> Tested-by: Michael Pfeiffer <michael.pfeiffer@tu-ilmenau.de>
> ---
> kernel/linux/kni/kni_misc.c | 24 ++++++++++++------------
> 1 file changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/kernel/linux/kni/kni_misc.c b/kernel/linux/kni/kni_misc.c
> index f4944e1ddf..d4e00abc40 100644
> --- a/kernel/linux/kni/kni_misc.c
> +++ b/kernel/linux/kni/kni_misc.c
> @@ -245,7 +245,7 @@ kni_check_param(struct kni_dev *kni, struct rte_kni_device_info *dev)
> return 0;
> }
>
> -static int
> +static long
> kni_run_thread(struct kni_net *knet, struct kni_dev *kni, uint8_t force_bind)
> {
> /**
> @@ -286,12 +286,12 @@ kni_run_thread(struct kni_net *knet, struct kni_dev *kni, uint8_t force_bind)
> return 0;
> }
>
> -static int
> +static long
> kni_ioctl_create(struct net *net, uint32_t ioctl_num,
> unsigned long ioctl_param)
> {
> struct kni_net *knet = net_generic(net, kni_net_id);
> - int ret;
> + long ret;
> struct rte_kni_device_info dev_info;
> struct net_device *net_dev = NULL;
> struct kni_dev *kni, *dev, *n;
> @@ -416,7 +416,7 @@ kni_ioctl_create(struct net *net, uint32_t ioctl_num,
>
> ret = register_netdev(net_dev);
> if (ret) {
> - pr_err("error %i registering device \"%s\"\n",
> + pr_err("error %li registering device \"%s\"\n",
> ret, dev_info.name);
> kni->net_dev = NULL;
> kni_dev_remove(kni);> @@ -437,12 +437,12 @@ kni_ioctl_create(struct net *net, uint32_t ioctl_num,
> return 0;
> }
>
> -static int
> +static long
> kni_ioctl_release(struct net *net, uint32_t ioctl_num,
> unsigned long ioctl_param)
> {
> struct kni_net *knet = net_generic(net, kni_net_id);
> - int ret = -EINVAL;
> + long ret = -EINVAL;
> struct kni_dev *dev, *n;
> struct rte_kni_device_info dev_info;
>
'kni_run_thread()', 'kni_ioctl_create()' & 'kni_ioctl_release()' are all
static functions, I think it is OK to keep their signature.
At works their return value assigned to a 'long' in 'kni_ioctl()', which
is safe.
> @@ -478,8 +478,8 @@ kni_ioctl_release(struct net *net, uint32_t ioctl_num,
> return ret;
> }
>
> -static int
> -kni_ioctl(struct inode *inode, uint32_t ioctl_num, unsigned long ioctl_param)
> +static long
> +kni_ioctl(struct file *file, uint32_t ioctl_num, unsigned long ioctl_param)
> {
> int ret = -EINVAL;
What do you think about changing 'ret' variable type to 'long'?
It will be implicitly cast to 'long' anyway during return, but keeping it
same with return type looks more proper.
> struct net *net = current->nsproxy->net_ns;
> @@ -507,8 +507,8 @@ kni_ioctl(struct inode *inode, uint32_t ioctl_num, unsigned long ioctl_param)
> return ret;
> }
>
> -static int
> -kni_compat_ioctl(struct inode *inode, uint32_t ioctl_num,
> +static long
> +kni_compat_ioctl(struct file *file, uint32_t ioctl_num,
> unsigned long ioctl_param)
> {
> /* 32 bits app on 64 bits OS to be supported later */
> @@ -521,8 +521,8 @@ static const struct file_operations kni_fops = {
> .owner = THIS_MODULE,
> .open = kni_open,
> .release = kni_release,
> - .unlocked_ioctl = (void *)kni_ioctl,
> - .compat_ioctl = (void *)kni_compat_ioctl,
> + .unlocked_ioctl = kni_ioctl,
> + .compat_ioctl = kni_compat_ioctl,
> };
>
> static struct miscdevice kni_misc = {
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2] kni: fix ioctl signature
2021-11-22 13:19 ` Ferruh Yigit
@ 2021-11-28 13:14 ` Markus Theil
2021-11-28 23:05 ` Stephen Hemminger
0 siblings, 1 reply; 7+ messages in thread
From: Markus Theil @ 2021-11-28 13:14 UTC (permalink / raw)
To: ferruh.yigit; +Cc: dev, markus.theil, Michael Pfeiffer
From: Markus Theil <markus.theil@secunet.com>
Fix kni's ioctl signature to correctly match the kernel's
structs. This shaves off the (void*) casts and uses struct file*
instead of struct inode*. With the correct signature, control flow
integrity checkers are no longer confused at this point.
Signed-off-by: Markus Theil <markus.theil@secunet.com>
Tested-by: Michael Pfeiffer <michael.pfeiffer@tu-ilmenau.de>
---
v2: adapt to suggestions from Ferruh Yigit
kernel/linux/kni/kni_misc.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/kernel/linux/kni/kni_misc.c b/kernel/linux/kni/kni_misc.c
index f10dcd069d..8d56006512 100644
--- a/kernel/linux/kni/kni_misc.c
+++ b/kernel/linux/kni/kni_misc.c
@@ -482,10 +482,10 @@ kni_ioctl_release(struct net *net, uint32_t ioctl_num,
return ret;
}
-static int
-kni_ioctl(struct inode *inode, uint32_t ioctl_num, unsigned long ioctl_param)
+static long
+kni_ioctl(struct file *file, uint32_t ioctl_num, unsigned long ioctl_param)
{
- int ret = -EINVAL;
+ long ret = -EINVAL;
struct net *net = current->nsproxy->net_ns;
pr_debug("IOCTL num=0x%0x param=0x%0lx\n", ioctl_num, ioctl_param);
@@ -511,8 +511,8 @@ kni_ioctl(struct inode *inode, uint32_t ioctl_num, unsigned long ioctl_param)
return ret;
}
-static int
-kni_compat_ioctl(struct inode *inode, uint32_t ioctl_num,
+static long
+kni_compat_ioctl(struct file *file, uint32_t ioctl_num,
unsigned long ioctl_param)
{
/* 32 bits app on 64 bits OS to be supported later */
@@ -525,8 +525,8 @@ static const struct file_operations kni_fops = {
.owner = THIS_MODULE,
.open = kni_open,
.release = kni_release,
- .unlocked_ioctl = (void *)kni_ioctl,
- .compat_ioctl = (void *)kni_compat_ioctl,
+ .unlocked_ioctl = kni_ioctl,
+ .compat_ioctl = kni_compat_ioctl,
};
static struct miscdevice kni_misc = {
--
2.34.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] kni: fix ioctl signature
2021-11-28 13:14 ` [PATCH v2] " Markus Theil
@ 2021-11-28 23:05 ` Stephen Hemminger
2021-12-03 7:19 ` [PATCH v3] " Markus Theil
0 siblings, 1 reply; 7+ messages in thread
From: Stephen Hemminger @ 2021-11-28 23:05 UTC (permalink / raw)
To: Markus Theil; +Cc: ferruh.yigit, dev, markus.theil, Michael Pfeiffer
On Sun, 28 Nov 2021 14:14:26 +0100
Markus Theil <markus.theil@tu-ilmenau.de> wrote:
> From: Markus Theil <markus.theil@secunet.com>
>
> Fix kni's ioctl signature to correctly match the kernel's
> structs. This shaves off the (void*) casts and uses struct file*
> instead of struct inode*. With the correct signature, control flow
> integrity checkers are no longer confused at this point.
>
> Signed-off-by: Markus Theil <markus.theil@secunet.com>
> Tested-by: Michael Pfeiffer <michael.pfeiffer@tu-ilmenau.de>
> ---
> v2: adapt to suggestions from Ferruh Yigit
>
> kernel/linux/kni/kni_misc.c | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
> -static int
> -kni_compat_ioctl(struct inode *inode, uint32_t ioctl_num,
> +static long
> +kni_compat_ioctl(struct file *file, uint32_t ioctl_num,
> unsigned long ioctl_param)
> {
Shouldn't be uint32_t should be unsigned int.
In fs.h.
long (*compat_ioctl) (struct file *, unsigned int, unsigned long);
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v3] kni: fix ioctl signature
2021-11-28 23:05 ` Stephen Hemminger
@ 2021-12-03 7:19 ` Markus Theil
2021-12-03 16:28 ` Stephen Hemminger
0 siblings, 1 reply; 7+ messages in thread
From: Markus Theil @ 2021-12-03 7:19 UTC (permalink / raw)
To: ferruh.yigit; +Cc: dev, markus.theil, Michael Pfeiffer
From: Markus Theil <markus.theil@secunet.com>
Fix kni's ioctl signature to correctly match the kernel's
structs. This shaves off the (void*) casts and uses struct file*
instead of struct inode*. With the correct signature, control flow
integrity checkers are no longer confused at this point.
Signed-off-by: Markus Theil <markus.theil@secunet.com>
Tested-by: Michael Pfeiffer <michael.pfeiffer@tu-ilmenau.de>
---
v3: adapt to suggestions from Stephen Hemminger (use unsigned int)
v2: adapt to suggestions from Ferruh Yigit
kernel/linux/kni/kni_misc.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/kernel/linux/kni/kni_misc.c b/kernel/linux/kni/kni_misc.c
index f10dcd069d..cc5172fefc 100644
--- a/kernel/linux/kni/kni_misc.c
+++ b/kernel/linux/kni/kni_misc.c
@@ -482,10 +482,10 @@ kni_ioctl_release(struct net *net, uint32_t ioctl_num,
return ret;
}
-static int
-kni_ioctl(struct inode *inode, uint32_t ioctl_num, unsigned long ioctl_param)
+static long
+kni_ioctl(struct file *file, unsigned int ioctl_num, unsigned long ioctl_param)
{
- int ret = -EINVAL;
+ long ret = -EINVAL;
struct net *net = current->nsproxy->net_ns;
pr_debug("IOCTL num=0x%0x param=0x%0lx\n", ioctl_num, ioctl_param);
@@ -511,8 +511,8 @@ kni_ioctl(struct inode *inode, uint32_t ioctl_num, unsigned long ioctl_param)
return ret;
}
-static int
-kni_compat_ioctl(struct inode *inode, uint32_t ioctl_num,
+static long
+kni_compat_ioctl(struct file *file, unsigned int ioctl_num,
unsigned long ioctl_param)
{
/* 32 bits app on 64 bits OS to be supported later */
@@ -525,8 +525,8 @@ static const struct file_operations kni_fops = {
.owner = THIS_MODULE,
.open = kni_open,
.release = kni_release,
- .unlocked_ioctl = (void *)kni_ioctl,
- .compat_ioctl = (void *)kni_compat_ioctl,
+ .unlocked_ioctl = kni_ioctl,
+ .compat_ioctl = kni_compat_ioctl,
};
static struct miscdevice kni_misc = {
--
2.34.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3] kni: fix ioctl signature
2021-12-03 7:19 ` [PATCH v3] " Markus Theil
@ 2021-12-03 16:28 ` Stephen Hemminger
2022-02-02 19:59 ` Thomas Monjalon
0 siblings, 1 reply; 7+ messages in thread
From: Stephen Hemminger @ 2021-12-03 16:28 UTC (permalink / raw)
To: Markus Theil; +Cc: ferruh.yigit, dev, markus.theil, Michael Pfeiffer
On Fri, 3 Dec 2021 08:19:07 +0100
Markus Theil <markus.theil@tu-ilmenau.de> wrote:
> From: Markus Theil <markus.theil@secunet.com>
>
> Fix kni's ioctl signature to correctly match the kernel's
> structs. This shaves off the (void*) casts and uses struct file*
> instead of struct inode*. With the correct signature, control flow
> integrity checkers are no longer confused at this point.
>
> Signed-off-by: Markus Theil <markus.theil@secunet.com>
> Tested-by: Michael Pfeiffer <michael.pfeiffer@tu-ilmenau.de>
> ---
Thanks for fixing.
Acked-by: Stephen Hemminger <stephen@networkplumber.org>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3] kni: fix ioctl signature
2021-12-03 16:28 ` Stephen Hemminger
@ 2022-02-02 19:59 ` Thomas Monjalon
0 siblings, 0 replies; 7+ messages in thread
From: Thomas Monjalon @ 2022-02-02 19:59 UTC (permalink / raw)
To: Markus Theil, markus.theil
Cc: dev, ferruh.yigit, Michael Pfeiffer, Stephen Hemminger
03/12/2021 17:28, Stephen Hemminger:
> On Fri, 3 Dec 2021 08:19:07 +0100
> Markus Theil <markus.theil@tu-ilmenau.de> wrote:
>
> > From: Markus Theil <markus.theil@secunet.com>
> >
> > Fix kni's ioctl signature to correctly match the kernel's
> > structs. This shaves off the (void*) casts and uses struct file*
> > instead of struct inode*. With the correct signature, control flow
> > integrity checkers are no longer confused at this point.
> >
> > Signed-off-by: Markus Theil <markus.theil@secunet.com>
> > Tested-by: Michael Pfeiffer <michael.pfeiffer@tu-ilmenau.de>
> > ---
>
> Thanks for fixing.
>
> Acked-by: Stephen Hemminger <stephen@networkplumber.org>
Applied, thanks
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-02-02 19:59 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-20 13:14 [PATCH] kni: fix ioctl signature Markus Theil
2021-11-22 13:19 ` Ferruh Yigit
2021-11-28 13:14 ` [PATCH v2] " Markus Theil
2021-11-28 23:05 ` Stephen Hemminger
2021-12-03 7:19 ` [PATCH v3] " Markus Theil
2021-12-03 16:28 ` Stephen Hemminger
2022-02-02 19:59 ` Thomas Monjalon
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).