linux-kernel - Re: [RFC PATCH] Allow optional module parameters

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 for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <9F26321B-7088-4D16-80D5-72EB388F8E49@redhat.com>
Date:	Mon, 1 Jul 2013 12:33:11 -0400 (EDT)
From:	Jonathan Masters <jcm@...hat.com>
To:	Rusty Russell <rusty@...tcorp.com.au>
Cc:	Lucas De Marchi <lucas.demarchi@...fusion.mobi>,
	Andy Lutomirski <luto@...capital.net>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Ben Hutchings <bhutchings@...arflare.com>,
	"linux-kbuild@...r.kernel.org" <linux-kbuild@...r.kernel.org>
Subject: Re: [RFC PATCH] Allow optional module parameters

One caveat. Sometimes we have manufactured parameters intentionally to cause a module to fail. We should standardize that piece.

-- 
Sent from my iPad

On Jul 1, 2013, at 4:53, Rusty Russell <rusty@...tcorp.com.au> wrote:

> Rusty Russell <rusty@...tcorp.com.au> writes:
>> Lucas De Marchi <lucas.demarchi@...fusion.mobi> writes:
>>> Hi Rusty,
>>> 
>>> On Mon, Mar 18, 2013 at 11:32 PM, Rusty Russell <rusty@...tcorp.com.au> wrote:
>>>> Andy Lutomirski <luto@...capital.net> writes:
>>>>> On Sun, Mar 17, 2013 at 7:24 PM, Rusty Russell <rusty@...tcorp.com.au> wrote:
>>>>>> Andy Lutomirski <luto@...capital.net> writes:
>>>>>>> On Thu, Mar 14, 2013 at 10:03 PM, Rusty Russell <rusty@...tcorp.com.au> wrote:
>>>>>>>> Err, yes.  Don't remove module parameters, they're part of the API.  Do
>>>>>>>> you have a particular example?
>>>>>>> 
>>>>>>> So things like i915.i915_enable_ppgtt, which is there to enable
>>>>>>> something experimental, needs to stay forever once the relevant
>>>>>>> feature becomes non-experimental and non-optional?  This seems silly.
>>>> ...
>>>>>>> Having the module parameter go away while still allowing the module to
>>>>>>> load seems like a good solution (possibly with a warning in the logs
>>>>>>> so the user can eventually delete the parameter).
>>>>>> 
>>>>>> Why not do that for *every* missing parameter then?  Why have this weird
>>>>>> notation where the user must know that the parameter might one day go
>>>>>> away?
>>>>> 
>>>>> Fair enough.  What about the other approach, then?  Always warn if an
>>>>> option doesn't match (built-in or otherwise) but load the module
>>>>> anyways.
>>>> 
>>>> What does everyone think of this?  Jon, Lucas, does this match your
>>>> experience?
>>>> 
>>>> Thanks,
>>>> Rusty.
>>>> 
>>>> Subject: modules: don't fail to load on unknown parameters.
>>>> 
>>>> Although parameters are supposed to be part of the kernel API, experimental
>>>> parameters are often removed.  In addition, downgrading a kernel might cause
>>>> previously-working modules to fail to load.
>>> 
>>> I agree with this reasoning
>>> 
>>>> 
>>>> On balance, it's probably better to warn, and load the module anyway.
>>> 
>>> However loading the module anyway would bring at least one drawback:
>>> if the user made a typo when passing the option the module would load
>>> anyway and he will probably not even look in the log, since there's
>>> was no errors from modprobe.
> 
> OK, so I've had this patch on the backburner, but noone has come up with
> anything better so I'll queue it into modules-next now.
> 
> Thanks,
> Rusty.
> 
> modules: don't fail to load on unknown parameters.
> 
> Although parameters are supposed to be part of the kernel API, experimental
> parameters are often removed.  In addition, downgrading a kernel might cause
> previously-working modules to fail to load.
> 
> On balance, it's probably better to warn, and load the module anyway.
> This may let through a typo, but at least the logs will show it.
> 
> Reported-by: Andy Lutomirski <luto@...capital.net>
> Signed-off-by: Rusty Russell <rusty@...tcorp.com.au>
> 
> diff --git a/kernel/module.c b/kernel/module.c
> index 3c2c72d..46db10a 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -3206,6 +3206,17 @@ out:
>    return err;
> }
> 
> +static int unknown_module_param_cb(char *param, char *val, const char *modname)
> +{
> +    /* Check for magic 'dyndbg' arg */ 
> +    int ret = ddebug_dyndbg_module_param_cb(param, val, modname);
> +    if (ret != 0) {
> +        printk(KERN_WARNING "%s: unknown parameter '%s' ignored\n",
> +               modname, param);
> +    }
> +    return 0;
> +}
> +
> /* Allocate and load the module: note that size of section 0 is always
>    zero, and we rely on this for optional sections. */
> static int load_module(struct load_info *info, const char __user *uargs,
> @@ -3292,7 +3303,7 @@ static int load_module(struct load_info *info, const char __user *uargs,
> 
>    /* Module is ready to execute: parsing args may do that. */
>    err = parse_args(mod->name, mod->args, mod->kp, mod->num_kp,
> -             -32768, 32767, &ddebug_dyndbg_module_param_cb);
> +             -32768, 32767, unknown_module_param_cb);
>    if (err < 0)
>        goto bug_cleanup;
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/