* [PATCH] PM / devfreq: Check NULL governor in available_governors_show @ 2019-09-23 16:34 ` Leonard Crestez 2019-09-23 18:50 ` Matthias Kaehlcke 2019-09-24 1:52 ` Chanwoo Choi 0 siblings, 2 replies; 5+ messages in thread From: Leonard Crestez @ 2019-09-23 16:34 UTC (permalink / raw) To: Matthias Kaehlcke, MyungJoo Ham, Kyungmin Park Cc: Artur Świgoń, linux-pm, Krzysztof Kozlowski, Lukasz Luba, Chanwoo Choi, linux-arm-kernel The governor is initialized after sysfs attributes become visible so in theory the governor field can be NULL here. Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com> --- drivers/devfreq/devfreq.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Found this by hacking device core to call attribute "show" functions from inside device_add. diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c index 00fc23fea5b2..896fb2312f2f 100644 --- a/drivers/devfreq/devfreq.c +++ b/drivers/devfreq/devfreq.c @@ -1322,11 +1322,11 @@ static ssize_t available_governors_show(struct device *d, /* * The devfreq with immutable governor (e.g., passive) shows * only own governor. */ - if (df->governor->immutable) { + if (df->governor && df->governor->immutable) { count = scnprintf(&buf[count], DEVFREQ_NAME_LEN, "%s ", df->governor_name); /* * The devfreq device shows the registered governor except for * immutable governors such as passive governor . -- 2.17.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] PM / devfreq: Check NULL governor in available_governors_show 2019-09-23 16:34 ` [PATCH] PM / devfreq: Check NULL governor in available_governors_show Leonard Crestez @ 2019-09-23 18:50 ` Matthias Kaehlcke 2019-09-24 1:52 ` Chanwoo Choi 1 sibling, 0 replies; 5+ messages in thread From: Matthias Kaehlcke @ 2019-09-23 18:50 UTC (permalink / raw) To: Leonard Crestez Cc: Artur Świgoń, linux-pm, Krzysztof Kozlowski, Lukasz Luba, Chanwoo Choi, Kyungmin Park, MyungJoo Ham, linux-arm-kernel On Mon, Sep 23, 2019 at 07:34:43PM +0300, Leonard Crestez wrote: > The governor is initialized after sysfs attributes become visible so in > theory the governor field can be NULL here. > > Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com> > --- > drivers/devfreq/devfreq.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > Found this by hacking device core to call attribute "show" functions > from inside device_add. > > diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c > index 00fc23fea5b2..896fb2312f2f 100644 > --- a/drivers/devfreq/devfreq.c > +++ b/drivers/devfreq/devfreq.c > @@ -1322,11 +1322,11 @@ static ssize_t available_governors_show(struct device *d, > > /* > * The devfreq with immutable governor (e.g., passive) shows > * only own governor. > */ > - if (df->governor->immutable) { > + if (df->governor && df->governor->immutable) { > count = scnprintf(&buf[count], DEVFREQ_NAME_LEN, > "%s ", df->governor_name); > /* > * The devfreq device shows the registered governor except for > * immutable governors such as passive governor . Reviewed-by: Matthias Kaehlcke <mka@chromium.org> _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] PM / devfreq: Check NULL governor in available_governors_show 2019-09-23 16:34 ` [PATCH] PM / devfreq: Check NULL governor in available_governors_show Leonard Crestez 2019-09-23 18:50 ` Matthias Kaehlcke @ 2019-09-24 1:52 ` Chanwoo Choi 2019-09-24 7:17 ` Leonard Crestez 1 sibling, 1 reply; 5+ messages in thread From: Chanwoo Choi @ 2019-09-24 1:52 UTC (permalink / raw) To: Leonard Crestez, Matthias Kaehlcke, MyungJoo Ham, Kyungmin Park Cc: Artur Świgoń, linux-pm, linux-arm-kernel, Krzysztof Kozlowski, Lukasz Luba Hi, On 19. 9. 24. 오전 1:34, Leonard Crestez wrote: > The governor is initialized after sysfs attributes become visible so in > theory the governor field can be NULL here. > > Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com> > --- > drivers/devfreq/devfreq.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > Found this by hacking device core to call attribute "show" functions > from inside device_add. > > diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c > index 00fc23fea5b2..896fb2312f2f 100644 > --- a/drivers/devfreq/devfreq.c > +++ b/drivers/devfreq/devfreq.c > @@ -1322,11 +1322,11 @@ static ssize_t available_governors_show(struct device *d, > > /* > * The devfreq with immutable governor (e.g., passive) shows > * only own governor. > */ > - if (df->governor->immutable) { > + if (df->governor && df->governor->immutable) { > count = scnprintf(&buf[count], DEVFREQ_NAME_LEN, > "%s ", df->governor_name); > /* > * The devfreq device shows the registered governor except for > * immutable governors such as passive governor . > As you mentioned, it create sysfs and then initialize the governor instance as following: device_register() device_add() device_add_attrs() creating sysfs entries. governor = try_then_request_governor(...) Thanks for fix-up. Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com> Additionally, you have to add the following 'fixes' tag and then send it to stable mailing list(stable@vger.kernel.org). - Fixes: bcf23c79c4e46 ("PM / devfreq: Fix available_governor sysfs") -- Best Regards, Chanwoo Choi Samsung Electronics _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] PM / devfreq: Check NULL governor in available_governors_show 2019-09-24 1:52 ` Chanwoo Choi @ 2019-09-24 7:17 ` Leonard Crestez 2019-09-24 7:25 ` Chanwoo Choi 0 siblings, 1 reply; 5+ messages in thread From: Leonard Crestez @ 2019-09-24 7:17 UTC (permalink / raw) To: Chanwoo Choi, Matthias Kaehlcke, MyungJoo Ham, Kyungmin Park Cc: Artur Świgoń, linux-pm, linux-arm-kernel, Krzysztof Kozlowski, Lukasz Luba On 2019-09-24 4:48 AM, Chanwoo Choi wrote: > On 19. 9. 24. 오전 1:34, Leonard Crestez wrote: >> The governor is initialized after sysfs attributes become visible so in >> theory the governor field can be NULL here. >> >> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com> >> --- >> drivers/devfreq/devfreq.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> Found this by hacking device core to call attribute "show" functions >> from inside device_add. >> >> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c >> index 00fc23fea5b2..896fb2312f2f 100644 >> --- a/drivers/devfreq/devfreq.c >> +++ b/drivers/devfreq/devfreq.c >> @@ -1322,11 +1322,11 @@ static ssize_t available_governors_show(struct device *d, >> >> /* >> * The devfreq with immutable governor (e.g., passive) shows >> * only own governor. >> */ >> - if (df->governor->immutable) { >> + if (df->governor && df->governor->immutable) { >> count = scnprintf(&buf[count], DEVFREQ_NAME_LEN, >> "%s ", df->governor_name); >> /* >> * The devfreq device shows the registered governor except for >> * immutable governors such as passive governor . >> > > As you mentioned, it create sysfs and then initialize the governor instance > as following: > > device_register() > device_add() > device_add_attrs() > creating sysfs entries. > governor = try_then_request_governor(...) > > > Thanks for fix-up. > Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com> > > Additionally, you have to add the following 'fixes' tag > and then send it to stable mailing list(stable@vger.kernel.org). > - Fixes: bcf23c79c4e46 ("PM / devfreq: Fix available_governor sysfs") OK, but I'm not sure this meets the criteria for inclusion into linux stable: * It must fix a real bug that bothers people (not a, "This could be a problem..." type thing). * No "theoretical race condition" issues, unless an explanation of how the race can be exploited is also provided. This patch fixes a theoretical race condition which has been there since the start. -- Regards, Leonard _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] PM / devfreq: Check NULL governor in available_governors_show 2019-09-24 7:17 ` Leonard Crestez @ 2019-09-24 7:25 ` Chanwoo Choi 0 siblings, 0 replies; 5+ messages in thread From: Chanwoo Choi @ 2019-09-24 7:25 UTC (permalink / raw) To: Leonard Crestez, Matthias Kaehlcke, MyungJoo Ham, Kyungmin Park Cc: Artur Świgoń, linux-pm, linux-arm-kernel, Krzysztof Kozlowski, Lukasz Luba On 19. 9. 24. 오후 4:17, Leonard Crestez wrote: > On 2019-09-24 4:48 AM, Chanwoo Choi wrote: >> On 19. 9. 24. 오전 1:34, Leonard Crestez wrote: >>> The governor is initialized after sysfs attributes become visible so in >>> theory the governor field can be NULL here. >>> >>> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com> >>> --- >>> drivers/devfreq/devfreq.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> Found this by hacking device core to call attribute "show" functions >>> from inside device_add. >>> >>> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c >>> index 00fc23fea5b2..896fb2312f2f 100644 >>> --- a/drivers/devfreq/devfreq.c >>> +++ b/drivers/devfreq/devfreq.c >>> @@ -1322,11 +1322,11 @@ static ssize_t available_governors_show(struct device *d, >>> >>> /* >>> * The devfreq with immutable governor (e.g., passive) shows >>> * only own governor. >>> */ >>> - if (df->governor->immutable) { >>> + if (df->governor && df->governor->immutable) { >>> count = scnprintf(&buf[count], DEVFREQ_NAME_LEN, >>> "%s ", df->governor_name); >>> /* >>> * The devfreq device shows the registered governor except for >>> * immutable governors such as passive governor . >>> >> >> As you mentioned, it create sysfs and then initialize the governor instance >> as following: >> >> device_register() >> device_add() >> device_add_attrs() >> creating sysfs entries. >> governor = try_then_request_governor(...) >> >> >> Thanks for fix-up. >> Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com> >> >> Additionally, you have to add the following 'fixes' tag >> and then send it to stable mailing list(stable@vger.kernel.org). >> - Fixes: bcf23c79c4e46 ("PM / devfreq: Fix available_governor sysfs") > > OK, but I'm not sure this meets the criteria for inclusion into linux > stable: > > * It must fix a real bug that bothers people (not a, "This could be a > problem..." type thing). > * No "theoretical race condition" issues, unless an explanation of how > the race can be exploited is also provided. OK. If you think that it is not necessary to send it to stable mailing list, don't send it. Thanks. > > This patch fixes a theoretical race condition which has been there since > the start. > > -- > Regards, > Leonard > -- Best Regards, Chanwoo Choi Samsung Electronics _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-09-24 7:21 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <CGME20190923163453epcas4p1f9cff7d9f1a33fabcf4c980560d6c27d@epcas4p1.samsung.com> 2019-09-23 16:34 ` [PATCH] PM / devfreq: Check NULL governor in available_governors_show Leonard Crestez 2019-09-23 18:50 ` Matthias Kaehlcke 2019-09-24 1:52 ` Chanwoo Choi 2019-09-24 7:17 ` Leonard Crestez 2019-09-24 7:25 ` Chanwoo Choi
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).