* [PATCH RESEND] driver-core: fix modparam async_probe request
@ 2016-01-15 23:42 Luis R. Rodriguez
2016-01-16 19:05 ` Dmitry Torokhov
0 siblings, 1 reply; 4+ messages in thread
From: Luis R. Rodriguez @ 2016-01-15 23:42 UTC (permalink / raw)
To: rusty; +Cc: linux-kernel, hare, dmitry.torokhov, Luis R. Rodriguez, 4.2+
From: "Luis R. Rodriguez" <mcgrof@suse.com>
Commit f2411da746985 ("driver-core: add driver module
asynchronous probe support") added async probe support,
in two forms:
* in-kernel driver specification annotation
* generic async_probe module parameter (modprobe foo async_probe)
To support the generic kernel parameter parse_args() was
extended via commit ecc8617053e0 ("module: add extra
argument for parse_params() callback") however commit
failed to f2411da746985 failed to add the required argument.
This causes a crash then whenever async_probe generic
module parameter is used. This was overlooked when the
form in which in-kernel async probe support was reworked
a bit... Fix this as originally intended.
Cc: Hannes Reinecke <hare@suse.de>
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: stable@vger.kernel.org (4.2+)
Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
---
Resending and addressing Rusty, the other patch I sent on Dec 19
was addressed to Greg by mistake. Sorry about that.
kernel/module.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/kernel/module.c b/kernel/module.c
index 8f051a106676..88100ea77c55 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -3402,16 +3402,22 @@ out:
static int unknown_module_param_cb(char *param, char *val, const char *modname,
void *arg)
{
- struct module *mod = arg;
+ struct module *mod;
int ret;
if (strcmp(param, "async_probe") == 0) {
+ mod = arg;
+ if (!mod) {
+ ret = -ENOENT;
+ goto out;
+ }
mod->async_probe_requested = true;
return 0;
}
/* Check for magic 'dyndbg' arg */
ret = ddebug_dyndbg_module_param_cb(param, val, modname);
+out:
if (ret != 0)
pr_warn("%s: unknown parameter '%s' ignored\n", modname, param);
return 0;
@@ -3515,7 +3521,7 @@ static int load_module(struct load_info *info, const char __user *uargs,
/* Module is ready to execute: parsing args may do that. */
after_dashes = parse_args(mod->name, mod->args, mod->kp, mod->num_kp,
- -32768, 32767, NULL,
+ -32768, 32767, mod,
unknown_module_param_cb);
if (IS_ERR(after_dashes)) {
err = PTR_ERR(after_dashes);
--
2.6.2
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH RESEND] driver-core: fix modparam async_probe request
2016-01-15 23:42 [PATCH RESEND] driver-core: fix modparam async_probe request Luis R. Rodriguez
@ 2016-01-16 19:05 ` Dmitry Torokhov
2016-01-16 21:21 ` Luis R. Rodriguez
0 siblings, 1 reply; 4+ messages in thread
From: Dmitry Torokhov @ 2016-01-16 19:05 UTC (permalink / raw)
To: Luis R. Rodriguez; +Cc: rusty, linux-kernel, hare, Luis R. Rodriguez, 4.2+
Hi Luis,
On Fri, Jan 15, 2016 at 03:42:17PM -0800, Luis R. Rodriguez wrote:
> From: "Luis R. Rodriguez" <mcgrof@suse.com>
>
> Commit f2411da746985 ("driver-core: add driver module
> asynchronous probe support") added async probe support,
> in two forms:
>
> * in-kernel driver specification annotation
> * generic async_probe module parameter (modprobe foo async_probe)
>
> To support the generic kernel parameter parse_args() was
> extended via commit ecc8617053e0 ("module: add extra
> argument for parse_params() callback") however commit
> failed to f2411da746985 failed to add the required argument.
>
> This causes a crash then whenever async_probe generic
> module parameter is used. This was overlooked when the
> form in which in-kernel async probe support was reworked
> a bit... Fix this as originally intended.
>
> Cc: Hannes Reinecke <hare@suse.de>
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: stable@vger.kernel.org (4.2+)
> Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
> ---
>
> Resending and addressing Rusty, the other patch I sent on Dec 19
> was addressed to Greg by mistake. Sorry about that.
>
> kernel/module.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/module.c b/kernel/module.c
> index 8f051a106676..88100ea77c55 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -3402,16 +3402,22 @@ out:
> static int unknown_module_param_cb(char *param, char *val, const char *modname,
> void *arg)
> {
> - struct module *mod = arg;
> + struct module *mod;
> int ret;
>
> if (strcmp(param, "async_probe") == 0) {
> + mod = arg;
> + if (!mod) {
> + ret = -ENOENT;
> + goto out;
> + }
Why do we need this chunk? We only call unknown_module_param_cb() from
one place and with your chunk below we do know that "mod" is never NULL.
> mod->async_probe_requested = true;
> return 0;
> }
>
> /* Check for magic 'dyndbg' arg */
> ret = ddebug_dyndbg_module_param_cb(param, val, modname);
> +out:
> if (ret != 0)
> pr_warn("%s: unknown parameter '%s' ignored\n", modname, param);
> return 0;
> @@ -3515,7 +3521,7 @@ static int load_module(struct load_info *info, const char __user *uargs,
>
> /* Module is ready to execute: parsing args may do that. */
> after_dashes = parse_args(mod->name, mod->args, mod->kp, mod->num_kp,
> - -32768, 32767, NULL,
> + -32768, 32767, mod,
I believe this is the only change that is needed.
> unknown_module_param_cb);
> if (IS_ERR(after_dashes)) {
> err = PTR_ERR(after_dashes);
> --
> 2.6.2
>
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH RESEND] driver-core: fix modparam async_probe request
2016-01-16 19:05 ` Dmitry Torokhov
@ 2016-01-16 21:21 ` Luis R. Rodriguez
2016-01-19 2:37 ` Rusty Russell
0 siblings, 1 reply; 4+ messages in thread
From: Luis R. Rodriguez @ 2016-01-16 21:21 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Rusty Russell, linux-kernel@vger.kernel.org, Hannes Reinecke,
4.2+
On Sat, Jan 16, 2016 at 11:05 AM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> Hi Luis,
>
> On Fri, Jan 15, 2016 at 03:42:17PM -0800, Luis R. Rodriguez wrote:
>> From: "Luis R. Rodriguez" <mcgrof@suse.com>
>>
>> Commit f2411da746985 ("driver-core: add driver module
>> asynchronous probe support") added async probe support,
>> in two forms:
>>
>> * in-kernel driver specification annotation
>> * generic async_probe module parameter (modprobe foo async_probe)
>>
>> To support the generic kernel parameter parse_args() was
>> extended via commit ecc8617053e0 ("module: add extra
>> argument for parse_params() callback") however commit
>> failed to f2411da746985 failed to add the required argument.
>>
>> This causes a crash then whenever async_probe generic
>> module parameter is used. This was overlooked when the
>> form in which in-kernel async probe support was reworked
>> a bit... Fix this as originally intended.
>>
>> Cc: Hannes Reinecke <hare@suse.de>
>> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>> Cc: stable@vger.kernel.org (4.2+)
>> Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
>> ---
>>
>> Resending and addressing Rusty, the other patch I sent on Dec 19
>> was addressed to Greg by mistake. Sorry about that.
>>
>> kernel/module.c | 10 ++++++++--
>> 1 file changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/kernel/module.c b/kernel/module.c
>> index 8f051a106676..88100ea77c55 100644
>> --- a/kernel/module.c
>> +++ b/kernel/module.c
>> @@ -3402,16 +3402,22 @@ out:
>> static int unknown_module_param_cb(char *param, char *val, const char *modname,
>> void *arg)
>> {
>> - struct module *mod = arg;
>> + struct module *mod;
>> int ret;
>>
>> if (strcmp(param, "async_probe") == 0) {
>> + mod = arg;
>> + if (!mod) {
>> + ret = -ENOENT;
>> + goto out;
>> + }
>
> Why do we need this chunk? We only call unknown_module_param_cb() from
> one place and with your chunk below we do know that "mod" is never NULL.
To prevent future bugs that might use this incorrectly.
>> mod->async_probe_requested = true;
>> return 0;
>> }
>>
>> /* Check for magic 'dyndbg' arg */
>> ret = ddebug_dyndbg_module_param_cb(param, val, modname);
>> +out:
>> if (ret != 0)
>> pr_warn("%s: unknown parameter '%s' ignored\n", modname, param);
>> return 0;
>> @@ -3515,7 +3521,7 @@ static int load_module(struct load_info *info, const char __user *uargs,
>>
>> /* Module is ready to execute: parsing args may do that. */
>> after_dashes = parse_args(mod->name, mod->args, mod->kp, mod->num_kp,
>> - -32768, 32767, NULL,
>> + -32768, 32767, mod,
>
> I believe this is the only change that is needed.
For the fix yes, that is true. We could split this in two. Up to Rusty.
Luis
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH RESEND] driver-core: fix modparam async_probe request
2016-01-16 21:21 ` Luis R. Rodriguez
@ 2016-01-19 2:37 ` Rusty Russell
0 siblings, 0 replies; 4+ messages in thread
From: Rusty Russell @ 2016-01-19 2:37 UTC (permalink / raw)
To: Luis R. Rodriguez, Dmitry Torokhov
Cc: linux-kernel@vger.kernel.org, Hannes Reinecke, 4.2+
"Luis R. Rodriguez" <mcgrof@do-not-panic.com> writes:
> On Sat, Jan 16, 2016 at 11:05 AM, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
>> Hi Luis,
>>
>> On Fri, Jan 15, 2016 at 03:42:17PM -0800, Luis R. Rodriguez wrote:
>>> From: "Luis R. Rodriguez" <mcgrof@suse.com>
>>>
>>> Commit f2411da746985 ("driver-core: add driver module
>>> asynchronous probe support") added async probe support,
>>> in two forms:
>>>
>>> * in-kernel driver specification annotation
>>> * generic async_probe module parameter (modprobe foo async_probe)
>>>
>>> To support the generic kernel parameter parse_args() was
>>> extended via commit ecc8617053e0 ("module: add extra
>>> argument for parse_params() callback") however commit
>>> failed to f2411da746985 failed to add the required argument.
>>>
>>> This causes a crash then whenever async_probe generic
>>> module parameter is used. This was overlooked when the
>>> form in which in-kernel async probe support was reworked
>>> a bit... Fix this as originally intended.
>>>
>>> Cc: Hannes Reinecke <hare@suse.de>
>>> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>>> Cc: stable@vger.kernel.org (4.2+)
>>> Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
>>> ---
>>>
>>> Resending and addressing Rusty, the other patch I sent on Dec 19
>>> was addressed to Greg by mistake. Sorry about that.
>>>
>>> kernel/module.c | 10 ++++++++--
>>> 1 file changed, 8 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/kernel/module.c b/kernel/module.c
>>> index 8f051a106676..88100ea77c55 100644
>>> --- a/kernel/module.c
>>> +++ b/kernel/module.c
>>> @@ -3402,16 +3402,22 @@ out:
>>> static int unknown_module_param_cb(char *param, char *val, const char *modname,
>>> void *arg)
>>> {
>>> - struct module *mod = arg;
>>> + struct module *mod;
>>> int ret;
>>>
>>> if (strcmp(param, "async_probe") == 0) {
>>> + mod = arg;
>>> + if (!mod) {
>>> + ret = -ENOENT;
>>> + goto out;
>>> + }
>>
>> Why do we need this chunk? We only call unknown_module_param_cb() from
>> one place and with your chunk below we do know that "mod" is never NULL.
>
> To prevent future bugs that might use this incorrectly.
>
>>> mod->async_probe_requested = true;
>>> return 0;
>>> }
>>>
>>> /* Check for magic 'dyndbg' arg */
>>> ret = ddebug_dyndbg_module_param_cb(param, val, modname);
>>> +out:
>>> if (ret != 0)
>>> pr_warn("%s: unknown parameter '%s' ignored\n", modname, param);
>>> return 0;
>>> @@ -3515,7 +3521,7 @@ static int load_module(struct load_info *info, const char __user *uargs,
>>>
>>> /* Module is ready to execute: parsing args may do that. */
>>> after_dashes = parse_args(mod->name, mod->args, mod->kp, mod->num_kp,
>>> - -32768, 32767, NULL,
>>> + -32768, 32767, mod,
>>
>> I believe this is the only change that is needed.
>
> For the fix yes, that is true. We could split this in two. Up to Rusty.
Confused me, too. This function is static, so it's pretty clearly
only called from one place.
I've applied the minimal fix, as below (and changed the Subject prefix
to "modules").
Thanks!
Rusty.
From: Luis R. Rodriguez <mcgrof@suse.com>
Subject: modules: fix modparam async_probe request
Commit f2411da746985 ("driver-core: add driver module
asynchronous probe support") added async probe support,
in two forms:
* in-kernel driver specification annotation
* generic async_probe module parameter (modprobe foo async_probe)
To support the generic kernel parameter parse_args() was
extended via commit ecc8617053e0 ("module: add extra
argument for parse_params() callback") however commit
failed to f2411da746985 failed to add the required argument.
This causes a crash then whenever async_probe generic
module parameter is used. This was overlooked when the
form in which in-kernel async probe support was reworked
a bit... Fix this as originally intended.
Cc: Hannes Reinecke <hare@suse.de>
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: stable@vger.kernel.org (4.2+)
Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> [minimized]
---
kernel/module.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/kernel/module.c b/kernel/module.c
index 8358f4697c0c..1ce7e0044c33 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -3496,7 +3502,7 @@ static int load_module(struct load_info *info, const char __user *uargs,
/* Module is ready to execute: parsing args may do that. */
after_dashes = parse_args(mod->name, mod->args, mod->kp, mod->num_kp,
- -32768, 32767, NULL,
+ -32768, 32767, mod,
unknown_module_param_cb);
if (IS_ERR(after_dashes)) {
err = PTR_ERR(after_dashes);
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-01-19 22:56 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-01-15 23:42 [PATCH RESEND] driver-core: fix modparam async_probe request Luis R. Rodriguez
2016-01-16 19:05 ` Dmitry Torokhov
2016-01-16 21:21 ` Luis R. Rodriguez
2016-01-19 2:37 ` Rusty Russell
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).