public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [Patch] Pointer dereference in net/irda/ircomm/ircomm_tty.c
@ 2006-03-22 22:46 Eric Sesterhenn
  2006-03-22 23:22 ` Alexey Dobriyan
  0 siblings, 1 reply; 4+ messages in thread
From: Eric Sesterhenn @ 2006-03-22 22:46 UTC (permalink / raw)
  To: linux-kernel

hi,

this fixes coverity bugs #855 and #854. In both cases tty
is dereferenced before getting checked for NULL.
Compile tested only.

Signed-off-by: Eric Sesterhenn <snakebyte@gmx.de>

--- linux-2.6.16/net/irda/ircomm/ircomm_tty.c.orig	2006-03-22 23:40:50.000000000 +0100
+++ linux-2.6.16/net/irda/ircomm/ircomm_tty.c	2006-03-22 23:42:40.000000000 +0100
@@ -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);
 



^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Patch] Pointer dereference in net/irda/ircomm/ircomm_tty.c
  2006-03-22 22:46 [Patch] Pointer dereference in net/irda/ircomm/ircomm_tty.c Eric Sesterhenn
@ 2006-03-22 23:22 ` Alexey Dobriyan
  2006-03-23 19:01   ` Eric Sesterhenn
  2006-04-05 13:17   ` Adrian Bunk
  0 siblings, 2 replies; 4+ messages in thread
From: Alexey Dobriyan @ 2006-03-22 23:22 UTC (permalink / raw)
  To: Eric Sesterhenn; +Cc: linux-kernel

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.

> --- 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);


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Patch] Pointer dereference in net/irda/ircomm/ircomm_tty.c
  2006-03-22 23:22 ` Alexey Dobriyan
@ 2006-03-23 19:01   ` Eric Sesterhenn
  2006-04-05 13:17   ` Adrian Bunk
  1 sibling, 0 replies; 4+ messages in thread
From: Eric Sesterhenn @ 2006-03-23 19:01 UTC (permalink / raw)
  To: Alexey Dobriyan; +Cc: linux-kernel

hi,

On Thu, 2006-03-23 at 02:22 +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,

I know you prefer doing it yourself :)

> IMO, what should be done is removing asserts checking for "self",
> because ->driver_data is filled in ircomm_tty_open() with valid pointer.

Updated patch below.

Signed-off-by: Eric Sesterhenn <snakebyte@gmx.de>

--- linux-2.6.16-git6/net/irda/ircomm/ircomm_tty.c.orig	2006-03-23 19:58:50.000000000 +0100
+++ linux-2.6.16-git6/net/irda/ircomm/ircomm_tty.c	2006-03-23 19:59:31.000000000 +0100
@@ -501,7 +501,6 @@ static void ircomm_tty_close(struct tty_
 	if (!tty)
 		return;
 
-	IRDA_ASSERT(self != NULL, return;);
 	IRDA_ASSERT(self->magic == IRCOMM_TTY_MAGIC, return;);
 
 	spin_lock_irqsave(&self->spinlock, flags);
@@ -1011,7 +1010,6 @@ static void ircomm_tty_hangup(struct tty
 
 	IRDA_DEBUG(0, "%s()\n", __FUNCTION__ );
 
-	IRDA_ASSERT(self != NULL, return;);
 	IRDA_ASSERT(self->magic == IRCOMM_TTY_MAGIC, return;);
 
 	if (!tty)



^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Patch] Pointer dereference in net/irda/ircomm/ircomm_tty.c
  2006-03-22 23:22 ` Alexey Dobriyan
  2006-03-23 19:01   ` Eric Sesterhenn
@ 2006-04-05 13:17   ` Adrian Bunk
  1 sibling, 0 replies; 4+ messages in thread
From: Adrian Bunk @ 2006-04-05 13:17 UTC (permalink / raw)
  To: Alexey Dobriyan; +Cc: Eric Sesterhenn, linux-kernel

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


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2006-04-05 13:17 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-03-22 22:46 [Patch] Pointer dereference in net/irda/ircomm/ircomm_tty.c Eric Sesterhenn
2006-03-22 23:22 ` Alexey Dobriyan
2006-03-23 19:01   ` Eric Sesterhenn
2006-04-05 13:17   ` Adrian Bunk

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox