* baycom_par dereference before check.
@ 2004-04-16 21:20 Dave Jones
2004-04-16 21:25 ` viro
0 siblings, 1 reply; 8+ messages in thread
From: Dave Jones @ 2004-04-16 21:20 UTC (permalink / raw)
To: jgarzik; +Cc: Linux Kernel
--- linux-2.6.5/drivers/net/hamradio/baycom_par.c~ 2004-04-16 22:18:53.000000000 +0100
+++ linux-2.6.5/drivers/net/hamradio/baycom_par.c 2004-04-16 22:19:33.000000000 +0100
@@ -272,9 +272,13 @@
static void par96_interrupt(int irq, void *dev_id, struct pt_regs *regs)
{
struct net_device *dev = (struct net_device *)dev_id;
- struct baycom_state *bc = netdev_priv(dev);
+ struct baycom_state *bc;
- if (!dev || !bc || bc->hdrv.magic != HDLCDRV_MAGIC)
+ if (!dev)
+ return;
+
+ bc = netdev_priv(dev);
+ if (!bc || bc->hdrv.magic != HDLCDRV_MAGIC)
return;
baycom_int_freq(bc);
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: baycom_par dereference before check.
2004-04-16 21:20 baycom_par dereference before check Dave Jones
@ 2004-04-16 21:25 ` viro
2004-04-16 21:27 ` Dave Jones
0 siblings, 1 reply; 8+ messages in thread
From: viro @ 2004-04-16 21:25 UTC (permalink / raw)
To: Dave Jones, jgarzik, Linux Kernel
On Fri, Apr 16, 2004 at 10:20:04PM +0100, Dave Jones wrote:
>
> --- linux-2.6.5/drivers/net/hamradio/baycom_par.c~ 2004-04-16 22:18:53.000000000 +0100
> +++ linux-2.6.5/drivers/net/hamradio/baycom_par.c 2004-04-16 22:19:33.000000000 +0100
> @@ -272,9 +272,13 @@
> static void par96_interrupt(int irq, void *dev_id, struct pt_regs *regs)
> {
> struct net_device *dev = (struct net_device *)dev_id;
> - struct baycom_state *bc = netdev_priv(dev);
> + struct baycom_state *bc;
>
> - if (!dev || !bc || bc->hdrv.magic != HDLCDRV_MAGIC)
> + if (!dev)
> + return;
> +
> + bc = netdev_priv(dev);
That's OK - netdev_priv(p) just adds a constant offset to p; no problem
with doing that to NULL.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: baycom_par dereference before check.
2004-04-16 21:25 ` viro
@ 2004-04-16 21:27 ` Dave Jones
2004-04-16 21:48 ` Jeff Garzik
0 siblings, 1 reply; 8+ messages in thread
From: Dave Jones @ 2004-04-16 21:27 UTC (permalink / raw)
To: viro; +Cc: jgarzik, Linux Kernel
On Fri, Apr 16, 2004 at 10:25:41PM +0100, viro@parcelfarce.linux.theplanet.co.uk wrote:
> > +++ linux-2.6.5/drivers/net/hamradio/baycom_par.c 2004-04-16 22:19:33.000000000 +0100
> > @@ -272,9 +272,13 @@
> > static void par96_interrupt(int irq, void *dev_id, struct pt_regs *regs)
> > {
> > struct net_device *dev = (struct net_device *)dev_id;
> > - struct baycom_state *bc = netdev_priv(dev);
> > + struct baycom_state *bc;
> >
> > - if (!dev || !bc || bc->hdrv.magic != HDLCDRV_MAGIC)
> > + if (!dev)
> > + return;
> > +
> > + bc = netdev_priv(dev);
>
> That's OK - netdev_priv(p) just adds a constant offset to p; no problem
> with doing that to NULL.
Good point. Still doesn't strike me as particularly nice though.
Dave
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: baycom_par dereference before check.
2004-04-16 21:27 ` Dave Jones
@ 2004-04-16 21:48 ` Jeff Garzik
2004-04-16 21:53 ` Dave Jones
0 siblings, 1 reply; 8+ messages in thread
From: Jeff Garzik @ 2004-04-16 21:48 UTC (permalink / raw)
To: Dave Jones; +Cc: viro, Linux Kernel
Dave Jones wrote:
> On Fri, Apr 16, 2004 at 10:25:41PM +0100, viro@parcelfarce.linux.theplanet.co.uk wrote:
>
> > > +++ linux-2.6.5/drivers/net/hamradio/baycom_par.c 2004-04-16 22:19:33.000000000 +0100
> > > @@ -272,9 +272,13 @@
> > > static void par96_interrupt(int irq, void *dev_id, struct pt_regs *regs)
> > > {
> > > struct net_device *dev = (struct net_device *)dev_id;
> > > - struct baycom_state *bc = netdev_priv(dev);
> > > + struct baycom_state *bc;
> > >
> > > - if (!dev || !bc || bc->hdrv.magic != HDLCDRV_MAGIC)
> > > + if (!dev)
> > > + return;
> > > +
> > > + bc = netdev_priv(dev);
> >
> > That's OK - netdev_priv(p) just adds a constant offset to p; no problem
> > with doing that to NULL.
>
> Good point. Still doesn't strike me as particularly nice though.
I would rather just remove the checks completely. The success of any of
those checks is a BUG() condition that should never occur.
Jeff
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: baycom_par dereference before check.
2004-04-16 21:48 ` Jeff Garzik
@ 2004-04-16 21:53 ` Dave Jones
2004-04-16 23:07 ` Chris Wright
2004-04-19 16:39 ` Jeff Garzik
0 siblings, 2 replies; 8+ messages in thread
From: Dave Jones @ 2004-04-16 21:53 UTC (permalink / raw)
To: Jeff Garzik; +Cc: viro, Linux Kernel
On Fri, Apr 16, 2004 at 05:48:57PM -0400, Jeff Garzik wrote:
> >Good point. Still doesn't strike me as particularly nice though.
> I would rather just remove the checks completely. The success of any of
> those checks is a BUG() condition that should never occur.
That works for me too. How about this?
Dave
--- linux-2.6.5/drivers/net/hamradio/baycom_par.c~ 2004-04-16 22:52:35.000000000 +0100
+++ linux-2.6.5/drivers/net/hamradio/baycom_par.c 2004-04-16 22:52:53.000000000 +0100
@@ -274,8 +274,8 @@
struct net_device *dev = (struct net_device *)dev_id;
struct baycom_state *bc = netdev_priv(dev);
- if (!dev || !bc || bc->hdrv.magic != HDLCDRV_MAGIC)
- return;
+ BUG_ON(!bc);
+ BUG_ON(bc->hdrv.magic != HDLCDRV_MAGIC);
baycom_int_freq(bc);
/*
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: baycom_par dereference before check.
2004-04-16 21:53 ` Dave Jones
@ 2004-04-16 23:07 ` Chris Wright
2004-04-16 23:40 ` Jeff Garzik
2004-04-19 16:39 ` Jeff Garzik
1 sibling, 1 reply; 8+ messages in thread
From: Chris Wright @ 2004-04-16 23:07 UTC (permalink / raw)
To: Dave Jones, Jeff Garzik, viro, Linux Kernel
* Dave Jones (davej@redhat.com) wrote:
> On Fri, Apr 16, 2004 at 05:48:57PM -0400, Jeff Garzik wrote:
> > >Good point. Still doesn't strike me as particularly nice though.
> > I would rather just remove the checks completely. The success of any of
> > those checks is a BUG() condition that should never occur.
>
> That works for me too. How about this?
>
> Dave
>
> --- linux-2.6.5/drivers/net/hamradio/baycom_par.c~ 2004-04-16 22:52:35.000000000 +0100
> +++ linux-2.6.5/drivers/net/hamradio/baycom_par.c 2004-04-16 22:52:53.000000000 +0100
> @@ -274,8 +274,8 @@
> struct net_device *dev = (struct net_device *)dev_id;
> struct baycom_state *bc = netdev_priv(dev);
>
> - if (!dev || !bc || bc->hdrv.magic != HDLCDRV_MAGIC)
> - return;
> + BUG_ON(!bc);
If it's adding a constant offset to dev, the bc would be non-NULL in a bug
case, no?
thanks,
-chris
--
Linux Security Modules http://lsm.immunix.org http://lsm.bkbits.net
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: baycom_par dereference before check.
2004-04-16 23:07 ` Chris Wright
@ 2004-04-16 23:40 ` Jeff Garzik
0 siblings, 0 replies; 8+ messages in thread
From: Jeff Garzik @ 2004-04-16 23:40 UTC (permalink / raw)
To: Chris Wright; +Cc: Dave Jones, viro, Linux Kernel
Chris Wright wrote:
> * Dave Jones (davej@redhat.com) wrote:
>
>>On Fri, Apr 16, 2004 at 05:48:57PM -0400, Jeff Garzik wrote:
>> > >Good point. Still doesn't strike me as particularly nice though.
>> > I would rather just remove the checks completely. The success of any of
>> > those checks is a BUG() condition that should never occur.
>>
>>That works for me too. How about this?
>>
>> Dave
>>
>>--- linux-2.6.5/drivers/net/hamradio/baycom_par.c~ 2004-04-16 22:52:35.000000000 +0100
>>+++ linux-2.6.5/drivers/net/hamradio/baycom_par.c 2004-04-16 22:52:53.000000000 +0100
>>@@ -274,8 +274,8 @@
>> struct net_device *dev = (struct net_device *)dev_id;
>> struct baycom_state *bc = netdev_priv(dev);
>>
>>- if (!dev || !bc || bc->hdrv.magic != HDLCDRV_MAGIC)
>>- return;
>>+ BUG_ON(!bc);
>
>
> If it's adding a constant offset to dev, the bc would be non-NULL in a bug
> case, no?
I'm just going to remove the checks completely.
Jeff
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: baycom_par dereference before check.
2004-04-16 21:53 ` Dave Jones
2004-04-16 23:07 ` Chris Wright
@ 2004-04-19 16:39 ` Jeff Garzik
1 sibling, 0 replies; 8+ messages in thread
From: Jeff Garzik @ 2004-04-19 16:39 UTC (permalink / raw)
To: Dave Jones; +Cc: viro, Linux Kernel
Dave Jones wrote:
> On Fri, Apr 16, 2004 at 05:48:57PM -0400, Jeff Garzik wrote:
>
> > >Good point. Still doesn't strike me as particularly nice though.
> > I would rather just remove the checks completely. The success of any of
> > those checks is a BUG() condition that should never occur.
>
> That works for me too. How about this?
>
> Dave
>
> --- linux-2.6.5/drivers/net/hamradio/baycom_par.c~ 2004-04-16 22:52:35.000000000 +0100
> +++ linux-2.6.5/drivers/net/hamradio/baycom_par.c 2004-04-16 22:52:53.000000000 +0100
> @@ -274,8 +274,8 @@
> struct net_device *dev = (struct net_device *)dev_id;
> struct baycom_state *bc = netdev_priv(dev);
>
> - if (!dev || !bc || bc->hdrv.magic != HDLCDRV_MAGIC)
> - return;
> + BUG_ON(!bc);
> + BUG_ON(bc->hdrv.magic != HDLCDRV_MAGIC);
I applied a patch which simplied removed the checks.
Jeff
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2004-04-19 16:40 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2004-04-16 21:20 baycom_par dereference before check Dave Jones
2004-04-16 21:25 ` viro
2004-04-16 21:27 ` Dave Jones
2004-04-16 21:48 ` Jeff Garzik
2004-04-16 21:53 ` Dave Jones
2004-04-16 23:07 ` Chris Wright
2004-04-16 23:40 ` Jeff Garzik
2004-04-19 16:39 ` Jeff Garzik
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox