Skip to contentSkip to conversation

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

PATCH In review Rasmus Andersen 2 participants 4 messages Jun 24, 2001 – Jun 25, 2001 Char and misc drivers

lore
Share & tools
lore
#0 lore raw

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

Hi.

(My last mail to [email protected] 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.

drivers/char/ip2main.c +28 -26
@@ -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
 
Signature · 7 lines
Regards,
        Rasmus([email protected])

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..."
#1 lore raw
Quoted 4 lines · Rasmus Andersen

Hi.

(My last mail to [email protected] 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.

Quoted 81 lines

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.

drivers/char/ip2main.c +28 -26
@@ -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
 
Signature · 7 lines
Regards,
        Rasmus([email protected])

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..."
Signature · 4 lines
 Michael H. Warfield    |  (770) 985-6132   |  [email protected]
  (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!
#2 lore raw
In reply to Rasmus Andersen
Quoted 8 lines · Michael H. Warfield
Quoted 4 lines · Rasmus Andersen

Hi.

(My last mail to [email protected] 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

#3 lore raw
In reply to Rasmus Andersen

On Sun, Jun 24, 2001 at 07:14:00PM -0400, Michael H. Warfield wrote:
[...]

Quoted 6 lines

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.

[..]

Quoted 7 lines

drivers/char/ip2main.c line 887

Quoted 7 lines
drivers/char/ip2main.c +1 -5
-			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