[PATCH] Unsafe MODULE_ usage in crc32.c

[PATCH] Unsafe MODULE_ usage in crc32.c

[Matt Reppert]

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]

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

[Petr Vandrovec]

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
					vandrove@vc.cvut.cz

> 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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

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

[Arnd Bergmann]

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;
 }

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

[Alan Cox]

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)