Skip to contentSkip to conversation

request_irq: avoid slash in proc directory entries

PATCH In review Olaf Hering 4 participants 8 messages Jan 5, 2005 – Jan 6, 2005

Subsystems (5)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 request_irq: avoid slash in proc directory entries

A few users of request_irq pass a string with '/'.
As a result, ls -l /proc/irq/*/* will fail to list these entries.

 drivers/input/serio/maceps2.c     |    2 +-
 drivers/macintosh/via-pmu.c       |    2 +-
 drivers/net/wan/hostess_sv11.c    |    2 +-
 include/asm-sh/mpc1211/keyboard.h |    2 +-
 include/asm-sh64/keyboard.h       |    2 +-
 sound/isa/opl3sa2.c               |    2 +-
 6 files changed, 6 insertions(+), 6 deletions(-)
drivers/input/serio/maceps2.c +1 -1
diff -purNx tags linux-2.6.10.orig/drivers/input/serio/maceps2.c linux-2.6.10-olh/drivers/input/serio/maceps2.c
@@ -90,7 +90,7 @@ static int maceps2_open(struct serio *de
 {
 	struct maceps2_data *data = (struct maceps2_data *)dev->port_data;
 
-	if (request_irq(data->irq, maceps2_interrupt, 0, "PS/2 port", dev)) {
+	if (request_irq(data->irq, maceps2_interrupt, 0, "PS2 port", dev)) {
 		printk(KERN_ERR "Could not allocate PS/2 IRQ\n");
 		return -EBUSY;
 	}
drivers/macintosh/via-pmu.c +1 -1
diff -purNx tags linux-2.6.10.orig/drivers/macintosh/via-pmu.c linux-2.6.10-olh/drivers/macintosh/via-pmu.c
@@ -418,7 +418,7 @@ static int __init via_pmu_start(void)
 	}
 
 	if (pmu_kind == PMU_KEYLARGO_BASED && gpio_irq != -1) {
-		if (request_irq(gpio_irq, gpio1_interrupt, 0, "GPIO1/ADB", (void *)0))
+		if (request_irq(gpio_irq, gpio1_interrupt, 0, "GPIO1-ADB", (void *)0))
 			printk(KERN_ERR "pmu: can't get irq %d (GPIO1)\n", gpio_irq);
 		gpio_irq_enabled = 1;
 	}
drivers/net/wan/hostess_sv11.c +1 -1
diff -purNx tags linux-2.6.10.orig/drivers/net/wan/hostess_sv11.c linux-2.6.10-olh/drivers/net/wan/hostess_sv11.c
@@ -263,7 +263,7 @@ static struct sv11_device *sv11_init(int
 	/* We want a fast IRQ for this device. Actually we'd like an even faster
 	   IRQ ;) - This is one driver RtLinux is made for */
 	   
