[PATCH] catch potential null derefs in drivers/char/ip2main.c (245ac16)

[PATCH] catch potential null derefs in drivers/char/ip2main.c (245ac16)

[Rasmus Andersen]

Hi.

(My last mail to dougm@computone.com bounced. Is there another
maintainer for drivers/char/ip2main.c somewhere?)

The patch below tries to avoid dereferencing (potential)
NULL pointers. It was reported by the Stanford team way
back and applies against 245ac16 and 246p6. It could
probably be done nicer but that would take someone that
actually understands this code.

--- linux-245-ac16-clean/drivers/char/ip2main.c	Sat May 19 20:58:17 2001
+++ linux-245-ac16/drivers/char/ip2main.c	Sun Jun 24 22:37:27 2001
@@ -866,36 +866,38 @@
 			}
 
 #ifdef	CONFIG_DEVFS_FS
-			sprintf( name, "ipl%d", i );
-			i2BoardPtrTable[i]->devfs_ipl_handle =
-				devfs_register (devfs_handle, name,
-					DEVFS_FL_DEFAULT,
-					IP2_IPL_MAJOR, 4 * i,
-					S_IRUSR | S_IWUSR | S_IRGRP | S_IFCHR,
-					&ip2_ipl, NULL);
+			if (i2BoardPtrTable[i] && pB) {
+				sprintf( name, "ipl%d", i );
+				i2BoardPtrTable[i]->devfs_ipl_handle =
+					devfs_register (devfs_handle, name,
+							DEVFS_FL_DEFAULT,
+							IP2_IPL_MAJOR, 4 * i,
+							S_IRUSR | S_IWUSR | S_IRGRP | S_IFCHR,
+							&ip2_ipl, NULL);
 
-			sprintf( name, "stat%d", i );
-			i2BoardPtrTable[i]->devfs_stat_handle =
-				devfs_register (devfs_handle, name,
-					DEVFS_FL_DEFAULT,
-					IP2_IPL_MAJOR, 4 * i + 1,
-					S_IRUSR | S_IWUSR | S_IRGRP | S_IFCHR,
-					&ip2_ipl, NULL);
+				sprintf( name, "stat%d", i );
+				i2BoardPtrTable[i]->devfs_stat_handle =
+					devfs_register (devfs_handle, name,
+							DEVFS_FL_DEFAULT,
+							IP2_IPL_MAJOR, 4 * i + 1,
+							S_IRUSR | S_IWUSR | S_IRGRP | S_IFCHR,
+							&ip2_ipl, NULL);
 
-			for ( box = 0; box < ABS_MAX_BOXES; ++box )
-			{
-			    for ( j = 0; j < ABS_BIGGEST_BOX; ++j )
-			    {
-				if ( pB->i2eChannelMap[box] & (1 << j) )
+				for ( box = 0; box < ABS_MAX_BOXES; ++box )
 				{
-				    tty_register_devfs(&ip2_tty_driver,
-					0, j + ABS_BIGGEST_BOX *
-						(box+i*ABS_MAX_BOXES));
-				    tty_register_devfs(&ip2_callout_driver,
-					0, j + ABS_BIGGEST_BOX *
-						(box+i*ABS_MAX_BOXES));
+					for ( j = 0; j < ABS_BIGGEST_BOX; ++j )
+					{
+						if ( pB->i2eChannelMap[box] & (1 << j) )
+						{
+							tty_register_devfs(&ip2_tty_driver,
+									   0, j + ABS_BIGGEST_BOX *
+									   (box+i*ABS_MAX_BOXES));
+							tty_register_devfs(&ip2_callout_driver,
+									   0, j + ABS_BIGGEST_BOX *
+									   (box+i*ABS_MAX_BOXES));
+						}
+					}
 				}
-			    }
 			}
 #endif
 
-- 
Regards,
        Rasmus(rasmus@jaquet.dk)

A chicken and an egg are lying in bed. The chicken is smoking a
cigarette with a satisfied smile on it's face and the egg is frowning
and looking a bit pissed off. The egg mutters, to no-one in particular,
"Well, I guess we answered THAT question..."

Re: [PATCH] catch potential null derefs in drivers/char/ip2main.c (245ac16)

[Michael H. Warfield]

On Sun, Jun 24, 2001 at 11:06:06PM +0200, Rasmus Andersen wrote:
> Hi.

> (My last mail to dougm@computone.com bounced. Is there another
> maintainer for drivers/char/ip2main.c somewhere?)

	I'm still here.  :-)  Just look one more line down below
Doug's line.  There I am.

	I'm responsible for the kernel / driver integration end of it
anyways.

	I'll find out what's up with Doug, but this is my issue to deal
with anyways.  And yes, I'm looking at it.  I've got a couple of other
patches on the back burner that are overdue for integration.

> The patch below tries to avoid dereferencing (potential)
> NULL pointers. It was reported by the Stanford team way
> back and applies against 245ac16 and 246p6. It could
> probably be done nicer but that would take someone that
> actually understands this code.

> --- linux-245-ac16-clean/drivers/char/ip2main.c	Sat May 19 20:58:17 2001
> +++ linux-245-ac16/drivers/char/ip2main.c	Sun Jun 24 22:37:27 2001
> @@ -866,36 +866,38 @@
>  			}
>  
>  #ifdef	CONFIG_DEVFS_FS
> -			sprintf( name, "ipl%d", i );
> -			i2BoardPtrTable[i]->devfs_ipl_handle =
> -				devfs_register (devfs_handle, name,
> -					DEVFS_FL_DEFAULT,
> -					IP2_IPL_MAJOR, 4 * i,
> -					S_IRUSR | S_IWUSR | S_IRGRP | S_IFCHR,
> -					&ip2_ipl, NULL);
> +			if (i2BoardPtrTable[i] && pB) {
> +				sprintf( name, "ipl%d", i );
> +				i2BoardPtrTable[i]->devfs_ipl_handle =
> +					devfs_register (devfs_handle, name,
> +							DEVFS_FL_DEFAULT,
> +							IP2_IPL_MAJOR, 4 * i,
> +							S_IRUSR | S_IWUSR | S_IRGRP | S_IFCHR,
> +							&ip2_ipl, NULL);
>  
> -			sprintf( name, "stat%d", i );
> -			i2BoardPtrTable[i]->devfs_stat_handle =
> -				devfs_register (devfs_handle, name,
> -					DEVFS_FL_DEFAULT,
> -					IP2_IPL_MAJOR, 4 * i + 1,
> -					S_IRUSR | S_IWUSR | S_IRGRP | S_IFCHR,
> -					&ip2_ipl, NULL);
> +				sprintf( name, "stat%d", i );
> +				i2BoardPtrTable[i]->devfs_stat_handle =
> +					devfs_register (devfs_handle, name,
> +							DEVFS_FL_DEFAULT,
> +							IP2_IPL_MAJOR, 4 * i + 1,
> +							S_IRUSR | S_IWUSR | S_IRGRP | S_IFCHR,
> +							&ip2_ipl, NULL);
>  
> -			for ( box = 0; box < ABS_MAX_BOXES; ++box )
> -			{
> -			    for ( j = 0; j < ABS_BIGGEST_BOX; ++j )
> -			    {
> -				if ( pB->i2eChannelMap[box] & (1 << j) )
> +				for ( box = 0; box < ABS_MAX_BOXES; ++box )
>  				{
> -				    tty_register_devfs(&ip2_tty_driver,
> -					0, j + ABS_BIGGEST_BOX *
> -						(box+i*ABS_MAX_BOXES));
> -				    tty_register_devfs(&ip2_callout_driver,
> -					0, j + ABS_BIGGEST_BOX *
> -						(box+i*ABS_MAX_BOXES));
> +					for ( j = 0; j < ABS_BIGGEST_BOX; ++j )
> +					{
> +						if ( pB->i2eChannelMap[box] & (1 << j) )
> +						{
> +							tty_register_devfs(&ip2_tty_driver,
> +									   0, j + ABS_BIGGEST_BOX *
> +									   (box+i*ABS_MAX_BOXES));
> +							tty_register_devfs(&ip2_callout_driver,
> +									   0, j + ABS_BIGGEST_BOX *
> +									   (box+i*ABS_MAX_BOXES));
> +						}
> +					}
>  				}
> -			    }
>  			}
>  #endif
>  
> -- 
> Regards,
>         Rasmus(rasmus@jaquet.dk)
> 
> A chicken and an egg are lying in bed. The chicken is smoking a
> cigarette with a satisfied smile on it's face and the egg is frowning
> and looking a bit pissed off. The egg mutters, to no-one in particular,
> "Well, I guess we answered THAT question..."

-- 
 Michael H. Warfield    |  (770) 985-6132   |  mhw@WittsEnd.com
  (The Mad Wizard)      |  (678) 463-0932   |  http://www.wittsend.com/mhw/
  NIC whois:  MHW9      |  An optimist believes we live in the best of all
 PGP Key: 0xDF1DD471    |  possible worlds.  A pessimist is sure of it!

Re: [PATCH] catch potential null derefs in drivers/char/ip2main.c (245ac16)

[Rasmus Andersen]

On Sun, Jun 24, 2001 at 07:14:00PM -0400, Michael H. Warfield wrote:
> On Sun, Jun 24, 2001 at 11:06:06PM +0200, Rasmus Andersen wrote:
> > Hi.
> 
> > (My last mail to dougm@computone.com bounced. Is there another
> > maintainer for drivers/char/ip2main.c somewhere?)
> 
> 	I'm still here.  :-)  Just look one more line down below
> Doug's line.  There I am.

Ah. My apoligies. I am not accustomed to the luxury of having
multiple maintainers to bug :) No slight intended.

Regards,
  Rasmus

Re: [PATCH] catch potential null derefs in drivers/char/ip2main.c (245ac16)

[Rasmus Andersen]

On Sun, Jun 24, 2001 at 07:14:00PM -0400, Michael H. Warfield wrote:
[...] 
> 	I'm responsible for the kernel / driver integration end of it
> anyways.
> 
> 	I'll find out what's up with Doug, but this is my issue to deal
> with anyways.  And yes, I'm looking at it.  I've got a couple of other
> patches on the back burner that are overdue for integration.

The pB deref below kinda bothers me. It is last set way above as
part of looping through stuff comparing it to NULL. It seems
bogus to use is as below but I have no clue what should be used
instead.

[..]
> > -			for ( box = 0; box < ABS_MAX_BOXES; ++box )
> > -			{
> > -			    for ( j = 0; j < ABS_BIGGEST_BOX; ++j )
> > -			    {
> > -				if ( pB->i2eChannelMap[box] & (1 << j) )
> > +				for ( box = 0; box < ABS_MAX_BOXES; ++box )
> >  				{

Regards,
  Rasmus