linux-kernel - Re: [PATCH] lib: add module unload support to sort tests

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20171218142138.f1b84a8c1ca072b15b54af33@linux-foundation.org>
Date:   Mon, 18 Dec 2017 14:21:38 -0800
From:   Andrew Morton <akpm@...ux-foundation.org>
To:     Pravin Shedge <pravin.shedge4linux@...il.com>
Cc:     fkostenzer@...e.at, andriy.shevchenko@...ux.intel.com,
        geert@...ux-m68k.org, paul.gortmaker@...driver.com,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] lib: add module unload support to sort tests

On Sun, 17 Dec 2017 15:19:27 +0530 Pravin Shedge <pravin.shedge4linux@...il.com> wrote:

> test_sort.c perform array-based and linked list sort test. Code allows to
> compile either as a loadable modules or builtin into the kernel.
> 
> Current code is not allow to unload the test_sort.ko module after
> successful completion.
> 
> This patch add support to unload the "test_sort.ko" module.
> 
> ...
>
> --- a/lib/test_sort.c
> +++ b/lib/test_sort.c
> @@ -13,11 +13,12 @@ static int __init cmpint(const void *a, const void *b)
>  
>  static int __init test_sort_init(void)
>  {
> -	int *a, i, r = 1, err = -ENOMEM;
> +	int *a, i, r = 1;
> +	int err = -EAGAIN; /* Fail will directly unload the module */
>  
>  	a = kmalloc_array(TEST_LEN, sizeof(*a), GFP_KERNEL);
>  	if (!a)
> -		return err;
> +		return -ENOMEM;
>  
>  	for (i = 0; i < TEST_LEN; i++) {
>  		r = (r * 725861) % 6599;
> @@ -26,13 +27,12 @@ static int __init test_sort_init(void)
>  
>  	sort(a, TEST_LEN, sizeof(*a), cmpint, NULL);
>  
> -	err = -EINVAL;
>  	for (i = 0; i < TEST_LEN-1; i++)
>  		if (a[i] > a[i+1]) {
>  			pr_err("test has failed\n");
> +			err = -EINVAL;
>  			goto exit;
>  		}
> -	err = 0;
>  	pr_info("test passed\n");
>  exit:
>  	kfree(a);

I'm struggling to understand this.

It seems that the current pattern for lib/test_*.c is:

- if test fails, module_init() handler returns -EFOO
- if test succeeds, module_init() handler returns 0

So the module will be auto-unloaded if it failed and will require an
rmmod if it succeeded.

Correct?

And it appears that lib/test_sort.c currently implements the above. 
And that your patch changes it so that the module_init() handler always
returns -EFOO, so the module will be aut-unloaded whether or not the
testing passed.

Correct?

If so, why do you think we shiould alter lib/test_sort.c to behave in
this atypical fashion?