linux-kernel - Re: [PATCH] staging: vt6655: wpactl.c: Fix sparse warnings

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 8 Aug 2014 09:07:55 +0200
From:	Martin Berglund <martin@...sta.net>
To:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:	Forest Bond <forest@...ttletooquiet.net>,
	Guido Martínez <guido@...guardiasur.com.ar>,
	devel@...verdev.osuosl.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] staging: vt6655: wpactl.c: Fix sparse warnings

On Thu, Aug 07, 2014 at 07:18:13PM -0700, Greg Kroah-Hartman wrote:
> On Thu, Aug 07, 2014 at 11:08:34PM +0100, Martin Berglund wrote:
> > Add missing __user macro casting in the function wpa_set_keys.
> > This is okay since the function handles the possibility of
> > param->u.wpa_key.key and param->u.wpa_key.seq pointing to
> > kernelspace using a flag, fcpfkernel.
> > 
> > Signed-off-by: Martin Berglund <martin@...sta.net>
> > ---
> > This was submitted as part of Eudyptula challenge task 16
> > 
> >  drivers/staging/vt6655/wpactl.c |    8 ++++++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/staging/vt6655/wpactl.c b/drivers/staging/vt6655/wpactl.c
> > index 5f454ca..d75dd79 100644
> > --- a/drivers/staging/vt6655/wpactl.c
> > +++ b/drivers/staging/vt6655/wpactl.c
> > @@ -224,7 +224,9 @@ int wpa_set_keys(PSDevice pDevice, void *ctx,
> >  	} else {
> >  		spin_unlock_irq(&pDevice->lock);
> >  		if (param->u.wpa_key.key &&
> > -		    copy_from_user(&abyKey[0], param->u.wpa_key.key, param->u.wpa_key.key_len)) {
> > +		    copy_from_user(&abyKey[0],
> > +				   (void __user *)param->u.wpa_key.key,
> 
> Would it be better to mark this pointer as __user in the structure
> itself?  Or is it also used as a kernel structure in other places?
> 
> thanks,
> 
> greg k-h

Yes, the structure is used as a kernel structure in some other places. 
Even this function is sometimes called with the pointers in the
structure pointing to kernel memory. However, that is correctly
handled with a flag also sent to the function.

As a side note: there are some uses of memcpy in this file that
might be good to switch to copy_from/to_user but it's not as clear
to me if these pointers never can point to kernel memory (because of
the mixing of the two). For example all copying of ssid and bssid.

Cheers,
                Martin
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/