-	if(request_irq(irq, &z8530_interrupt, SA_INTERRUPT, "Hostess SV/11", dev)<0)
+	if(request_irq(irq, &z8530_interrupt, SA_INTERRUPT, "Hostess SV-11", dev)<0)
 	{
 		printk(KERN_WARNING "hostess: IRQ %d already in use.\n", irq);
 		goto fail1;
include/asm-sh/mpc1211/keyboard.h +1 -1
diff -purNx tags linux-2.6.10.orig/include/asm-sh/mpc1211/keyboard.h linux-2.6.10-olh/include/asm-sh/mpc1211/keyboard.h
@@ -57,7 +57,7 @@ extern unsigned char pckbd_sysrq_xlate[1
 #define AUX_IRQ 12
 
 #define aux_request_irq(hand, dev_id)					\
-	request_irq(AUX_IRQ, hand, SA_SHIRQ, "PS/2 Mouse", dev_id)
+	request_irq(AUX_IRQ, hand, SA_SHIRQ, "PS2 Mouse", dev_id)
 
 #define aux_free_irq(dev_id) free_irq(AUX_IRQ, dev_id)
 
include/asm-sh64/keyboard.h +1 -1
diff -purNx tags linux-2.6.10.orig/include/asm-sh64/keyboard.h linux-2.6.10-olh/include/asm-sh64/keyboard.h
@@ -65,7 +65,7 @@ extern unsigned char pckbd_sysrq_xlate[1
 #endif
 
 #define aux_request_irq(hand, dev_id)					\
-	request_irq(AUX_IRQ, hand, SA_SHIRQ, "PS/2 Mouse", dev_id)
+	request_irq(AUX_IRQ, hand, SA_SHIRQ, "PS2 Mouse", dev_id)
 
 #define aux_free_irq(dev_id) free_irq(AUX_IRQ, dev_id)
 
sound/isa/opl3sa2.c +1 -1
diff -purNx tags linux-2.6.10.orig/sound/isa/opl3sa2.c linux-2.6.10-olh/sound/isa/opl3sa2.c
@@ -713,7 +713,7 @@ static int __devinit snd_opl3sa2_probe(i
 		chip->single_dma = 1;
 	if ((err = snd_opl3sa2_detect(chip)) < 0)
 		goto __error;
-	if (request_irq(xirq, snd_opl3sa2_interrupt, SA_INTERRUPT, "OPL3-SA2/3", (void *)chip)) {
+	if (request_irq(xirq, snd_opl3sa2_interrupt, SA_INTERRUPT, "OPL3-SA2", (void *)chip)) {
 		snd_printk(KERN_ERR "opl3sa2: can't grab IRQ %d\n", xirq);
 		err = -ENODEV;
 		goto __error;
#1 lore raw
Quoted 3 lines · Olaf Hering

A few users of request_irq pass a string with '/'.
As a result, ls -l /proc/irq/*/* will fail to list these entries.

hrm, interesting. So how do these entries appear in /proc? Do they actually have slashes in them?

I get the feeling that something somewhere should be detecting this and should be propagating an error back.

#2 lore raw
Quoted 7 lines · Andrew Morton
Quoted 3 lines · Olaf Hering

A few users of request_irq pass a string with '/'.
As a result, ls -l /proc/irq/*/* will fail to list these entries.

hrm, interesting. So how do these entries appear in /proc? Do they actually have slashes in them?

Yes, ls /proc/irq/*/* works, but ls -l does not because you have to stat() the entry. I havent looked in detail, just poked around in /proc.

Quoted 2 lines

I get the feeling that something somewhere should be detecting this and should be propagating an error back.

Yeah, a quick sanity check for procfile creation would be a good thing.

#3 lore raw

PATCH 1/1 prohibit slash in proc directory entry names

In reply to Andrew Morton
Quoted 10 lines · Andrew Morton
Quoted 3 lines · Olaf Hering

A few users of request_irq pass a string with '/'.
As a result, ls -l /proc/irq/*/* will fail to list these entries.

hrm, interesting. So how do these entries appear in /proc? Do they actually have slashes in them?

I get the feeling that something somewhere should be detecting this and should be propagating an error back.

proc_create() needs to check that the name of an entry to be created does not contain a '/' character.

To test, I hacked the ibmveth driver to try to call request_irq with a bogus "foo/bar" devname. The creation of the /proc/irq/1234/xxx entry silently fails, as intended. Perhaps the irq code should be made to check for the failure.

fs/proc/generic.c +5 -0
Index: 2.6.10/fs/proc/generic.c
===================================================================
@@ -551,6 +551,11 @@
 
 	if (!(*parent) && xlate_proc_name(name, parent, &fn) != 0)
 		goto out;
+
+	/* At this point there must not be any '/' characters beyond *fn */
+	if (strchr(fn, '/'))
+		goto out;
+
 	len = strlen(fn);
 
 	ent = kmalloc(sizeof(struct proc_dir_entry) + len + 1, GFP_KERNEL);
#4 lore raw

Re: [PATCH] prohibit slash in proc directory entry names

Quoted 17 lines · Nathan Lynch

proc_create() needs to check that the name of an entry to be created
does not contain a '/' character.

To test, I hacked the ibmveth driver to try to call request_irq with a bogus "foo/bar" devname. The creation of the /proc/irq/1234/xxx entry silently fails, as intended. Perhaps the irq code should be made to check for the failure.

Index: 2.6.10/fs/proc/generic.c
===================================================================
--- 2.6.10.orig/fs/proc/generic.c 2004-12-24 21:35:40.000000000 +0000
+++ 2.6.10/fs/proc/generic.c 2005-01-05 18:44:56.000000000 +0000
@@ -551,6 +551,11 @@

Quoted 7 lines

if (!(*parent) && xlate_proc_name(name, parent, &fn) != 0)
goto out;
+
+ /* At this point there must not be any '/' characters beyond *fn */
+ if (strchr(fn, '/'))
+ goto out;
+

hm. From a brief code-squint I don't see how the string can ever have a slash in it by this stage. Unless the caller provided a non-null *parent and we never called xlate_proc_name()?

#5 lore raw

Re: [PATCH] prohibit slash in proc directory entry names

Quoted 26 lines · Andrew Morton
Quoted 17 lines · Nathan Lynch

proc_create() needs to check that the name of an entry to be created
does not contain a '/' character.

To test, I hacked the ibmveth driver to try to call request_irq with a bogus "foo/bar" devname. The creation of the /proc/irq/1234/xxx entry silently fails, as intended. Perhaps the irq code should be made to check for the failure.

Index: 2.6.10/fs/proc/generic.c
===================================================================
--- 2.6.10.orig/fs/proc/generic.c 2004-12-24 21:35:40.000000000 +0000
+++ 2.6.10/fs/proc/generic.c 2005-01-05 18:44:56.000000000 +0000
@@ -551,6 +551,11 @@

Quoted 7 lines

if (!(*parent) && xlate_proc_name(name, parent, &fn) != 0)
goto out;
+
+ /* At this point there must not be any '/' characters beyond *fn */
+ if (strchr(fn, '/'))
+ goto out;
+

Quoted 3 lines

hm. From a brief code-squint I don't see how the string can ever have a slash in it by this stage. Unless the caller provided a non-null *parent and we never called xlate_proc_name()?

Right.

#6 lore raw
In reply to Olaf Hering

drivers/net/wan/hostess_sv11.c static struct sv11_device *sv11_init(int · line 269

Quoted 12 lines · Olaf Hering

diff -purNx tags linux-2.6.10.orig/drivers/net/wan/hostess_sv11.c linux-2.6.10-olh/drivers/net/wan/hostess_sv11.c

drivers/net/wan/hostess_sv11.c +1 -1
@@ -263,7 +263,7 @@ static struct sv11_device *sv11_init(int
 	/* We want a fast IRQ for this device. Actually we'd like an even faster
 	   IRQ ;) - This is one driver RtLinux is made for */
 
-	if(request_irq(irq, &z8530_interrupt, SA_INTERRUPT, "Hostess SV/11", dev)<0)
+	if(request_irq(irq, &z8530_interrupt, SA_INTERRUPT, "Hostess SV-11", dev)<0)
 	{
 		printk(KERN_WARNING "hostess: IRQ %d already in use.\n", irq);
 		goto fail1;

SV11 would probably be better but fine by me as the sv11 author

#7 lore raw
Quoted 8 lines · Alan Cox
Quoted 7 lines · Olaf Hering

diff -purNx tags linux-2.6.10.orig/drivers/net/wan/hostess_sv11.c linux-2.6.10-olh/drivers/net/wan/hostess_sv11.c

drivers/net/wan/hostess_sv11.c +0 -0
@@ -263,7 +263,7 @@ static struct sv11_device *sv11_init(int
 	/* We want a fast IRQ for this device. Actually we'd like an even faster
 	   IRQ ;) - This is one driver RtLinux is made for */
 
Quoted 7 lines

drivers/net/wan/hostess_sv11.c static struct sv11_device *sv11_init(int · line 269

Quoted 5 lines
drivers/net/wan/hostess_sv11.c +1 -1
-	if(request_irq(irq, &z8530_interrupt, SA_INTERRUPT, "Hostess SV/11", dev)<0)
+	if(request_irq(irq, &z8530_interrupt, SA_INTERRUPT, "Hostess SV-11", dev)<0)
 	{
 		printk(KERN_WARNING "hostess: IRQ %d already in use.\n", irq);
 		goto fail1;

SV11 would probably be better but fine by me as the sv11 author

Ok, and GPIO1-ADB could be 'GPIO1 ADB'. Andrew, want a new patch?