Re: [Patch] Pointer dereference in net/irda/ircomm/ircomm_tty.c

Adrian Bunk
2006-04-05T13:17:08Z

On Thu, Mar 23, 2006 at 02:22:47AM +0300, Alexey Dobriyan wrote:
> On Wed, Mar 22, 2006 at 11:46:05PM +0100, Eric Sesterhenn wrote:
> > this fixes coverity bugs #855 and #854. In both cases tty
> > is dereferenced before getting checked for NULL.
>
> Before Al will flame you,
>
> IMO, what should be done is removing asserts checking for "self",
> because ->driver_data is filled in ircomm_tty_open() with valid pointer.

That's not what the Coverity checker is warning about.

It warns that "tty" is first dereferenced and later checked for NULL.

> > --- linux-2.6.16/net/irda/ircomm/ircomm_tty.c.orig
> > +++ linux-2.6.16/net/irda/ircomm/ircomm_tty.c
> > @@ -493,7 +493,7 @@ static int ircomm_tty_open(struct tty_st
> > */
> > static void ircomm_tty_close(struct tty_struct *tty, struct file *filp)
> > {
> > - struct ircomm_tty_cb *self = (struct ircomm_tty_cb *) tty->driver_data;
> > + struct ircomm_tty_cb *self;
> > unsigned long flags;
> >
> > IRDA_DEBUG(0, "%s()\n", __FUNCTION__ );
> > @@ -501,6 +501,8 @@ static void ircomm_tty_close(struct tty_
> > if (!tty)
> > return;
> >
> > + self = (struct ircomm_tty_cb *) tty->driver_data;
> > +
> > IRDA_ASSERT(self != NULL, return;);
> > IRDA_ASSERT(self->magic == IRCOMM_TTY_MAGIC, return;);
> >
> > @@ -1006,17 +1008,19 @@ static void ircomm_tty_shutdown(struct i
> > */
> > static void ircomm_tty_hangup(struct tty_struct *tty)
> > {
> > - struct ircomm_tty_cb *self = (struct ircomm_tty_cb *) tty->driver_data;
> > + struct ircomm_tty_cb *self;
> > unsigned long flags;
> >
> > IRDA_DEBUG(0, "%s()\n", __FUNCTION__ );
> >
> > - IRDA_ASSERT(self != NULL, return;);
> > - IRDA_ASSERT(self->magic == IRCOMM_TTY_MAGIC, return;);
> > -
> > if (!tty)
> > return;
> >
> > + self = (struct ircomm_tty_cb *) tty->driver_data;
> > +
> > + IRDA_ASSERT(self != NULL, return;);
> > + IRDA_ASSERT(self->magic == IRCOMM_TTY_MAGIC, return;);
> > +
> > /* ircomm_tty_flush_buffer(tty); */
> > ircomm_tty_shutdown(self);

cu
Adrian

--

"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed