Messages in this thread | | | Date | Wed, 28 Nov 2007 13:53:46 +0300 | From | "Cyrill Gorcunov" <> | Subject | Re: [PATCH] PPC: CELLEB - fix potential NULL pointer dereference |
| |
On 11/28/07, Michael Ellerman <michael@ellerman.id.au> wrote: > On Mon, 2007-11-26 at 10:46 +0300, Cyrill Gorcunov wrote: > > This patch adds checking for NULL value returned to prevent possible > > NULL pointer dereference. > > Also two unneeded 'return' are removed. > > > > Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com> > > --- > > Any comments are welcome. > > I guess it's good to be paranoid, but this is a little verbose: > > wi0 = of_get_property(node, "device-id", NULL); > + if (unlikely((!wi0))) { > + printk(KERN_ERR "PCI: device-id not found.\n"); > + goto error; > + } > wi1 = of_get_property(node, "vendor-id", NULL); > + if (unlikely((!wi1))) { > + printk(KERN_ERR "PCI: vendor-id not found.\n"); > + goto error; > + } > wi2 = of_get_property(node, "class-code", NULL); > + if (unlikely((!wi2))) { > + printk(KERN_ERR "PCI: class-code not found.\n"); > + goto error; > + } > wi3 = of_get_property(node, "revision-id", NULL); > + if (unlikely((!wi3))) { > + printk(KERN_ERR "PCI: revision-id not found.\n"); > + goto error; > + } > > Perhaps instead: > > wi0 = of_get_property(node, "device-id", NULL); > wi1 = of_get_property(node, "vendor-id", NULL); > wi2 = of_get_property(node, "class-code", NULL); > wi3 = of_get_property(node, "revision-id", NULL); > > if (!wi0 || !wi1 || !wi2 || !wi3) { > printk(KERN_ERR "PCI: Missing device tree properties.\n"); > goto error; > }
Hi Michael, yes that is much better (actually I was doubt about what form of which the checking style to use - your form is much compact but mine does show where *exactly* the problem appeared). So 'case that is the fake driver your form is preferred ;) Ishizaki, could you use Michael's part then?
> > > cheers > > -- > Michael Ellerman > OzLabs, IBM Australia Development Lab > > wwweb: http://michael.ellerman.id.au > phone: +61 2 6212 1183 (tie line 70 21183) > > We do not inherit the earth from our ancestors, > we borrow it from our children. - S.M.A.R.T Person > >
Cyrill - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |