linux-kernel - Re: Staging: vt6656 ?

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:	Thu, 2 Jul 2009 14:22:08 -0400
From:	Forest Bond <forest@...ttletooquiet.net>
To:	Bartlomiej Zolnierkiewicz <bzolnier@...il.com>
Cc:	Greg KH <gregkh@...e.de>,
	Alexander Beregalov <a.beregalov@...il.com>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: Staging: vt6656 ?

Hi,

On Thu, Jul 02, 2009 at 07:45:49PM +0200, Bartlomiej Zolnierkiewicz wrote:
> On Sunday 28 June 2009 18:47:16 Forest Bond wrote:
> > On Sun, Jun 28, 2009 at 05:59:45PM +0200, Bartlomiej Zolnierkiewicz wrote:
> > > [ I'll later setup vt665x branch of my misc.git tree, merge your patches,
> > >   merge all outstanding vt6655 patches from Alexander and investigate a bit
> > >   more whether merge of vt665x drivers is feasible and what needs to be
> > >   done if so.. ]
> > 
> > Good.
> 
> The temporary tree is here:
> 
> git://git.kernel.org:/pub/scm/linux/kernel/git/bart/misc.git vt665x
> 
> and I'll happily apply patches to it till Greg digs out from under the
> overdue patch queues.. :)

Thanks for doing this, Bartlomiej.

> > FYI, there is a known issue with the drivers as I've submitted them that causes
> > lock-ups.  Please see the attached message for a suggested fix.
> 
> I think that all netdev_priv() changes should be reverted for now:

I'm happy to defer to you on this.  I don't really understand the code, to be
frank.  However, if those changes are simply reverted, the driver will not
compile.  I assume that you mean those areas should be removed?

> --- a/drivers/staging/vt6655/wpactl.c
> +++ b/drivers/staging/vt6655/wpactl.c
> @@ -112,14 +112,17 @@ static void wpadev_setup(struct net_device *dev)
>  
>  static int wpa_init_wpadev(PSDevice pDevice)
>  {
> +    PSDevice wpadev_priv;
>  	struct net_device *dev = pDevice->dev;
>           int ret=0;
>  
> -	pDevice->wpadev = alloc_netdev(0, "vntwpa", wpadev_setup);
> +	pDevice->wpadev = alloc_netdev(sizeof(PSDevice), "vntwpa", wpadev_setup);
>  	if (pDevice->wpadev == NULL)
>  		return -ENOMEM;
>  
> -	pDevice->wpadev->priv = pDevice;
> +    wpadev_priv = netdev_priv(pDevice->wpadev);
> +    *wpadev_priv = *pDevice;
> +
>  	memcpy(pDevice->wpadev->dev_addr, dev->dev_addr, U_ETHER_ADDR_LEN);
>           pDevice->wpadev->base_addr = dev->base_addr;
>  	pDevice->wpadev->irq = dev->irq;
> 
> This will copy the current state of pDevice to newly allocated private part
> of ->apdev but later modifications to the original pDevice won't be seen if
> we access it through netdev_priv(pDevice->apdev) instead of apdev->priv.
> 
> [ I don't know whether this is a problem currently but it looks suspicious. ]

Agreed.  I gave this a best effort, but was not very confident about the result.

Feel free to aggressively rework my changes if it seems appropriate.

Thanks,
Forest
-- 
Forest Bond
http://www.alittletooquiet.net
http://www.pytagsfs.org

Download attachment "signature.asc" of type "application/pgp-signature" (190 bytes)