From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752773Ab1ARQmH (ORCPT ); Tue, 18 Jan 2011 11:42:07 -0500 Received: from kanga.kvack.org ([205.233.56.17]:39891 "EHLO kanga.kvack.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750867Ab1ARQmC (ORCPT ); Tue, 18 Jan 2011 11:42:02 -0500 Date: Tue, 18 Jan 2011 11:42:00 -0500 From: Benjamin LaHaise To: Jesper Juhl Cc: netdev@vger.kernel.org, linux-ns83820@kvack.org, linux-kernel@vger.kernel.org, Tejun Heo , Kulikov Vasiliy , Denis Kirjanov , "David S. Miller" Subject: Re: [PATCH] ns83820: Avoid bad pointer deref in ns83820_init_one(). Message-ID: <20110118164200.GI17839@kvack.org> References: Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.4.2.2i Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Jan 17, 2011 at 09:24:57PM +0100, Jesper Juhl wrote: > In drivers/net/ns83820.c::ns83820_init_one() we dynamically allocate > memory via alloc_etherdev(). We then call PRIV() on the returned storage > which is 'return netdev_priv()'. netdev_priv() takes the pointer it is > passed and adds 'ALIGN(sizeof(struct net_device), NETDEV_ALIGN)' to it and > returns it. Then we test the resulting pointer for NULL, which it is > unlikely to be at this point, and later dereference it. This will go bad > if alloc_etherdev() actually returned NULL. > > This patch reworks the code slightly so that we test for a NULL pointer > (and return -ENOMEM) directly after calling alloc_etherdev(). Looks good. -ben Signed-off-by: Benjamin LaHaise > Signed-off-by: Jesper Juhl > --- > ns83820.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > Compile tested only. I have no way to test this for real. > > diff --git a/drivers/net/ns83820.c b/drivers/net/ns83820.c > index 84134c7..a41b2cf 100644 > --- a/drivers/net/ns83820.c > +++ b/drivers/net/ns83820.c > @@ -1988,12 +1988,11 @@ static int __devinit ns83820_init_one(struct pci_dev *pci_dev, > } > > ndev = alloc_etherdev(sizeof(struct ns83820)); > - dev = PRIV(ndev); > - > err = -ENOMEM; > - if (!dev) > + if (!ndev) > goto out; > > + dev = PRIV(ndev); > dev->ndev = ndev; > > spin_lock_init(&dev->rx_info.lock); > > > -- > Jesper Juhl http://www.chaosbits.net/ > Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html > Plain text mails only, please.