2021-02-20 11:57:13

by Karthik Alapati

[permalink] [raw]
Subject: [PATCH] staging: wimax: fix sparse incorrect type issue

fix sparse warning by casting to explicit user address-space
pointer type

Signed-off-by: karthik alapati <[email protected]>
---
drivers/staging/wlan-ng/p80211netdev.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/wlan-ng/p80211netdev.c b/drivers/staging/wlan-ng/p80211netdev.c
index 6f9666dc0..70570e8a5 100644
--- a/drivers/staging/wlan-ng/p80211netdev.c
+++ b/drivers/staging/wlan-ng/p80211netdev.c
@@ -569,7 +569,7 @@ static int p80211knetdev_do_ioctl(struct net_device *dev,
goto bail;
}

- msgbuf = memdup_user(req->data, req->len);
+ msgbuf = memdup_user((void __user *)req->data, req->len);
if (IS_ERR(msgbuf)) {
result = PTR_ERR(msgbuf);
goto bail;
--
2.30.1


2021-02-22 05:43:51

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH] staging: wimax: fix sparse incorrect type issue

On Sat, Feb 20, 2021 at 05:04:00PM +0530, karthik alapati wrote:
> fix sparse warning by casting to explicit user address-space
> pointer type
>
> Signed-off-by: karthik alapati <[email protected]>
> ---
> drivers/staging/wlan-ng/p80211netdev.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/wlan-ng/p80211netdev.c b/drivers/staging/wlan-ng/p80211netdev.c
> index 6f9666dc0..70570e8a5 100644
> --- a/drivers/staging/wlan-ng/p80211netdev.c
> +++ b/drivers/staging/wlan-ng/p80211netdev.c
> @@ -569,7 +569,7 @@ static int p80211knetdev_do_ioctl(struct net_device *dev,
> goto bail;
> }
>
> - msgbuf = memdup_user(req->data, req->len);
> + msgbuf = memdup_user((void __user *)req->data, req->len);

This doesn't fix anything it just silences the warning. Linus Torvalds
worked very hard to create Sparse for the express purpose of printing
the warning. People don't realize that warnings are very valuable
because they show where the bugs are.

Please look at this some more and figure out how to fix the warning.

To be honest, I'm tempted to not accept any patch which doesn't also fix
the buffer overflows when we pass:

result = p80211req_dorequest(wlandev, msgbuf);

How do we know that "msgbuf" is large enough?

regards,
dan carpenter

2021-02-22 06:04:37

by Karthik Alapati

[permalink] [raw]
Subject: Re: [PATCH] staging: wimax: fix sparse incorrect type issue

On Mon, Feb 22, 2021 at 11:10 AM Dan Carpenter <[email protected]> wrote:
>
> On Sat, Feb 20, 2021 at 05:04:00PM +0530, karthik alapati wrote:
> > fix sparse warning by casting to explicit user address-space
> > pointer type
> >
> > Signed-off-by: karthik alapati <[email protected]>
> > ---
> > drivers/staging/wlan-ng/p80211netdev.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/staging/wlan-ng/p80211netdev.c b/drivers/staging/wlan-ng/p80211netdev.c
> > index 6f9666dc0..70570e8a5 100644
> > --- a/drivers/staging/wlan-ng/p80211netdev.c
> > +++ b/drivers/staging/wlan-ng/p80211netdev.c
> > @@ -569,7 +569,7 @@ static int p80211knetdev_do_ioctl(struct net_device *dev,
> > goto bail;
> > }
> >
> > - msgbuf = memdup_user(req->data, req->len);
> > + msgbuf = memdup_user((void __user *)req->data, req->len);
>
> This doesn't fix anything it just silences the warning. Linus Torvalds
> worked very hard to create Sparse for the express purpose of printing
> the warning. People don't realize that warnings are very valuable
> because they show where the bugs are.
>
> Please look at this some more and figure out how to fix the warning.
>
> To be honest, I'm tempted to not accept any patch which doesn't also fix
> the buffer overflows when we pass:
>
> result = p80211req_dorequest(wlandev, msgbuf);
>
> How do we know that "msgbuf" is large enough?
>
> regards,
> dan carpenter
>

Thanks dan but right after sending this patch i immediately replied to
it stating
to ignore this patch as i found this already applied in staging-testing branch
https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git/commit/?h=staging-testing&id=3a8a144d2a754df45127c74e273fa166f690ba43

2021-02-22 08:14:36

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH] staging: wimax: fix sparse incorrect type issue

On Mon, Feb 22, 2021 at 11:31:48AM +0530, karthek wrote:
> On Mon, Feb 22, 2021 at 11:10 AM Dan Carpenter <[email protected]> wrote:
> >
> > On Sat, Feb 20, 2021 at 05:04:00PM +0530, karthik alapati wrote:
> > > fix sparse warning by casting to explicit user address-space
> > > pointer type
> > >
> > > Signed-off-by: karthik alapati <[email protected]>
> > > ---
> > > drivers/staging/wlan-ng/p80211netdev.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/staging/wlan-ng/p80211netdev.c b/drivers/staging/wlan-ng/p80211netdev.c
> > > index 6f9666dc0..70570e8a5 100644
> > > --- a/drivers/staging/wlan-ng/p80211netdev.c
> > > +++ b/drivers/staging/wlan-ng/p80211netdev.c
> > > @@ -569,7 +569,7 @@ static int p80211knetdev_do_ioctl(struct net_device *dev,
> > > goto bail;
> > > }
> > >
> > > - msgbuf = memdup_user(req->data, req->len);
> > > + msgbuf = memdup_user((void __user *)req->data, req->len);
> >
> > This doesn't fix anything it just silences the warning. Linus Torvalds
> > worked very hard to create Sparse for the express purpose of printing
> > the warning. People don't realize that warnings are very valuable
> > because they show where the bugs are.
> >
> > Please look at this some more and figure out how to fix the warning.
> >
> > To be honest, I'm tempted to not accept any patch which doesn't also fix
> > the buffer overflows when we pass:
> >
> > result = p80211req_dorequest(wlandev, msgbuf);
> >
> > How do we know that "msgbuf" is large enough?
> >
> > regards,
> > dan carpenter
> >
>
> Thanks dan but right after sending this patch i immediately replied to
> it stating
> to ignore this patch as i found this already applied in staging-testing branch
> https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git/commit/?h=staging-testing&id=3a8a144d2a754df45127c74e273fa166f690ba43


It's still possible to fix this in the correct way and fix the buffer
overflows.

regards,
dan carpenter