[PATCH] Hamradio: Fix a NULL pointer dereference in net/hamradio/mkiss.c

[PATCH] Hamradio: Fix a NULL pointer dereference in net/hamradio/mkiss.c

[Eugene Teo]

Pointer ax is dereferenced before NULL check.

Coverity bug #817

Signed-off-by: Eugene Teo <eugene.teo@eugeneteo.net>

--- linux-2.6/drivers/net/hamradio/mkiss.c~	2006-03-15 10:05:35.000000000 +0800
+++ linux-2.6/drivers/net/hamradio/mkiss.c	2006-03-16 14:31:35.000000000 +0800
@@ -844,13 +844,16 @@ static void mkiss_close(struct tty_struc
 static int mkiss_ioctl(struct tty_struct *tty, struct file *file,
 	unsigned int cmd, unsigned long arg)
 {
-	struct mkiss *ax = mkiss_get(tty);
-	struct net_device *dev = ax->dev;
+	struct mkiss *ax;
+	struct net_device *dev;
 	unsigned int tmp, err;
 
 	/* First make sure we're connected. */
 	if (ax == NULL)
 		return -ENXIO;
+	
+	ax = mkiss_get(tty);
+	dev = ax->dev;
 
 	switch (cmd) {
  	case SIOCGIFNAME:

-- 
1024D/A6D12F80 print D51D 2633 8DAC 04DB 7265  9BB8 5883 6DAA A6D1 2F80
main(i) { putchar(182623909 >> (i-1) * 5&31|!!(i<7)<<6) && main(++i); }

Re: [PATCH] Hamradio: Fix a NULL pointer dereference in net/hamradio/mkiss.c

[Eugene Teo]

<quote sender="Eugene Teo">
> Pointer ax is dereferenced before NULL check.
> 
> Coverity bug #817
> 
> Signed-off-by: Eugene Teo <eugene.teo@eugeneteo.net>

Ignore the previous patch please. Here's a resend.

--
Pointer ax is dereferenced before NULL check.

Coverity bug #817

Signed-off-by: Eugene Teo <eugene.teo@eugeneteo.net>

--- linux-2.6/drivers/net/hamradio/mkiss.c~	2006-03-15 10:05:35.000000000 +0800
+++ linux-2.6/drivers/net/hamradio/mkiss.c	2006-03-16 15:06:02.000000000 +0800
@@ -845,13 +845,15 @@ static int mkiss_ioctl(struct tty_struct
 	unsigned int cmd, unsigned long arg)
 {
 	struct mkiss *ax = mkiss_get(tty);
-	struct net_device *dev = ax->dev;
+	struct net_device *dev;
 	unsigned int tmp, err;
 
 	/* First make sure we're connected. */
 	if (ax == NULL)
 		return -ENXIO;
 
+	dev = ax->dev;
+	
 	switch (cmd) {
  	case SIOCGIFNAME:
 		err = copy_to_user((void __user *) arg, ax->dev->name,

-- 
1024D/A6D12F80 print D51D 2633 8DAC 04DB 7265  9BB8 5883 6DAA A6D1 2F80
main(i) { putchar(182623909 >> (i-1) * 5&31|!!(i<7)<<6) && main(++i); }

Re: [PATCH] Hamradio: Fix a NULL pointer dereference in net/hamradio/mkiss.c

[David S. Miller]

From: Eugene Teo <eugene.teo@eugeneteo.net>
Date: Thu, 16 Mar 2006 15:07:37 +0800

> Pointer ax is dereferenced before NULL check.
> 
> Coverity bug #817
> 
> Signed-off-by: Eugene Teo <eugene.teo@eugeneteo.net>

Applied, thanks Eugene.

Re: [PATCH] Hamradio: Fix a NULL pointer dereference in net/hamradio/mkiss.c

[Alexey Dobriyan]

On Thu, Mar 16, 2006 at 03:07:37PM +0800, Eugene Teo wrote:
> Pointer ax is dereferenced before NULL check.
>
> Coverity bug #817

> --- linux-2.6/drivers/net/hamradio/mkiss.c
> +++ linux-2.6/drivers/net/hamradio/mkiss.c
> @@ -845,13 +845,15 @@ static int mkiss_ioctl(struct tty_struct
>  	unsigned int cmd, unsigned long arg)
>  {
>  	struct mkiss *ax = mkiss_get(tty);
> -	struct net_device *dev = ax->dev;
> +	struct net_device *dev;
>  	unsigned int tmp, err;
>
>  	/* First make sure we're connected. */
>  	if (ax == NULL)
>  		return -ENXIO;
>
> +	dev = ax->dev;
> +

Actual codepath, please... valid "ax" is plonked into ->disc_data in
mkiss_open().

Re: [PATCH] Hamradio: Fix a NULL pointer dereference in net/hamradio/mkiss.c

[David S. Miller]

From: Alexey Dobriyan <adobriyan@gmail.com>
Date: Thu, 16 Mar 2006 11:24:13 +0300

> Actual codepath, please... valid "ax" is plonked into ->disc_data in
> mkiss_open().

Please be more clear about what you are advocating.
I had to sit and think about what you were saying
before I could figure out that you were actually
suggesting that the NULL check need not be there to
begin with.

Thanks.

(no subject)

[Raj Kumar Yadav]

unsubscribe linux-kernel 

(no subject)

[David S. Miller]

From: "Raj Kumar Yadav" <ryadav@neomagic.com>
Date: Thu, 16 Mar 2006 15:44:27 +0530

> unsubscribe linux-kernel 

It's not as if every single list posting doesn't give you precise
directions on how to unsubscribe properly.

> 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/

Sigh...

I've removed you manually from the list, why is this so
hard?

Re: [PATCH] Hamradio: Fix a NULL pointer dereference in net/hamradio/mkiss.c

[Ralf Baechle]

On Thu, Mar 16, 2006 at 02:42:11PM +0800, Eugene Teo wrote:

> Pointer ax is dereferenced before NULL check.
> 
> Coverity bug #817

Coverity non-bug #817.  The line discipline's ioctl method can only be
called as long as sp_get(tty) is valid.  Same for mkiss.

Unless I'm wrong on the "locking rules" of the tty code that is and maybe
that unobviousness is the real reason why the patch should be applied.

  Ralf

Re: [PATCH] Hamradio: Fix a NULL pointer dereference in net/hamradio/mkiss.c

[Ralf Baechle]

On Thu, Mar 16, 2006 at 11:20:45AM +0000, Ralf Baechle wrote:

> On Thu, Mar 16, 2006 at 02:42:11PM +0800, Eugene Teo wrote:
> 
> > Pointer ax is dereferenced before NULL check.
> > 
> > Coverity bug #817
> 
> Coverity non-bug #817.  The line discipline's ioctl method can only be
> called as long as sp_get(tty) is valid.  Same for mkiss.
> 
> Unless I'm wrong on the "locking rules" of the tty code that is and maybe
> that unobviousness is the real reason why the patch should be applied.

Oh and the same applies to Coverity bug #816.

  Ralf