diff mbox series

PM / devfreq: try_then_request_governor should not return NULL

Message ID 20190523215853.16622-1-robdclark@gmail.com (mailing list archive)
State Not Applicable, archived
Headers show
Series PM / devfreq: try_then_request_governor should not return NULL | expand

Commit Message

Rob Clark May 23, 2019, 9:58 p.m. UTC 
From: Rob Clark <robdclark@chromium.org>

The two spots it is called expect either an IS_ERR() or a valid pointer,
but not NULL.

Fixes this crash that I came across:

   Unable to handle kernel NULL pointer dereference at virtual address 0000000000000030
   Mem abort info:
     ESR = 0x96000005
     Exception class = DABT (current EL), IL = 32 bits
     SET = 0, FnV = 0
     EA = 0, S1PTW = 0
   Data abort info:
     ISV = 0, ISS = 0x00000005
     CM = 0, WnR = 0
   [0000000000000030] user address but active_mm is swapper
   Internal error: Oops: 96000005 [#1] PREEMPT SMP
   Modules linked in:
   Process kworker/2:1 (pid: 212, stack limit = 0x(____ptrval____))
   CPU: 2 PID: 212 Comm: kworker/2:1 Not tainted 5.1.0-43338-g460e6984675c-dirty #54
   Hardware name: Google Cheza (rev3+) (DT)
   Workqueue: events deferred_probe_work_func
   pstate: 00c00009 (nzcv daif +PAN +UAO)
   pc : devfreq_add_device+0x2e4/0x410
   lr : devfreq_add_device+0x2d4/0x410
   sp : ffffff8013d93740
   x29: ffffff8013d93790 x28: ffffffc0f54f8670
   x27: 0000000000000001 x26: 0000000000000007
   x25: ffffff80124abfd8 x24: 0000000000000000
   x23: ffffffc0fabc4048 x22: ffffffc0fabc4388
   x21: ffffffc0fabc4010 x20: ffffffc0fa243010
   x19: ffffffc0fabc4000 x18: 0000000091c3d373
   x17: 0000000000000400 x16: 000000000000001a
   x15: 000000019e06d400 x14: 0000000000000001
   x13: 0000000000000000 x12: 00000000000006b6
   x11: 0000000000000000 x10: 0000000000000000
   x9 : ffffffc0fa18ba00 x8 : 0000000000000000
   x7 : 0000000000000000 x6 : ffffff80127a3d9a
   x5 : ffffff8013d93550 x4 : 0000000000000000
   x3 : 0000000000000000 x2 : 0000000000000000
   x1 : 00000000000000fe x0 : 0000000000000000
   Call trace:
    devfreq_add_device+0x2e4/0x410
    devm_devfreq_add_device+0x64/0xac
    msm_gpu_init+0x320/0x5c0
    adreno_gpu_init+0x21c/0x274
    a6xx_gpu_init+0x68/0xf4
    adreno_bind+0x158/0x284
    component_bind_all+0x110/0x204
    msm_drm_bind+0x118/0x5b8
    try_to_bring_up_master+0x15c/0x19c
    component_master_add_with_match+0xb4/0xec
    msm_pdev_probe+0x1f0/0x27c
    platform_drv_probe+0x90/0xb0
    really_probe+0x120/0x298
    driver_probe_device+0x64/0xfc
    __device_attach_driver+0x8c/0xa4
    bus_for_each_drv+0x88/0xd0
    __device_attach+0xac/0x134
    device_initial_probe+0x20/0x2c
    bus_probe_device+0x34/0x90
    deferred_probe_work_func+0x74/0xac
    process_one_work+0x210/0x428
    worker_thread+0x278/0x3e4
    kthread+0x120/0x130
    ret_from_fork+0x10/0x18
   Code: aa0003f8 b13ffc1f 54000762 f901c278 (f9401b08)
   ---[ end trace a6ecc18ce5894375 ]---
   Kernel panic - not syncing: Fatal exception

Signed-off-by: Rob Clark <robdclark@chromium.org>
---
 drivers/devfreq/devfreq.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Chanwoo Choi May 24, 2019, 7:15 a.m. UTC | #1 
Hi,

This issue[1] is already fixed on latest linux.git 
You can check it. Thanks.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=b53b0128052ffd687797d5f4deeb76327e7b5711

Regards,
Chanwoo Choi


On 19. 5. 24. 오전 6:58, Rob Clark wrote:
> From: Rob Clark <robdclark@chromium.org>
> 
> The two spots it is called expect either an IS_ERR() or a valid pointer,
> but not NULL.
> 
> Fixes this crash that I came across:
> 
>    Unable to handle kernel NULL pointer dereference at virtual address 0000000000000030
>    Mem abort info:
>      ESR = 0x96000005
>      Exception class = DABT (current EL), IL = 32 bits
>      SET = 0, FnV = 0
>      EA = 0, S1PTW = 0
>    Data abort info:
>      ISV = 0, ISS = 0x00000005
>      CM = 0, WnR = 0
>    [0000000000000030] user address but active_mm is swapper
>    Internal error: Oops: 96000005 [#1] PREEMPT SMP
>    Modules linked in:
>    Process kworker/2:1 (pid: 212, stack limit = 0x(____ptrval____))
>    CPU: 2 PID: 212 Comm: kworker/2:1 Not tainted 5.1.0-43338-g460e6984675c-dirty #54
>    Hardware name: Google Cheza (rev3+) (DT)
>    Workqueue: events deferred_probe_work_func
>    pstate: 00c00009 (nzcv daif +PAN +UAO)
>    pc : devfreq_add_device+0x2e4/0x410
>    lr : devfreq_add_device+0x2d4/0x410
>    sp : ffffff8013d93740
>    x29: ffffff8013d93790 x28: ffffffc0f54f8670
>    x27: 0000000000000001 x26: 0000000000000007
>    x25: ffffff80124abfd8 x24: 0000000000000000
>    x23: ffffffc0fabc4048 x22: ffffffc0fabc4388
>    x21: ffffffc0fabc4010 x20: ffffffc0fa243010
>    x19: ffffffc0fabc4000 x18: 0000000091c3d373
>    x17: 0000000000000400 x16: 000000000000001a
>    x15: 000000019e06d400 x14: 0000000000000001
>    x13: 0000000000000000 x12: 00000000000006b6
>    x11: 0000000000000000 x10: 0000000000000000
>    x9 : ffffffc0fa18ba00 x8 : 0000000000000000
>    x7 : 0000000000000000 x6 : ffffff80127a3d9a
>    x5 : ffffff8013d93550 x4 : 0000000000000000
>    x3 : 0000000000000000 x2 : 0000000000000000
>    x1 : 00000000000000fe x0 : 0000000000000000
>    Call trace:
>     devfreq_add_device+0x2e4/0x410
>     devm_devfreq_add_device+0x64/0xac
>     msm_gpu_init+0x320/0x5c0
>     adreno_gpu_init+0x21c/0x274
>     a6xx_gpu_init+0x68/0xf4
>     adreno_bind+0x158/0x284
>     component_bind_all+0x110/0x204
>     msm_drm_bind+0x118/0x5b8
>     try_to_bring_up_master+0x15c/0x19c
>     component_master_add_with_match+0xb4/0xec
>     msm_pdev_probe+0x1f0/0x27c
>     platform_drv_probe+0x90/0xb0
>     really_probe+0x120/0x298
>     driver_probe_device+0x64/0xfc
>     __device_attach_driver+0x8c/0xa4
>     bus_for_each_drv+0x88/0xd0
>     __device_attach+0xac/0x134
>     device_initial_probe+0x20/0x2c
>     bus_probe_device+0x34/0x90
>     deferred_probe_work_func+0x74/0xac
>     process_one_work+0x210/0x428
>     worker_thread+0x278/0x3e4
>     kthread+0x120/0x130
>     ret_from_fork+0x10/0x18
>    Code: aa0003f8 b13ffc1f 54000762 f901c278 (f9401b08)
>    ---[ end trace a6ecc18ce5894375 ]---
>    Kernel panic - not syncing: Fatal exception
> 
> Signed-off-by: Rob Clark <robdclark@chromium.org>
> ---
>  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 0ae3de76833b..d29f66f0e52a 100644
> --- a/drivers/devfreq/devfreq.c
> +++ b/drivers/devfreq/devfreq.c
> @@ -254,7 +254,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 NULL;
> +			return ERR_PTR(err);
>  
>  		governor = find_devfreq_governor(name);
>  	}
>
Rob Clark May 24, 2019, 1:15 p.m. UTC | #2 
Ahh, thanks, I've not moved to the latest -rc yet..

That commit would be a good candidate for 5.1.y stable branch

BR,
-R

On Fri, May 24, 2019 at 12:13 AM Chanwoo Choi <cw00.choi@samsung.com> wrote:
>
> Hi,
>
> This issue[1] is already fixed on latest linux.git
> You can check it. Thanks.
>
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=b53b0128052ffd687797d5f4deeb76327e7b5711
>
> Regards,
> Chanwoo Choi
>
>
> On 19. 5. 24. 오전 6:58, Rob Clark wrote:
> > From: Rob Clark <robdclark@chromium.org>
> >
> > The two spots it is called expect either an IS_ERR() or a valid pointer,
> > but not NULL.
> >
> > Fixes this crash that I came across:
> >
> >    Unable to handle kernel NULL pointer dereference at virtual address 0000000000000030
> >    Mem abort info:
> >      ESR = 0x96000005
> >      Exception class = DABT (current EL), IL = 32 bits
> >      SET = 0, FnV = 0
> >      EA = 0, S1PTW = 0
> >    Data abort info:
> >      ISV = 0, ISS = 0x00000005
> >      CM = 0, WnR = 0
> >    [0000000000000030] user address but active_mm is swapper
> >    Internal error: Oops: 96000005 [#1] PREEMPT SMP
> >    Modules linked in:
> >    Process kworker/2:1 (pid: 212, stack limit = 0x(____ptrval____))
> >    CPU: 2 PID: 212 Comm: kworker/2:1 Not tainted 5.1.0-43338-g460e6984675c-dirty #54
> >    Hardware name: Google Cheza (rev3+) (DT)
> >    Workqueue: events deferred_probe_work_func
> >    pstate: 00c00009 (nzcv daif +PAN +UAO)
> >    pc : devfreq_add_device+0x2e4/0x410
> >    lr : devfreq_add_device+0x2d4/0x410
> >    sp : ffffff8013d93740
> >    x29: ffffff8013d93790 x28: ffffffc0f54f8670
> >    x27: 0000000000000001 x26: 0000000000000007
> >    x25: ffffff80124abfd8 x24: 0000000000000000
> >    x23: ffffffc0fabc4048 x22: ffffffc0fabc4388
> >    x21: ffffffc0fabc4010 x20: ffffffc0fa243010
> >    x19: ffffffc0fabc4000 x18: 0000000091c3d373
> >    x17: 0000000000000400 x16: 000000000000001a
> >    x15: 000000019e06d400 x14: 0000000000000001
> >    x13: 0000000000000000 x12: 00000000000006b6
> >    x11: 0000000000000000 x10: 0000000000000000
> >    x9 : ffffffc0fa18ba00 x8 : 0000000000000000
> >    x7 : 0000000000000000 x6 : ffffff80127a3d9a
> >    x5 : ffffff8013d93550 x4 : 0000000000000000
> >    x3 : 0000000000000000 x2 : 0000000000000000
> >    x1 : 00000000000000fe x0 : 0000000000000000
> >    Call trace:
> >     devfreq_add_device+0x2e4/0x410
> >     devm_devfreq_add_device+0x64/0xac
> >     msm_gpu_init+0x320/0x5c0
> >     adreno_gpu_init+0x21c/0x274
> >     a6xx_gpu_init+0x68/0xf4
> >     adreno_bind+0x158/0x284
> >     component_bind_all+0x110/0x204
> >     msm_drm_bind+0x118/0x5b8
> >     try_to_bring_up_master+0x15c/0x19c
> >     component_master_add_with_match+0xb4/0xec
> >     msm_pdev_probe+0x1f0/0x27c
> >     platform_drv_probe+0x90/0xb0
> >     really_probe+0x120/0x298
> >     driver_probe_device+0x64/0xfc
> >     __device_attach_driver+0x8c/0xa4
> >     bus_for_each_drv+0x88/0xd0
> >     __device_attach+0xac/0x134
> >     device_initial_probe+0x20/0x2c
> >     bus_probe_device+0x34/0x90
> >     deferred_probe_work_func+0x74/0xac
> >     process_one_work+0x210/0x428
> >     worker_thread+0x278/0x3e4
> >     kthread+0x120/0x130
> >     ret_from_fork+0x10/0x18
> >    Code: aa0003f8 b13ffc1f 54000762 f901c278 (f9401b08)
> >    ---[ end trace a6ecc18ce5894375 ]---
> >    Kernel panic - not syncing: Fatal exception
> >
> > Signed-off-by: Rob Clark <robdclark@chromium.org>
> > ---
> >  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 0ae3de76833b..d29f66f0e52a 100644
> > --- a/drivers/devfreq/devfreq.c
> > +++ b/drivers/devfreq/devfreq.c
> > @@ -254,7 +254,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 NULL;
> > +                     return ERR_PTR(err);
> >
> >               governor = find_devfreq_governor(name);
> >       }
> >
>
>
> --
> Best Regards,
> Chanwoo Choi
> Samsung Electronics
Chanwoo Choi May 28, 2019, 1:18 a.m. UTC | #3 
Hi Rob,

You're right. It have to be posted to stable@vger.kernel.org.
As you recommended, I send it[1] to stable@vger.kernel.org for fixup.
[1]https://lkml.org/lkml/2019/5/27/547

Best Regards,
Chanwoo Choi

On 19. 5. 24. 오후 10:15, Rob Clark wrote:
> Ahh, thanks, I've not moved to the latest -rc yet..
> 
> That commit would be a good candidate for 5.1.y stable branch
> 
> BR,
> -R
> 
> On Fri, May 24, 2019 at 12:13 AM Chanwoo Choi <cw00.choi@samsung.com> wrote:
>>
>> Hi,
>>
>> This issue[1] is already fixed on latest linux.git
>> You can check it. Thanks.
>>
>> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=b53b0128052ffd687797d5f4deeb76327e7b5711
>>
>> Regards,
>> Chanwoo Choi
>>
>>
>> On 19. 5. 24. 오전 6:58, Rob Clark wrote:
>>> From: Rob Clark <robdclark@chromium.org>
>>>
>>> The two spots it is called expect either an IS_ERR() or a valid pointer,
>>> but not NULL.
>>>
>>> Fixes this crash that I came across:
>>>
>>>    Unable to handle kernel NULL pointer dereference at virtual address 0000000000000030
>>>    Mem abort info:
>>>      ESR = 0x96000005
>>>      Exception class = DABT (current EL), IL = 32 bits
>>>      SET = 0, FnV = 0
>>>      EA = 0, S1PTW = 0
>>>    Data abort info:
>>>      ISV = 0, ISS = 0x00000005
>>>      CM = 0, WnR = 0
>>>    [0000000000000030] user address but active_mm is swapper
>>>    Internal error: Oops: 96000005 [#1] PREEMPT SMP
>>>    Modules linked in:
>>>    Process kworker/2:1 (pid: 212, stack limit = 0x(____ptrval____))
>>>    CPU: 2 PID: 212 Comm: kworker/2:1 Not tainted 5.1.0-43338-g460e6984675c-dirty #54
>>>    Hardware name: Google Cheza (rev3+) (DT)
>>>    Workqueue: events deferred_probe_work_func
>>>    pstate: 00c00009 (nzcv daif +PAN +UAO)
>>>    pc : devfreq_add_device+0x2e4/0x410
>>>    lr : devfreq_add_device+0x2d4/0x410
>>>    sp : ffffff8013d93740
>>>    x29: ffffff8013d93790 x28: ffffffc0f54f8670
>>>    x27: 0000000000000001 x26: 0000000000000007
>>>    x25: ffffff80124abfd8 x24: 0000000000000000
>>>    x23: ffffffc0fabc4048 x22: ffffffc0fabc4388
>>>    x21: ffffffc0fabc4010 x20: ffffffc0fa243010
>>>    x19: ffffffc0fabc4000 x18: 0000000091c3d373
>>>    x17: 0000000000000400 x16: 000000000000001a
>>>    x15: 000000019e06d400 x14: 0000000000000001
>>>    x13: 0000000000000000 x12: 00000000000006b6
>>>    x11: 0000000000000000 x10: 0000000000000000
>>>    x9 : ffffffc0fa18ba00 x8 : 0000000000000000
>>>    x7 : 0000000000000000 x6 : ffffff80127a3d9a
>>>    x5 : ffffff8013d93550 x4 : 0000000000000000
>>>    x3 : 0000000000000000 x2 : 0000000000000000
>>>    x1 : 00000000000000fe x0 : 0000000000000000
>>>    Call trace:
>>>     devfreq_add_device+0x2e4/0x410
>>>     devm_devfreq_add_device+0x64/0xac
>>>     msm_gpu_init+0x320/0x5c0
>>>     adreno_gpu_init+0x21c/0x274
>>>     a6xx_gpu_init+0x68/0xf4
>>>     adreno_bind+0x158/0x284
>>>     component_bind_all+0x110/0x204
>>>     msm_drm_bind+0x118/0x5b8
>>>     try_to_bring_up_master+0x15c/0x19c
>>>     component_master_add_with_match+0xb4/0xec
>>>     msm_pdev_probe+0x1f0/0x27c
>>>     platform_drv_probe+0x90/0xb0
>>>     really_probe+0x120/0x298
>>>     driver_probe_device+0x64/0xfc
>>>     __device_attach_driver+0x8c/0xa4
>>>     bus_for_each_drv+0x88/0xd0
>>>     __device_attach+0xac/0x134
>>>     device_initial_probe+0x20/0x2c
>>>     bus_probe_device+0x34/0x90
>>>     deferred_probe_work_func+0x74/0xac
>>>     process_one_work+0x210/0x428
>>>     worker_thread+0x278/0x3e4
>>>     kthread+0x120/0x130
>>>     ret_from_fork+0x10/0x18
>>>    Code: aa0003f8 b13ffc1f 54000762 f901c278 (f9401b08)
>>>    ---[ end trace a6ecc18ce5894375 ]---
>>>    Kernel panic - not syncing: Fatal exception
>>>
>>> Signed-off-by: Rob Clark <robdclark@chromium.org>
>>> ---
>>>  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 0ae3de76833b..d29f66f0e52a 100644
>>> --- a/drivers/devfreq/devfreq.c
>>> +++ b/drivers/devfreq/devfreq.c
>>> @@ -254,7 +254,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 NULL;
>>> +                     return ERR_PTR(err);
>>>
>>>               governor = find_devfreq_governor(name);
>>>       }
>>>
>>
>>
>> --
>> Best Regards,
>> Chanwoo Choi
>> Samsung Electronics
> 
>
diff mbox series

Patch

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index 0ae3de76833b..d29f66f0e52a 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -254,7 +254,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 NULL;
+			return ERR_PTR(err);
 
 		governor = find_devfreq_governor(name);
 	}