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
| ||
|
Date: Tue, 4 Sep 2007 14:20:21 +0530 (IST) From: Satyam Sharma <satyam@...radead.org> To: Micah Gruber <micah.gruber@...il.com> cc: Linux Kernel Mailing List <linux-kernel@...r.kernel.org>, Linux Netdev Mailing List <netdev@...r.kernel.org>, jgarzik@...ox.com Subject: Re: [PATCH] Fix a potential NULL pointer dereference in uli526x_interrupt() in drivers/net/tulip/uli526x.c On Tue, 4 Sep 2007, Satyam Sharma wrote: > Hi Micah, > > > On Tue, 4 Sep 2007, Micah Gruber wrote: > > > This patch fixes a potential null dereference bug where we dereference > > dev before a null check. This patch simply moves the dereferencing after > > the null check. > > > > Signed-off-by: Micah Gruber <micah.gruber@...il.com> > > --- > > > > --- a/drivers/net/tulip/uli526x.c > > +++ b/drivers/net/tulip/uli526x.c > > @@ -663,7 +663,7 @@ > > { > > struct net_device *dev = dev_id; > > struct uli526x_board_info *db = netdev_priv(dev); > > - unsigned long ioaddr; > > + unsigned long ioaddr = dev->base_addr; > > unsigned long flags; > > > > if (!dev) { > > @@ -671,8 +671,6 @@ > > return IRQ_NONE; > > } > > > > - ioaddr = dev->base_addr; > > - > > spin_lock_irqsave(&db->lock, flags); > > outl(0, ioaddr + DCR7); > > > [The patch is reversed.] > > But it is also complete -- you've left out the "netdev_priv(dev)" ^^^^^^^^ I meant: incomplete > statement _just_ before the dereference that you've pushed down. > Coverity wasn't smart enough to tell, but that's actually a dev->priv, > so we'll _still_ be dereferencing dev before checking it for NULL below. > And lastly, the patch isn't quite correct. It is the (!dev) check that > is redundant and must be removed instead :-) > > I bet more such pointless checks exist all over. It makes more sense to > remove them than unnecessarily try to solve "potential" NULL dereferences, > that we (naturally) never saw earlier, because they're impossible. ISTR > pointing out two similar patches a month or so back when Jeff pushed net > driver patches upstream, IIRC ... ] > > > Satyam - 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/
Powered by blists - more mailing lists