Skip to contentSkip to conversation

drivers/cdrom/isp16.c check_region() fix

PATCH In review Omkhar Arasaratnam 3 participants 3 messages Dec 29, 2003 Kbuild

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 drivers/cdrom/isp16.c check_region() fix

check_region is depreciated in 2.6, this replaces it with request_region

As this is my first patch to the kernel, please let me know if I did anything wrong

cdrom/isp16.c renamed from usr/src/linux/drivers/cdrom/isp16.c +3 -2
@@ -121,11 +121,12 @@
 		return (0);
 	}
 
-	if (check_region(ISP16_IO_BASE, ISP16_IO_SIZE)) {
+	if (!request_region(ISP16_IO_BASE, ISP16_IO_SIZE,"isp16")) {
 		printk("ISP16: i/o ports already in use.\n");
 		return (-EIO);
 	}
-
+	release_region(ISP16_IO_BASE, ISP16_IO_SIZE);
+	
 	if ((isp16_type = isp16_detect()) < 0) {
 		printk("ISP16: no cdrom interface found.\n");
 		return (-EIO);
#1 lore raw

cdrom/isp16.c line 128

Quoted 13 lines · Omkhar Arasaratnam
cdrom/isp16.c renamed from usr/src/linux/drivers/cdrom/isp16.c +2 -2
@@ -121,11 +121,12 @@
 		return (0);
 	}
 
-	if (check_region(ISP16_IO_BASE, ISP16_IO_SIZE)) {
+	if (!request_region(ISP16_IO_BASE, ISP16_IO_SIZE,"isp16")) {
 		printk("ISP16: i/o ports already in use.\n");
 		return (-EIO);
 	}
-
+	release_region(ISP16_IO_BASE, ISP16_IO_SIZE);

This doesn't really buy you anything. You must claim the I/O ports as long as you're possibly using them - i.e. claim them early when probing for the device, and release them only when you're done with the device.

#2 lore raw
In reply to Omkhar Arasaratnam
Quoted 3 lines · Omkhar Arasaratnam

check_region is depreciated in 2.6, this replaces it with request_region

As this is my first patch to the kernel, please let me know if I did anything wrong

It's pointless - the reason why check_region() is bad applies to your code. The problem with check_region() is simple - it gives no protection against somebody else grabbing the ports in question just as you've got "it's free" from check_region(). The same problem exists with your replacement - as soon as you've released the region it could've been grabbed by anybody.

Note that there's another problem - on rmmod we release the region we hadn't grabbed. Proper fix is
a) replace check_region with request_region
b) check that we don't have any IO on those ports before that
point (surprisingly many drivers are buggy that way - they do some IO and then go "oops, looks like somebody held these ports after all; oh, well, let's hope we hadn't screwed them too badly"). AFAICS isp16 is OK in that repect, though.
c) make sure that we release that region on all failure exits
past that point, so that insmod failure would not leave it grabbed.