| Message ID | 20190621213949.27018-1-ezequiel@collabora.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | [v2] PM / devfreq: Fix kernel oops on governor module load | expand |
Hi, 2019년 6월 22일 (토) 오전 6:42, Ezequiel Garcia <ezequiel@collabora.com>님이 작성: > > A bit unexpectedly (but still documented), request_module may > return a positive value, in case of a modprobe error. > This is currently causing issues in the devfreq framework. > > When a request_module exits with a positive value, we currently > return that via ERR_PTR. However, because the value is positive, > it's not a ERR_VALUE proper, and is therefore treated as a > valid struct devfreq_governor pointer, leading to a kernel oops. > > Fix this by returning -EINVAL if request_module returns a positive > value. > > Fixes: b53b0128052ff ("PM / devfreq: Fix static checker warning in try_then_request_governor") > Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com> > --- > Changes from v1: > * Rework the fix as suggested by Enric and Chanwoo, > handling the return vaue. > --- > drivers/devfreq/devfreq.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c > index 6b6991f0e873..258f70c1e48f 100644 > --- a/drivers/devfreq/devfreq.c > +++ b/drivers/devfreq/devfreq.c > @@ -257,7 +257,7 @@ static struct devfreq_governor *try_then_request_governor(const char *name) > /* Restore previous state before return */ > mutex_lock(&devfreq_list_lock); > if (err) > - return ERR_PTR(err); > + return (err < 0) ? ERR_PTR(err) : ERR_PTR(-EINVAL); > > governor = find_devfreq_governor(name); > } Thanks you for fix-up. Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com> But, you are missing the stable mailing list. In order to apply this fix-up patch,\ you have to send it to stable mailing list. Please send it.
> A bit unexpectedly (but still documented), request_module may > return a positive value, in case of a modprobe error. > This is currently causing issues in the devfreq framework. > > When a request_module exits with a positive value, we currently > return that via ERR_PTR. However, because the value is positive, > it's not a ERR_VALUE proper, and is therefore treated as a > valid struct devfreq_governor pointer, leading to a kernel oops. > > Fix this by returning -EINVAL if request_module returns a positive > value. > > Fixes: b53b0128052ff ("PM / devfreq: Fix static checker warning in try_then_request_governor") > Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com> > --- > Changes from v1: > * Rework the fix as suggested by Enric and Chanwoo, > handling the return vaue. > --- > drivers/devfreq/devfreq.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Acked-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Hello Chanwoo, On Sat, 2019-06-22 at 19:46 +0900, Chanwoo Choi wrote: > Hi, > > 2019년 6월 22일 (토) 오전 6:42, Ezequiel Garcia <ezequiel@collabora.com>님이 작성: > > A bit unexpectedly (but still documented), request_module may > > return a positive value, in case of a modprobe error. > > This is currently causing issues in the devfreq framework. > > > > When a request_module exits with a positive value, we currently > > return that via ERR_PTR. However, because the value is positive, > > it's not a ERR_VALUE proper, and is therefore treated as a > > valid struct devfreq_governor pointer, leading to a kernel oops. > > > > Fix this by returning -EINVAL if request_module returns a positive > > value. > > > > Fixes: b53b0128052ff ("PM / devfreq: Fix static checker warning in try_then_request_governor") > > Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com> > > --- > > Changes from v1: > > * Rework the fix as suggested by Enric and Chanwoo, > > handling the return vaue. > > --- > > drivers/devfreq/devfreq.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c > > index 6b6991f0e873..258f70c1e48f 100644 > > --- a/drivers/devfreq/devfreq.c > > +++ b/drivers/devfreq/devfreq.c > > @@ -257,7 +257,7 @@ static struct devfreq_governor *try_then_request_governor(const char *name) > > /* Restore previous state before return */ > > mutex_lock(&devfreq_list_lock); > > if (err) > > - return ERR_PTR(err); > > + return (err < 0) ? ERR_PTR(err) : ERR_PTR(-EINVAL); > > > > governor = find_devfreq_governor(name); > > } > > Thanks you for fix-up. > Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com> > > But, you are missing the stable mailing list. In order to apply this > fix-up patch,\ > you have to send it to stable mailing list. Please send it. > > If I understand correctly, you or any of the devfreq maintainer can simply add a Cc: stable@vger.kernel.org tag. This is documented as Option 1 in stable-kernel-rules.rst. The Acked-by and Reviewed-by tags need to be collected anyway :-) Thanks! Eze
Hi, On 19. 7. 11. 오전 3:30, Ezequiel Garcia wrote: > Hello Chanwoo, > > On Sat, 2019-06-22 at 19:46 +0900, Chanwoo Choi wrote: >> Hi, >> >> 2019년 6월 22일 (토) 오전 6:42, Ezequiel Garcia <ezequiel@collabora.com>님이 작성: >>> A bit unexpectedly (but still documented), request_module may >>> return a positive value, in case of a modprobe error. >>> This is currently causing issues in the devfreq framework. >>> >>> When a request_module exits with a positive value, we currently >>> return that via ERR_PTR. However, because the value is positive, >>> it's not a ERR_VALUE proper, and is therefore treated as a >>> valid struct devfreq_governor pointer, leading to a kernel oops. >>> >>> Fix this by returning -EINVAL if request_module returns a positive >>> value. >>> >>> Fixes: b53b0128052ff ("PM / devfreq: Fix static checker warning in try_then_request_governor") >>> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com> >>> --- >>> Changes from v1: >>> * Rework the fix as suggested by Enric and Chanwoo, >>> handling the return vaue. >>> --- >>> drivers/devfreq/devfreq.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c >>> index 6b6991f0e873..258f70c1e48f 100644 >>> --- a/drivers/devfreq/devfreq.c >>> +++ b/drivers/devfreq/devfreq.c >>> @@ -257,7 +257,7 @@ static struct devfreq_governor *try_then_request_governor(const char *name) >>> /* Restore previous state before return */ >>> mutex_lock(&devfreq_list_lock); >>> if (err) >>> - return ERR_PTR(err); >>> + return (err < 0) ? ERR_PTR(err) : ERR_PTR(-EINVAL); >>> >>> governor = find_devfreq_governor(name); >>> } >> >> Thanks you for fix-up. >> Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com> >> >> But, you are missing the stable mailing list. In order to apply this >> fix-up patch,\ >> you have to send it to stable mailing list. Please send it. >> >> > > If I understand correctly, you or any of the devfreq maintainer > can simply add a Cc: stable@vger.kernel.org tag. Originally, the author have to send the stable mailing list with the required style. You can do it. > > This is documented as Option 1 in stable-kernel-rules.rst. > > The Acked-by and Reviewed-by tags need to be collected anyway :-) This patch[1] was picked to devfreq.git. [1] https://git.kernel.org/pub/scm/linux/kernel/git/mzx/devfreq.git/commit/?h=for-next&id=4f065f69ebc2bdf1fbe224816a3c471babd370dd > > Thanks! > Eze > > >
diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c index 6b6991f0e873..258f70c1e48f 100644 --- a/drivers/devfreq/devfreq.c +++ b/drivers/devfreq/devfreq.c @@ -257,7 +257,7 @@ static struct devfreq_governor *try_then_request_governor(const char *name) /* Restore previous state before return */ mutex_lock(&devfreq_list_lock); if (err) - return ERR_PTR(err); + return (err < 0) ? ERR_PTR(err) : ERR_PTR(-EINVAL); governor = find_devfreq_governor(name); }
A bit unexpectedly (but still documented), request_module may return a positive value, in case of a modprobe error. This is currently causing issues in the devfreq framework. When a request_module exits with a positive value, we currently return that via ERR_PTR. However, because the value is positive, it's not a ERR_VALUE proper, and is therefore treated as a valid struct devfreq_governor pointer, leading to a kernel oops. Fix this by returning -EINVAL if request_module returns a positive value. Fixes: b53b0128052ff ("PM / devfreq: Fix static checker warning in try_then_request_governor") Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com> --- Changes from v1: * Rework the fix as suggested by Enric and Chanwoo, handling the return vaue. --- drivers/devfreq/devfreq.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)