Skip to contentSkip to conversation

hostap: procfs fix for hostap_fw.c

PATCH In review Mathieu Chouquet-Stringer 3 participants 4 messages May 6, 2008 – May 16, 2008

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

PATCH 1/1 hostap: procfs fix for hostap_fw.c

Hello,

I've been toying with hostap_pci and found a bug that is triggered when you unload/reload the module.

If you don't have PRISM2_NO_PROCFS_DEBUG defined (which is the default, as per hostap_config.h), the driver creates a "registers" file in /proc used to dump PRISM registers (function prism2_init_local_data):

#ifndef PRISM2_NO_PROCFS_DEBUG
        create_proc_read_entry("registers", 0, local->proc,
                               prism2_registers_proc_read, local);

#endif /* PRISM2_NO_PROCFS_DEBUG */

Note that create_proc_read_entry is being called with local->proc which isn't defined at the time: "registers" ends up being created as /proc/registers.

When hostap_pci is done initializing the hardware, it calls hostap_hw_ready which in turns creates all the files in /proc through hostap_init_proc (hostap_proc.c):

        local->proc = NULL;

        if (hostap_proc == NULL) {
                printk(KERN_WARNING "%s: hostap proc directory not created\n",
                       local->dev->name);
                return;
        }

        local->proc = proc_mkdir(local->ddev->name, hostap_proc);

So far so good, well sort of... When you unload the module, prism2_free_local_data does this:

#ifndef PRISM2_NO_PROCFS_DEBUG
        if (local->proc != NULL)
                remove_proc_entry("registers", local->proc);
#endif /* PRISM2_NO_PROCFS_DEBUG */
        hostap_remove_proc(local);

Problem is local->proc has now been overwritten by hostap_init_proc meaning "registers" isn't removed. When you reload the module you get an error telling you "registers" already exists (and you then get duplicate "registers" files in /proc).

So in a nutshell "registers" should be created after calling hostap_init_proc (this way it'll be created where it belongs - /proc/net/hostap/%s) making it removable at module unload.

Please apply the patch below which does just that: it creates "register" in hostap_hw_ready right after creating all the proc entries.

Best,
Mathieu

drivers/net/wireless/hostap/hostap_hw.c +0 -4
index 7be68db..454fcb4 100644
@@ -3276,11 +3276,6 @@ while (0)
 	}
 	printk(KERN_INFO "%s: Registered netdevice %s\n", dev_info, dev->name);
 
-#ifndef PRISM2_NO_PROCFS_DEBUG
-	create_proc_read_entry("registers", 0, local->proc,
-			       prism2_registers_proc_read, local);
-#endif /* PRISM2_NO_PROCFS_DEBUG */
#1 lore raw
Quoted 4 lines · Mathieu Chouquet-Stringer

Problem is local->proc has now been overwritten by hostap_init_proc meaning "registers" isn't removed. When you reload the module you get an error telling you "registers" already exists (and you then get duplicate "registers" files in /proc).

I've seen it many times. Thanks for tracking it down! I've tested the patch with PCI and PCMCIA devices, and everything appears to be correct. /proc/registers is not created, "registers" is created under /proc/net/hostap/wlan0/, no more warnings.

Quoted 1 line

If a more concise description needs to be written, I can do it.

Signature · 2 lines
Regards,
Pavel Roskin
#2 lore raw
Quoted 12 lines · Pavel Roskin
Quoted 4 lines · Mathieu Chouquet-Stringer

Problem is local->proc has now been overwritten by hostap_init_proc meaning "registers" isn't removed. When you reload the module you get an error telling you "registers" already exists (and you then get duplicate "registers" files in /proc).

I've seen it many times. Thanks for tracking it down! I've tested the patch with PCI and PCMCIA devices, and everything appears to be correct. /proc/registers is not created, "registers" is created under /proc/net/hostap/wlan0/, no more warnings.

Quoted 5 lines
Quoted 1 line

If a more concise description needs to be written, I can do it.

If you don't mind, I would appreciate a more concise description. I appreciate the full description of the bug, but I think it is a bit much for the changelog as-is.

Thanks!

John

Signature · 2 lines
John W. Linville
[email protected]
#3 lore raw
Quoted 11 lines · John W. Linville
Quoted 6 lines · Pavel Roskin
Quoted 1 line · Mathieu Chouquet-Stringer

If a more concise description needs to be written, I can do it.

If you don't mind, I would appreciate a more concise description. I appreciate the full description of the bug, but I think it is a bit much for the changelog as-is.

What about something like this:
"registers" was wrongly created under /proc which was generating warning during module removal and warning of duplicates objects in /proc after
module reloading. The file now lies in /proc/net/hostap, where it was
meant to be.

Signature · 3 lines
Mathieu Chouquet-Stringer                           [email protected]
             The sun itself sees not till heaven clears.
               -- William Shakespeare --