2002-12-01 00:05:00

by Matt Reppert

Subject: [PATCH] Unsafe MODULE_ usage in crc32.c

Hi,

Okay, I know, it's just a library module, doesn't need to ever be unloaded
anyway. But error noise in dmesg annoys me, hence this patch.

Matt

Convert CRC32 to try_module_get; fixes an unsafe usage that
prevents unloading.


lib/crc32.c | 5 ++++-
1 files changed, 4 insertions(+), 1 deletion(-)

--- linux-2.5.50/lib/crc32.c~crc32-unsafe 2002-11-30 05:31:19.000000000 -0600
+++ linux-2.5.50-arashi/lib/crc32.c 2002-11-30 05:36:17.000000000 -0600
@@ -551,7 +551,10 @@ static int __init init_crc32(void)
rc1 = crc32init_le();
rc2 = crc32init_be();
rc = rc1 || rc2;
- if (!rc) MOD_INC_USE_COUNT;
+ if (!rc) {
+ if (!try_module_get(THIS_MODULE))
+ rc = -1;
+ }
return rc;
}


[patch ends]

2002-12-01 00:18:09

by Petr Vandrovec

Subject: Re: [PATCH] Unsafe MODULE_ usage in crc32.c

On Sat, Nov 30, 2002 at 06:12:24PM -0600, Matt Reppert wrote:
> Hi,
>
> Okay, I know, it's just a library module, doesn't need to ever be unloaded
> anyway. But error noise in dmesg annoys me, hence this patch.

Better asking Rusty, why module cannot call MOD_INC_USE_COUNT on itself
during its own init... I'm pretty sure that crc32 module knows that nobody
can unload it at this point: it is executing its own code, isn't it?
Petr Vandrovec
[email protected]

> Matt
>
> Convert CRC32 to try_module_get; fixes an unsafe usage that
> prevents unloading.
>
>
> lib/crc32.c | 5 ++++-
> 1 files changed, 4 insertions(+), 1 deletion(-)
>
> --- linux-2.5.50/lib/crc32.c~crc32-unsafe 2002-11-30 05:31:19.000000000 -0600
> +++ linux-2.5.50-arashi/lib/crc32.c 2002-11-30 05:36:17.000000000 -0600
> @@ -551,7 +551,10 @@ static int __init init_crc32(void)
> rc1 = crc32init_le();
> rc2 = crc32init_be();
> rc = rc1 || rc2;
> - if (!rc) MOD_INC_USE_COUNT;
> + if (!rc) {
> + if (!try_module_get(THIS_MODULE))
> + rc = -1;
> + }
> return rc;
> }
>
>
> [patch ends]
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

2002-12-02 11:58:15

by Arnd Bergmann

Subject: Re: [PATCH] Unsafe MODULE_ usage in crc32.c

Matt Reppert wrote:

> Okay, I know, it's just a library module, doesn't need to ever be unloaded
> anyway. But error noise in dmesg annoys me, hence this patch.

I don't think you even need to set the use count at all for crc32:
As long as another module is using it, you can't unload it because
the exported symbols are used. When those symbols are not known
to other modules, it is also safe to unload crc32.

I noticed another small problem with init_crc: if crc32init_be()
fails, the memory allocated by crc32init_le() is never freed,
see below.

Arnd <><

--- 1.5/lib/crc32.c Mon Apr 8 22:22:00 2002
+++ edited/lib/crc32.c Mon Dec 2 14:25:37 2002
@@ -547,11 +547,13 @@
*/
static int __init init_crc32(void)
{
- int rc1, rc2, rc;
- rc1 = crc32init_le();
- rc2 = crc32init_be();
- rc = rc1 || rc2;
- if (!rc) MOD_INC_USE_COUNT;
+ int rc;
+ rc = crc32init_le();
+ if (rc)
+ return rc;
+ rc = crc32init_be();
+ if (rc)
+ crc32cleanup_le();
return rc;
}

2002-12-02 15:28:15

by Alan

Subject: Re: [PATCH] Unsafe MODULE_ usage in crc32.c

On Mon, 2002-12-02 at 13:31, Arnd Bergmann wrote:
> I noticed another small problem with init_crc: if crc32init_be()
> fails, the memory allocated by crc32init_le() is never freed,
> see below.

The -ac tree solves that by compiling in the table. Saves on boot up
time too 8)