* null ptr deref in __power_supply_is_system_supplied
@ 2022-08-20 14:16 Jason A. Donenfeld
2022-08-22 16:44 ` Rafael J. Wysocki
0 siblings, 1 reply; 12+ messages in thread
From: Jason A. Donenfeld @ 2022-08-20 14:16 UTC (permalink / raw)
To: rafael, linux-acpi
Hi,
With 963a70bee5880640d0fd83ed29dc1e7ec0d2bd4a, I'm seeing this OOOPs
from __power_supply_is_system_supplied:
BUG: kernel NULL pointer dereference, address: 0000000000000000
#PF: supervisor instruction fetch in kernel mode
#PF: error_code(0x0010) - not-present page
PGD 0 P4D 0
Oops: 0010 [#1] PREEMPT SMP
CPU: 14 PID: 1156 Comm: upowerd Tainted: G S U 6.0.0-rc1+ #366
Hardware name: LENOVO 20Y5CTO1WW/20Y5CTO1WW, BIOS N40ET36W (1.18 ) 07/19/2022
RIP: 0010:0x0
Code: Unable to access opcode bytes at RIP 0xffffffffffffffd6.
RSP: 0018:ffff88815350bd08 EFLAGS: 00010212
RAX: ffff88810207d620 RBX: ffff88815350bd7c RCX: 000000000000394e
RDX: ffff88815350bd10 RSI: 0000000000000004 RDI: ffff888111722c00
RBP: ffff88815350bd68 R08: ffff8881187a8af8 R09: ffff8881187a8af8
R10: 0000000000000000 R11: 000000000000005f R12: ffffffff8162d0b0
R13: ffff88810159a038 R14: ffffffff823b3768 R15: ffff88810159a000
FS: 00007fd1f0958140(0000) GS:ffff88901f780000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: ffffffffffffffd6 CR3: 0000000152c7a004 CR4: 0000000000770ee0
PKRU: 55555554
Call Trace:
<TASK>
__power_supply_is_system_supplied+0x26/0x40
class_for_each_device+0xa5/0xd0
? acpi_battery_get_state+0x4e/0x1f0
power_supply_is_system_supplied+0x26/0x40
acpi_battery_get_property+0x301/0x310
power_supply_show_property+0xa5/0x1d0
dev_attr_show+0x10/0x30
sysfs_kf_seq_show+0x78/0xc0
seq_read_iter+0xfd/0x3e0
vfs_read+0x1cb/0x290
ksys_read+0x4e/0xc0
do_syscall_64+0x2b/0x50
entry_SYSCALL_64_after_hwframe+0x46/0xb0
RIP: 0033:0x7fd1f0bed70c
Code: ec 28 48 89 54 24 18 48 89 74 24 10 89 7c 24 08 e8 39 a4 f8 ff 41 89 c0 48 8b 54 24 18 48 8b 74 24 10 8b 7c 24 08 31 c0 0f 05 <48> 3d 00 f0 ff ff 77 34 44 89 c7 48 89 44 24 08 e8 8f a4 f8 ff 48
RSP: 002b:00007ffc8d3f27e0 EFLAGS: 00000246 ORIG_RAX: 0000000000000000
RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007fd1f0bed70c
RDX: 0000000000001000 RSI: 000055957d534850 RDI: 000000000000000c
RBP: 000055957d50b1d0 R08: 0000000000000000 R09: 0000000000001000
R10: 000000000000006f R11: 0000000000000246 R12: 00007ffc8d3f2910
R13: 0000000000000000 R14: 0000000000000000 R15: 000000000000000c
</TASK>
Modules linked in: rfcomm snd_seq_dummy snd_hrtimer snd_seq snd_seq_device cmac algif_skcipher bnep uvcvideo videobuf2_vmalloc btusb videobuf2_memops videobuf2_v4l2 btintel videobuf2_common bluetooth video>
i2c_designware_platform i2c_designware_core snd_hda_intel i2c_algo_bit drm_buddy snd_intel_dspcfg kvm_intel spi_nor intel_gtt think_lmi snd_intel_sdw_acpi mtd mei_pxp mei_hdcp firmware_attributes_class wm>
CR2: 0000000000000000
---[ end trace 0000000000000000 ]---
RIP: 0010:0x0
Code: Unable to access opcode bytes at RIP 0xffffffffffffffd6.
RSP: 0018:ffff88815350bd08 EFLAGS: 00010212
RAX: ffff88810207d620 RBX: ffff88815350bd7c RCX: 000000000000394e
RDX: ffff88815350bd10 RSI: 0000000000000004 RDI: ffff888111722c00
RBP: ffff88815350bd68 R08: ffff8881187a8af8 R09: ffff8881187a8af8
R10: 0000000000000000 R11: 000000000000005f R12: ffffffff8162d0b0
R13: ffff88810159a038 R14: ffffffff823b3768 R15: ffff88810159a000
FS: 00007fd1f0958140(0000) GS:ffff88901f780000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: ffffffffffffffd6 CR3: 0000000152c7a004 CR4: 0000000000770ee0
PKRU: 55555554
Regards,
Jason
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: null ptr deref in __power_supply_is_system_supplied
2022-08-20 14:16 null ptr deref in __power_supply_is_system_supplied Jason A. Donenfeld
@ 2022-08-22 16:44 ` Rafael J. Wysocki
2022-08-22 17:26 ` Jason A. Donenfeld
0 siblings, 1 reply; 12+ messages in thread
From: Rafael J. Wysocki @ 2022-08-22 16:44 UTC (permalink / raw)
To: Jason A. Donenfeld; +Cc: Rafael J. Wysocki, ACPI Devel Maling List
On Sat, Aug 20, 2022 at 4:17 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
>
> Hi,
>
> With 963a70bee5880640d0fd83ed29dc1e7ec0d2bd4a, I'm seeing this OOOPs
> from __power_supply_is_system_supplied:
>
> BUG: kernel NULL pointer dereference, address: 0000000000000000
> #PF: supervisor instruction fetch in kernel mode
> #PF: error_code(0x0010) - not-present page
> PGD 0 P4D 0
> Oops: 0010 [#1] PREEMPT SMP
> CPU: 14 PID: 1156 Comm: upowerd Tainted: G S U 6.0.0-rc1+ #366
> Hardware name: LENOVO 20Y5CTO1WW/20Y5CTO1WW, BIOS N40ET36W (1.18 ) 07/19/2022
> RIP: 0010:0x0
> Code: Unable to access opcode bytes at RIP 0xffffffffffffffd6.
> RSP: 0018:ffff88815350bd08 EFLAGS: 00010212
> RAX: ffff88810207d620 RBX: ffff88815350bd7c RCX: 000000000000394e
> RDX: ffff88815350bd10 RSI: 0000000000000004 RDI: ffff888111722c00
> RBP: ffff88815350bd68 R08: ffff8881187a8af8 R09: ffff8881187a8af8
> R10: 0000000000000000 R11: 000000000000005f R12: ffffffff8162d0b0
> R13: ffff88810159a038 R14: ffffffff823b3768 R15: ffff88810159a000
> FS: 00007fd1f0958140(0000) GS:ffff88901f780000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: ffffffffffffffd6 CR3: 0000000152c7a004 CR4: 0000000000770ee0
> PKRU: 55555554
> Call Trace:
> <TASK>
> __power_supply_is_system_supplied+0x26/0x40
It looks like __power_supply_is_system_supplied() should test psy
against NULL, but I'm not sure why that wasn't necessary before.
Neither the ACPI battery nor the ACPI AC driver has changed since 5.19 FWIW.
> class_for_each_device+0xa5/0xd0
> ? acpi_battery_get_state+0x4e/0x1f0
> power_supply_is_system_supplied+0x26/0x40
> acpi_battery_get_property+0x301/0x310
> power_supply_show_property+0xa5/0x1d0
> dev_attr_show+0x10/0x30
> sysfs_kf_seq_show+0x78/0xc0
> seq_read_iter+0xfd/0x3e0
> vfs_read+0x1cb/0x290
> ksys_read+0x4e/0xc0
> do_syscall_64+0x2b/0x50
> entry_SYSCALL_64_after_hwframe+0x46/0xb0
> RIP: 0033:0x7fd1f0bed70c
> Code: ec 28 48 89 54 24 18 48 89 74 24 10 89 7c 24 08 e8 39 a4 f8 ff 41 89 c0 48 8b 54 24 18 48 8b 74 24 10 8b 7c 24 08 31 c0 0f 05 <48> 3d 00 f0 ff ff 77 34 44 89 c7 48 89 44 24 08 e8 8f a4 f8 ff 48
> RSP: 002b:00007ffc8d3f27e0 EFLAGS: 00000246 ORIG_RAX: 0000000000000000
> RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007fd1f0bed70c
> RDX: 0000000000001000 RSI: 000055957d534850 RDI: 000000000000000c
> RBP: 000055957d50b1d0 R08: 0000000000000000 R09: 0000000000001000
> R10: 000000000000006f R11: 0000000000000246 R12: 00007ffc8d3f2910
> R13: 0000000000000000 R14: 0000000000000000 R15: 000000000000000c
> </TASK>
> Modules linked in: rfcomm snd_seq_dummy snd_hrtimer snd_seq snd_seq_device cmac algif_skcipher bnep uvcvideo videobuf2_vmalloc btusb videobuf2_memops videobuf2_v4l2 btintel videobuf2_common bluetooth video>
> i2c_designware_platform i2c_designware_core snd_hda_intel i2c_algo_bit drm_buddy snd_intel_dspcfg kvm_intel spi_nor intel_gtt think_lmi snd_intel_sdw_acpi mtd mei_pxp mei_hdcp firmware_attributes_class wm>
>
> CR2: 0000000000000000
> ---[ end trace 0000000000000000 ]---
> RIP: 0010:0x0
> Code: Unable to access opcode bytes at RIP 0xffffffffffffffd6.
> RSP: 0018:ffff88815350bd08 EFLAGS: 00010212
> RAX: ffff88810207d620 RBX: ffff88815350bd7c RCX: 000000000000394e
> RDX: ffff88815350bd10 RSI: 0000000000000004 RDI: ffff888111722c00
> RBP: ffff88815350bd68 R08: ffff8881187a8af8 R09: ffff8881187a8af8
> R10: 0000000000000000 R11: 000000000000005f R12: ffffffff8162d0b0
> R13: ffff88810159a038 R14: ffffffff823b3768 R15: ffff88810159a000
> FS: 00007fd1f0958140(0000) GS:ffff88901f780000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: ffffffffffffffd6 CR3: 0000000152c7a004 CR4: 0000000000770ee0
> PKRU: 55555554
>
> Regards,
> Jason
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: null ptr deref in __power_supply_is_system_supplied
2022-08-22 16:44 ` Rafael J. Wysocki
@ 2022-08-22 17:26 ` Jason A. Donenfeld
2022-08-29 15:28 ` [PATCH] power: supply: avoid nullptr " Jason A. Donenfeld
0 siblings, 1 reply; 12+ messages in thread
From: Jason A. Donenfeld @ 2022-08-22 17:26 UTC (permalink / raw)
To: Rafael J. Wysocki; +Cc: ACPI Devel Maling List
I just reproduced in 5.19 actually. Interesting...
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH] power: supply: avoid nullptr deref in __power_supply_is_system_supplied
2022-08-22 17:26 ` Jason A. Donenfeld
@ 2022-08-29 15:28 ` Jason A. Donenfeld
2022-08-29 15:46 ` Rafael J. Wysocki
0 siblings, 1 reply; 12+ messages in thread
From: Jason A. Donenfeld @ 2022-08-29 15:28 UTC (permalink / raw)
To: ACPI Devel Maling List; +Cc: Jason A. Donenfeld, stable, Rafael J . Wysocki
Fix the following OOPS:
BUG: kernel NULL pointer dereference, address: 0000000000000000
PGD 0 P4D 0
Oops: 0010 [#1] PREEMPT SMP
CPU: 14 PID: 1156 Comm: upowerd Tainted: G S U 6.0.0-rc1+ #366
Hardware name: LENOVO 20Y5CTO1WW/20Y5CTO1WW, BIOS N40ET36W (1.18 ) 07/19/2022
RIP: 0010:0x0
Code: Unable to access opcode bytes at RIP 0xffffffffffffffd6.
RSP: 0018:ffff88815350bd08 EFLAGS: 00010212
RAX: ffff88810207d620 RBX: ffff88815350bd7c RCX: 000000000000394e
RDX: ffff88815350bd10 RSI: 0000000000000004 RDI: ffff888111722c00
RBP: ffff88815350bd68 R08: ffff8881187a8af8 R09: ffff8881187a8af8
R10: 0000000000000000 R11: 000000000000005f R12: ffffffff8162d0b0
R13: ffff88810159a038 R14: ffffffff823b3768 R15: ffff88810159a000
FS: 00007fd1f0958140(0000) GS:ffff88901f780000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: ffffffffffffffd6 CR3: 0000000152c7a004 CR4: 0000000000770ee0
PKRU: 55555554
Call Trace:
<TASK>
__power_supply_is_system_supplied+0x26/0x40
class_for_each_device+0xa5/0xd0
? acpi_battery_get_state+0x4e/0x1f0
power_supply_is_system_supplied+0x26/0x40
acpi_battery_get_property+0x301/0x310
power_supply_show_property+0xa5/0x1d0
dev_attr_show+0x10/0x30
sysfs_kf_seq_show+0x78/0xc0
seq_read_iter+0xfd/0x3e0
vfs_read+0x1cb/0x290
ksys_read+0x4e/0xc0
do_syscall_64+0x2b/0x50
entry_SYSCALL_64_after_hwframe+0x46/0xb0
RIP: 0033:0x7fd1f0bed70c
Code: ec 28 48 89 54 24 18 48 89 74 24 10 89 7c 24 08 e8 39 a4 f8 ff 41 89 c0 48 8b 54 24 18 48 8b 74 24 10 8b 7c 24 08 31 c0 0f 05 <48> 3d 00 f0 ff ff 77 34 44 89 c7 48 89 44 24 08 e8 8f a4 f8 ff 48
RSP: 002b:00007ffc8d3f27e0 EFLAGS: 00000246 ORIG_RAX: 0000000000000000
RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007fd1f0bed70c
RDX: 0000000000001000 RSI: 000055957d534850 RDI: 000000000000000c
RBP: 000055957d50b1d0 R08: 0000000000000000 R09: 0000000000001000
R10: 000000000000006f R11: 0000000000000246 R12: 00007ffc8d3f2910
R13: 0000000000000000 R14: 0000000000000000 R15: 000000000000000c
</TASK>
CR2: 0000000000000000
---[ end trace 0000000000000000 ]---
RIP: 0010:0x0
Code: Unable to access opcode bytes at RIP 0xffffffffffffffd6.
RSP: 0018:ffff88815350bd08 EFLAGS: 00010212
RAX: ffff88810207d620 RBX: ffff88815350bd7c RCX: 000000000000394e
RDX: ffff88815350bd10 RSI: 0000000000000004 RDI: ffff888111722c00
RBP: ffff88815350bd68 R08: ffff8881187a8af8 R09: ffff8881187a8af8
R10: 0000000000000000 R11: 000000000000005f R12: ffffffff8162d0b0
R13: ffff88810159a038 R14: ffffffff823b3768 R15: ffff88810159a000
FS: 00007fd1f0958140(0000) GS:ffff88901f780000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: ffffffffffffffd6 CR3: 0000000152c7a004 CR4: 0000000000770ee0
The disassembly of the top function in the stack trace is:
.text:0000000000000000 __power_supply_is_system_supplied proc near
.text:0000000000000000 ; DATA XREF: power_supply_is_system_supplied+12↓o
.text:0000000000000000
.text:0000000000000000 var_8 = qword ptr -8
.text:0000000000000000
.text:0000000000000000 sub rsp, 8
.text:0000000000000004 mov rdi, [rdi+78h]
.text:0000000000000008 inc dword ptr [rsi]
.text:000000000000000A mov [rsp+8+var_8], 0
.text:0000000000000012 mov rax, [rdi]
.text:0000000000000015 cmp dword ptr [rax+8], 1
.text:0000000000000019 jz short loc_2A
.text:000000000000001B mov rdx, rsp
.text:000000000000001E mov esi, 4
.text:0000000000000023 call qword ptr [rax+30h]
.text:0000000000000026 test eax, eax
.text:0000000000000028 jz short loc_31
.text:000000000000002A
.text:000000000000002A loc_2A: ; CODE XREF: __power_supply_is_system_supplied+19↑j
.text:000000000000002A xor eax, eax
.text:000000000000002C add rsp, 8
.text:0000000000000030 retn
.text:0000000000000031 ; ---------------------------------------------------------------------------
.text:0000000000000031
.text:0000000000000031 loc_31: ; CODE XREF: __power_supply_is_system_supplied+28↑j
.text:0000000000000031 mov eax, dword ptr [rsp+8+var_8]
.text:0000000000000034 add rsp, 8
.text:0000000000000038 retn
.text:0000000000000038 __power_supply_is_system_supplied endp
So presumably `call qword ptr [rax+30h]` is jumping to NULL.
Cc: stable@vger.kernel.org
Cc: Rafael J. Wysocki <rafael@kernel.org>
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
drivers/power/supply/power_supply_core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/power/supply/power_supply_core.c b/drivers/power/supply/power_supply_core.c
index 4b5fb172fa99..aa4c97f11588 100644
--- a/drivers/power/supply/power_supply_core.c
+++ b/drivers/power/supply/power_supply_core.c
@@ -349,7 +349,7 @@ static int __power_supply_is_system_supplied(struct device *dev, void *data)
unsigned int *count = data;
(*count)++;
- if (psy->desc->type != POWER_SUPPLY_TYPE_BATTERY)
+ if (psy->desc->type != POWER_SUPPLY_TYPE_BATTERY && psy->desc->get_property)
if (!psy->desc->get_property(psy, POWER_SUPPLY_PROP_ONLINE,
&ret))
return ret.intval;
--
2.37.2
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] power: supply: avoid nullptr deref in __power_supply_is_system_supplied
2022-08-29 15:28 ` [PATCH] power: supply: avoid nullptr " Jason A. Donenfeld
@ 2022-08-29 15:46 ` Rafael J. Wysocki
2022-09-05 17:24 ` [PATCH RESEND] " Jason A. Donenfeld
0 siblings, 1 reply; 12+ messages in thread
From: Rafael J. Wysocki @ 2022-08-29 15:46 UTC (permalink / raw)
To: Jason A. Donenfeld
Cc: ACPI Devel Maling List, Stable, Rafael J . Wysocki, Linux PM
CC: linux-pm (where the power supply subsystem is developed)
On Mon, Aug 29, 2022 at 5:28 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
>
> Fix the following OOPS:
>
> BUG: kernel NULL pointer dereference, address: 0000000000000000
> PGD 0 P4D 0
> Oops: 0010 [#1] PREEMPT SMP
> CPU: 14 PID: 1156 Comm: upowerd Tainted: G S U 6.0.0-rc1+ #366
> Hardware name: LENOVO 20Y5CTO1WW/20Y5CTO1WW, BIOS N40ET36W (1.18 ) 07/19/2022
> RIP: 0010:0x0
> Code: Unable to access opcode bytes at RIP 0xffffffffffffffd6.
> RSP: 0018:ffff88815350bd08 EFLAGS: 00010212
> RAX: ffff88810207d620 RBX: ffff88815350bd7c RCX: 000000000000394e
> RDX: ffff88815350bd10 RSI: 0000000000000004 RDI: ffff888111722c00
> RBP: ffff88815350bd68 R08: ffff8881187a8af8 R09: ffff8881187a8af8
> R10: 0000000000000000 R11: 000000000000005f R12: ffffffff8162d0b0
> R13: ffff88810159a038 R14: ffffffff823b3768 R15: ffff88810159a000
> FS: 00007fd1f0958140(0000) GS:ffff88901f780000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: ffffffffffffffd6 CR3: 0000000152c7a004 CR4: 0000000000770ee0
> PKRU: 55555554
> Call Trace:
> <TASK>
> __power_supply_is_system_supplied+0x26/0x40
> class_for_each_device+0xa5/0xd0
> ? acpi_battery_get_state+0x4e/0x1f0
> power_supply_is_system_supplied+0x26/0x40
> acpi_battery_get_property+0x301/0x310
> power_supply_show_property+0xa5/0x1d0
> dev_attr_show+0x10/0x30
> sysfs_kf_seq_show+0x78/0xc0
> seq_read_iter+0xfd/0x3e0
> vfs_read+0x1cb/0x290
> ksys_read+0x4e/0xc0
> do_syscall_64+0x2b/0x50
> entry_SYSCALL_64_after_hwframe+0x46/0xb0
> RIP: 0033:0x7fd1f0bed70c
> Code: ec 28 48 89 54 24 18 48 89 74 24 10 89 7c 24 08 e8 39 a4 f8 ff 41 89 c0 48 8b 54 24 18 48 8b 74 24 10 8b 7c 24 08 31 c0 0f 05 <48> 3d 00 f0 ff ff 77 34 44 89 c7 48 89 44 24 08 e8 8f a4 f8 ff 48
> RSP: 002b:00007ffc8d3f27e0 EFLAGS: 00000246 ORIG_RAX: 0000000000000000
> RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007fd1f0bed70c
> RDX: 0000000000001000 RSI: 000055957d534850 RDI: 000000000000000c
> RBP: 000055957d50b1d0 R08: 0000000000000000 R09: 0000000000001000
> R10: 000000000000006f R11: 0000000000000246 R12: 00007ffc8d3f2910
> R13: 0000000000000000 R14: 0000000000000000 R15: 000000000000000c
> </TASK>
>
> CR2: 0000000000000000
> ---[ end trace 0000000000000000 ]---
> RIP: 0010:0x0
> Code: Unable to access opcode bytes at RIP 0xffffffffffffffd6.
> RSP: 0018:ffff88815350bd08 EFLAGS: 00010212
> RAX: ffff88810207d620 RBX: ffff88815350bd7c RCX: 000000000000394e
> RDX: ffff88815350bd10 RSI: 0000000000000004 RDI: ffff888111722c00
> RBP: ffff88815350bd68 R08: ffff8881187a8af8 R09: ffff8881187a8af8
> R10: 0000000000000000 R11: 000000000000005f R12: ffffffff8162d0b0
> R13: ffff88810159a038 R14: ffffffff823b3768 R15: ffff88810159a000
> FS: 00007fd1f0958140(0000) GS:ffff88901f780000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: ffffffffffffffd6 CR3: 0000000152c7a004 CR4: 0000000000770ee0
>
> The disassembly of the top function in the stack trace is:
>
> .text:0000000000000000 __power_supply_is_system_supplied proc near
> .text:0000000000000000 ; DATA XREF: power_supply_is_system_supplied+12↓o
> .text:0000000000000000
> .text:0000000000000000 var_8 = qword ptr -8
> .text:0000000000000000
> .text:0000000000000000 sub rsp, 8
> .text:0000000000000004 mov rdi, [rdi+78h]
> .text:0000000000000008 inc dword ptr [rsi]
> .text:000000000000000A mov [rsp+8+var_8], 0
> .text:0000000000000012 mov rax, [rdi]
> .text:0000000000000015 cmp dword ptr [rax+8], 1
> .text:0000000000000019 jz short loc_2A
> .text:000000000000001B mov rdx, rsp
> .text:000000000000001E mov esi, 4
> .text:0000000000000023 call qword ptr [rax+30h]
> .text:0000000000000026 test eax, eax
> .text:0000000000000028 jz short loc_31
> .text:000000000000002A
> .text:000000000000002A loc_2A: ; CODE XREF: __power_supply_is_system_supplied+19↑j
> .text:000000000000002A xor eax, eax
> .text:000000000000002C add rsp, 8
> .text:0000000000000030 retn
> .text:0000000000000031 ; ---------------------------------------------------------------------------
> .text:0000000000000031
> .text:0000000000000031 loc_31: ; CODE XREF: __power_supply_is_system_supplied+28↑j
> .text:0000000000000031 mov eax, dword ptr [rsp+8+var_8]
> .text:0000000000000034 add rsp, 8
> .text:0000000000000038 retn
> .text:0000000000000038 __power_supply_is_system_supplied endp
>
> So presumably `call qword ptr [rax+30h]` is jumping to NULL.
>
> Cc: stable@vger.kernel.org
> Cc: Rafael J. Wysocki <rafael@kernel.org>
> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
Acked-by: Rafael J. Wysocki <rafael@kernel.org>
> ---
> drivers/power/supply/power_supply_core.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/power/supply/power_supply_core.c b/drivers/power/supply/power_supply_core.c
> index 4b5fb172fa99..aa4c97f11588 100644
> --- a/drivers/power/supply/power_supply_core.c
> +++ b/drivers/power/supply/power_supply_core.c
> @@ -349,7 +349,7 @@ static int __power_supply_is_system_supplied(struct device *dev, void *data)
> unsigned int *count = data;
>
> (*count)++;
> - if (psy->desc->type != POWER_SUPPLY_TYPE_BATTERY)
> + if (psy->desc->type != POWER_SUPPLY_TYPE_BATTERY && psy->desc->get_property)
> if (!psy->desc->get_property(psy, POWER_SUPPLY_PROP_ONLINE,
> &ret))
> return ret.intval;
> --
> 2.37.2
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH RESEND] power: supply: avoid nullptr deref in __power_supply_is_system_supplied
2022-08-29 15:46 ` Rafael J. Wysocki
@ 2022-09-05 17:24 ` Jason A. Donenfeld
2022-09-11 12:33 ` Sebastian Reichel
0 siblings, 1 reply; 12+ messages in thread
From: Jason A. Donenfeld @ 2022-09-05 17:24 UTC (permalink / raw)
To: linux-pm; +Cc: Jason A. Donenfeld, stable, Rafael J . Wysocki
Fix the following OOPS:
BUG: kernel NULL pointer dereference, address: 0000000000000000
PGD 0 P4D 0
Oops: 0010 [#1] PREEMPT SMP
CPU: 14 PID: 1156 Comm: upowerd Tainted: G S U 6.0.0-rc1+ #366
Hardware name: LENOVO 20Y5CTO1WW/20Y5CTO1WW, BIOS N40ET36W (1.18 ) 07/19/2022
RIP: 0010:0x0
Code: Unable to access opcode bytes at RIP 0xffffffffffffffd6.
RSP: 0018:ffff88815350bd08 EFLAGS: 00010212
RAX: ffff88810207d620 RBX: ffff88815350bd7c RCX: 000000000000394e
RDX: ffff88815350bd10 RSI: 0000000000000004 RDI: ffff888111722c00
RBP: ffff88815350bd68 R08: ffff8881187a8af8 R09: ffff8881187a8af8
R10: 0000000000000000 R11: 000000000000005f R12: ffffffff8162d0b0
R13: ffff88810159a038 R14: ffffffff823b3768 R15: ffff88810159a000
FS: 00007fd1f0958140(0000) GS:ffff88901f780000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: ffffffffffffffd6 CR3: 0000000152c7a004 CR4: 0000000000770ee0
PKRU: 55555554
Call Trace:
<TASK>
__power_supply_is_system_supplied+0x26/0x40
class_for_each_device+0xa5/0xd0
? acpi_battery_get_state+0x4e/0x1f0
power_supply_is_system_supplied+0x26/0x40
acpi_battery_get_property+0x301/0x310
power_supply_show_property+0xa5/0x1d0
dev_attr_show+0x10/0x30
sysfs_kf_seq_show+0x78/0xc0
seq_read_iter+0xfd/0x3e0
vfs_read+0x1cb/0x290
ksys_read+0x4e/0xc0
do_syscall_64+0x2b/0x50
entry_SYSCALL_64_after_hwframe+0x46/0xb0
RIP: 0033:0x7fd1f0bed70c
Code: ec 28 48 89 54 24 18 48 89 74 24 10 89 7c 24 08 e8 39 a4 f8 ff 41 89 c0 48 8b 54 24 18 48 8b 74 24 10 8b 7c 24 08 31 c0 0f 05 <48> 3d 00 f0 ff ff 77 34 44 89 c7 48 89 44 24 08 e8 8f a4 f8 ff 48
RSP: 002b:00007ffc8d3f27e0 EFLAGS: 00000246 ORIG_RAX: 0000000000000000
RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007fd1f0bed70c
RDX: 0000000000001000 RSI: 000055957d534850 RDI: 000000000000000c
RBP: 000055957d50b1d0 R08: 0000000000000000 R09: 0000000000001000
R10: 000000000000006f R11: 0000000000000246 R12: 00007ffc8d3f2910
R13: 0000000000000000 R14: 0000000000000000 R15: 000000000000000c
</TASK>
CR2: 0000000000000000
---[ end trace 0000000000000000 ]---
RIP: 0010:0x0
Code: Unable to access opcode bytes at RIP 0xffffffffffffffd6.
RSP: 0018:ffff88815350bd08 EFLAGS: 00010212
RAX: ffff88810207d620 RBX: ffff88815350bd7c RCX: 000000000000394e
RDX: ffff88815350bd10 RSI: 0000000000000004 RDI: ffff888111722c00
RBP: ffff88815350bd68 R08: ffff8881187a8af8 R09: ffff8881187a8af8
R10: 0000000000000000 R11: 000000000000005f R12: ffffffff8162d0b0
R13: ffff88810159a038 R14: ffffffff823b3768 R15: ffff88810159a000
FS: 00007fd1f0958140(0000) GS:ffff88901f780000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: ffffffffffffffd6 CR3: 0000000152c7a004 CR4: 0000000000770ee0
The disassembly of the top function in the stack trace is:
.text:0000000000000000 __power_supply_is_system_supplied proc near
.text:0000000000000000 ; DATA XREF: power_supply_is_system_supplied+12↓o
.text:0000000000000000
.text:0000000000000000 var_8 = qword ptr -8
.text:0000000000000000
.text:0000000000000000 sub rsp, 8
.text:0000000000000004 mov rdi, [rdi+78h]
.text:0000000000000008 inc dword ptr [rsi]
.text:000000000000000A mov [rsp+8+var_8], 0
.text:0000000000000012 mov rax, [rdi]
.text:0000000000000015 cmp dword ptr [rax+8], 1
.text:0000000000000019 jz short loc_2A
.text:000000000000001B mov rdx, rsp
.text:000000000000001E mov esi, 4
.text:0000000000000023 call qword ptr [rax+30h]
.text:0000000000000026 test eax, eax
.text:0000000000000028 jz short loc_31
.text:000000000000002A
.text:000000000000002A loc_2A: ; CODE XREF: __power_supply_is_system_supplied+19↑j
.text:000000000000002A xor eax, eax
.text:000000000000002C add rsp, 8
.text:0000000000000030 retn
.text:0000000000000031 ; ---------------------------------------------------------------------------
.text:0000000000000031
.text:0000000000000031 loc_31: ; CODE XREF: __power_supply_is_system_supplied+28↑j
.text:0000000000000031 mov eax, dword ptr [rsp+8+var_8]
.text:0000000000000034 add rsp, 8
.text:0000000000000038 retn
.text:0000000000000038 __power_supply_is_system_supplied endp
So presumably `call qword ptr [rax+30h]` is jumping to NULL.
Cc: stable@vger.kernel.org
Acked-by: Rafael J. Wysocki <rafael@kernel.org>
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
drivers/power/supply/power_supply_core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/power/supply/power_supply_core.c b/drivers/power/supply/power_supply_core.c
index 4b5fb172fa99..aa4c97f11588 100644
--- a/drivers/power/supply/power_supply_core.c
+++ b/drivers/power/supply/power_supply_core.c
@@ -349,7 +349,7 @@ static int __power_supply_is_system_supplied(struct device *dev, void *data)
unsigned int *count = data;
(*count)++;
- if (psy->desc->type != POWER_SUPPLY_TYPE_BATTERY)
+ if (psy->desc->type != POWER_SUPPLY_TYPE_BATTERY && psy->desc->get_property)
if (!psy->desc->get_property(psy, POWER_SUPPLY_PROP_ONLINE,
&ret))
return ret.intval;
--
2.37.3
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH RESEND] power: supply: avoid nullptr deref in __power_supply_is_system_supplied
2022-09-05 17:24 ` [PATCH RESEND] " Jason A. Donenfeld
@ 2022-09-11 12:33 ` Sebastian Reichel
2022-09-12 10:45 ` Jason A. Donenfeld
0 siblings, 1 reply; 12+ messages in thread
From: Sebastian Reichel @ 2022-09-11 12:33 UTC (permalink / raw)
To: Jason A. Donenfeld; +Cc: linux-pm, stable, Rafael J . Wysocki
[-- Attachment #1: Type: text/plain, Size: 6049 bytes --]
Hi,
On Mon, Sep 05, 2022 at 07:24:28PM +0200, Jason A. Donenfeld wrote:
> Fix the following OOPS:
>
> BUG: kernel NULL pointer dereference, address: 0000000000000000
> PGD 0 P4D 0
> Oops: 0010 [#1] PREEMPT SMP
> CPU: 14 PID: 1156 Comm: upowerd Tainted: G S U 6.0.0-rc1+ #366
> Hardware name: LENOVO 20Y5CTO1WW/20Y5CTO1WW, BIOS N40ET36W (1.18 ) 07/19/2022
> RIP: 0010:0x0
> Code: Unable to access opcode bytes at RIP 0xffffffffffffffd6.
> RSP: 0018:ffff88815350bd08 EFLAGS: 00010212
> RAX: ffff88810207d620 RBX: ffff88815350bd7c RCX: 000000000000394e
> RDX: ffff88815350bd10 RSI: 0000000000000004 RDI: ffff888111722c00
> RBP: ffff88815350bd68 R08: ffff8881187a8af8 R09: ffff8881187a8af8
> R10: 0000000000000000 R11: 000000000000005f R12: ffffffff8162d0b0
> R13: ffff88810159a038 R14: ffffffff823b3768 R15: ffff88810159a000
> FS: 00007fd1f0958140(0000) GS:ffff88901f780000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: ffffffffffffffd6 CR3: 0000000152c7a004 CR4: 0000000000770ee0
> PKRU: 55555554
> Call Trace:
> <TASK>
> __power_supply_is_system_supplied+0x26/0x40
> class_for_each_device+0xa5/0xd0
> ? acpi_battery_get_state+0x4e/0x1f0
> power_supply_is_system_supplied+0x26/0x40
> acpi_battery_get_property+0x301/0x310
> power_supply_show_property+0xa5/0x1d0
> dev_attr_show+0x10/0x30
> sysfs_kf_seq_show+0x78/0xc0
> seq_read_iter+0xfd/0x3e0
> vfs_read+0x1cb/0x290
> ksys_read+0x4e/0xc0
> do_syscall_64+0x2b/0x50
> entry_SYSCALL_64_after_hwframe+0x46/0xb0
> RIP: 0033:0x7fd1f0bed70c
> Code: ec 28 48 89 54 24 18 48 89 74 24 10 89 7c 24 08 e8 39 a4 f8 ff 41 89 c0 48 8b 54 24 18 48 8b 74 24 10 8b 7c 24 08 31 c0 0f 05 <48> 3d 00 f0 ff ff 77 34 44 89 c7 48 89 44 24 08 e8 8f a4 f8 ff 48
> RSP: 002b:00007ffc8d3f27e0 EFLAGS: 00000246 ORIG_RAX: 0000000000000000
> RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007fd1f0bed70c
> RDX: 0000000000001000 RSI: 000055957d534850 RDI: 000000000000000c
> RBP: 000055957d50b1d0 R08: 0000000000000000 R09: 0000000000001000
> R10: 000000000000006f R11: 0000000000000246 R12: 00007ffc8d3f2910
> R13: 0000000000000000 R14: 0000000000000000 R15: 000000000000000c
> </TASK>
>
> CR2: 0000000000000000
> ---[ end trace 0000000000000000 ]---
> RIP: 0010:0x0
> Code: Unable to access opcode bytes at RIP 0xffffffffffffffd6.
> RSP: 0018:ffff88815350bd08 EFLAGS: 00010212
> RAX: ffff88810207d620 RBX: ffff88815350bd7c RCX: 000000000000394e
> RDX: ffff88815350bd10 RSI: 0000000000000004 RDI: ffff888111722c00
> RBP: ffff88815350bd68 R08: ffff8881187a8af8 R09: ffff8881187a8af8
> R10: 0000000000000000 R11: 000000000000005f R12: ffffffff8162d0b0
> R13: ffff88810159a038 R14: ffffffff823b3768 R15: ffff88810159a000
> FS: 00007fd1f0958140(0000) GS:ffff88901f780000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: ffffffffffffffd6 CR3: 0000000152c7a004 CR4: 0000000000770ee0
>
> The disassembly of the top function in the stack trace is:
>
> .text:0000000000000000 __power_supply_is_system_supplied proc near
> .text:0000000000000000 ; DATA XREF: power_supply_is_system_supplied+12↓o
> .text:0000000000000000
> .text:0000000000000000 var_8 = qword ptr -8
> .text:0000000000000000
> .text:0000000000000000 sub rsp, 8
> .text:0000000000000004 mov rdi, [rdi+78h]
> .text:0000000000000008 inc dword ptr [rsi]
> .text:000000000000000A mov [rsp+8+var_8], 0
> .text:0000000000000012 mov rax, [rdi]
> .text:0000000000000015 cmp dword ptr [rax+8], 1
> .text:0000000000000019 jz short loc_2A
> .text:000000000000001B mov rdx, rsp
> .text:000000000000001E mov esi, 4
> .text:0000000000000023 call qword ptr [rax+30h]
> .text:0000000000000026 test eax, eax
> .text:0000000000000028 jz short loc_31
> .text:000000000000002A
> .text:000000000000002A loc_2A: ; CODE XREF: __power_supply_is_system_supplied+19↑j
> .text:000000000000002A xor eax, eax
> .text:000000000000002C add rsp, 8
> .text:0000000000000030 retn
> .text:0000000000000031 ; ---------------------------------------------------------------------------
> .text:0000000000000031
> .text:0000000000000031 loc_31: ; CODE XREF: __power_supply_is_system_supplied+28↑j
> .text:0000000000000031 mov eax, dword ptr [rsp+8+var_8]
> .text:0000000000000034 add rsp, 8
> .text:0000000000000038 retn
> .text:0000000000000038 __power_supply_is_system_supplied endp
>
> So presumably `call qword ptr [rax+30h]` is jumping to NULL.
>
> Cc: stable@vger.kernel.org
> Acked-by: Rafael J. Wysocki <rafael@kernel.org>
> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> ---
> drivers/power/supply/power_supply_core.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/power/supply/power_supply_core.c b/drivers/power/supply/power_supply_core.c
> index 4b5fb172fa99..aa4c97f11588 100644
> --- a/drivers/power/supply/power_supply_core.c
> +++ b/drivers/power/supply/power_supply_core.c
> @@ -349,7 +349,7 @@ static int __power_supply_is_system_supplied(struct device *dev, void *data)
> unsigned int *count = data;
>
> (*count)++;
> - if (psy->desc->type != POWER_SUPPLY_TYPE_BATTERY)
> + if (psy->desc->type != POWER_SUPPLY_TYPE_BATTERY && psy->desc->get_property)
> if (!psy->desc->get_property(psy, POWER_SUPPLY_PROP_ONLINE,
> &ret))
> return ret.intval;
Thanks, queued into power-supply's fixes branch. But I'm curioous
how you triggered this. Which power-supply driver does not add
a get_property function?
-- Sebastian
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH RESEND] power: supply: avoid nullptr deref in __power_supply_is_system_supplied
2022-09-11 12:33 ` Sebastian Reichel
@ 2022-09-12 10:45 ` Jason A. Donenfeld
2022-09-12 10:48 ` Jason A. Donenfeld
0 siblings, 1 reply; 12+ messages in thread
From: Jason A. Donenfeld @ 2022-09-12 10:45 UTC (permalink / raw)
To: Sebastian Reichel; +Cc: linux-pm, stable, Rafael J . Wysocki
Hi Sebastian,
On Sun, Sep 11, 2022 at 02:33:46PM +0200, Sebastian Reichel wrote:
> > - if (psy->desc->type != POWER_SUPPLY_TYPE_BATTERY)
> > + if (psy->desc->type != POWER_SUPPLY_TYPE_BATTERY && psy->desc->get_property)
> > if (!psy->desc->get_property(psy, POWER_SUPPLY_PROP_ONLINE,
> > &ret))
> > return ret.intval;
>
> Thanks, queued into power-supply's fixes branch. But I'm curioous
> how you triggered this. Which power-supply driver does not add
> a get_property function?
AFAIK, I'm just using the normal ACPI one. Really nothing fancy.
Thinkpad X1 Extreme Gen 4.
Maybe get_property was being set and unset during some kind of
initialization/deinitialization that was happening in response to some
other event? Not sure, except that I managed to trigger it twice before
patching my kernel so my laptop would keep working.
My machine went through three changes I know about between the threshold
of "not crashing" and "crashing":
- Upgraded to 5.19 and then 6.0-rc1.
- I used my laptop on batteries for a prolonged period of time for the
first time in a while.
- I updated KDE, whose power management UI elements may or may not make
frequent calls to this subsystem to update some visual representation.
I don't have any data to back up this claim at all, but something that
at least _sounds_ plausible is that an updated KDE thing was hammering
on this read() method, while a decreasing battery state caused some
aspect of the subsystem to reinitialize the power management object,
making ->get_property() temporarily NULL. But just a guess!
Jason
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH RESEND] power: supply: avoid nullptr deref in __power_supply_is_system_supplied
2022-09-12 10:45 ` Jason A. Donenfeld
@ 2022-09-12 10:48 ` Jason A. Donenfeld
2022-09-12 10:56 ` Jason A. Donenfeld
0 siblings, 1 reply; 12+ messages in thread
From: Jason A. Donenfeld @ 2022-09-12 10:48 UTC (permalink / raw)
To: Sebastian Reichel; +Cc: linux-pm, stable, Rafael J . Wysocki
Ah another thing:
On Mon, Sep 12, 2022 at 11:45 AM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> My machine went through three changes I know about between the threshold
> of "not crashing" and "crashing":
> - Upgraded to 5.19 and then 6.0-rc1.
> - I used my laptop on batteries for a prolonged period of time for the
> first time in a while.
> - I updated KDE, whose power management UI elements may or may not make
> frequent calls to this subsystem to update some visual representation.
- Updated my BIOS.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH RESEND] power: supply: avoid nullptr deref in __power_supply_is_system_supplied
2022-09-12 10:48 ` Jason A. Donenfeld
@ 2022-09-12 10:56 ` Jason A. Donenfeld
[not found] ` <TYZPR03MB599467EF3E0F17E72DF47E86BD449@TYZPR03MB5994.apcprd03.prod.outlook.com>
2022-09-12 17:12 ` Sebastian Reichel
0 siblings, 2 replies; 12+ messages in thread
From: Jason A. Donenfeld @ 2022-09-12 10:56 UTC (permalink / raw)
To: Sebastian Reichel, Mark Pearson; +Cc: linux-pm, stable, Rafael J . Wysocki
CC+ Mark Pearson from Lenovo
Full thread is here:
https://lore.kernel.org/all/YwDsy3ZUgTtlKH9r@zx2c4.com/
On Mon, Sep 12, 2022 at 11:48 AM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
>
> Ah another thing:
>
> On Mon, Sep 12, 2022 at 11:45 AM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> > My machine went through three changes I know about between the threshold
> > of "not crashing" and "crashing":
> > - Upgraded to 5.19 and then 6.0-rc1.
> > - I used my laptop on batteries for a prolonged period of time for the
> > first time in a while.
> > - I updated KDE, whose power management UI elements may or may not make
> > frequent calls to this subsystem to update some visual representation.
>
> - Updated my BIOS.
GASP! The plot thickens.
It appears that the BIOS update I applied has been removed from
https://pcsupport.lenovo.com/fr/en/downloads/ds551052-bios-update-utility-bootable-cd-for-windows-10-64-bit-and-linux-thinkpad-p1-gen-4-x1-extreme-gen-4
and now it only shows the 1.16 version. I updated from 1.16 to 1.18.
The missing release notes are still online if you futz with the URL:
https://download.lenovo.com/pccbbs/mobiles/n40ur14w.txt
https://download.lenovo.com/pccbbs/mobiles/n40ur15w.txt
One of the items for 1.17 says:
> - (Fix) Fixed an issue where it took a long time to update the battery FW.
So maybe something was happening here...
I'm CC'ing Mark from Lenovo to see if he has any insight as to why
this BIOS update was pulled.
Maybe the battery was appearing and disappearing rapidly. If that's
correct, then it'd indicate that this bandaid patch is *wrong* and
what actually is needed is some kind of reference counting or RCU
around that sysfs interface (and maybe others).
Jason
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH RESEND] power: supply: avoid nullptr deref in __power_supply_is_system_supplied
[not found] ` <TYZPR03MB599467EF3E0F17E72DF47E86BD449@TYZPR03MB5994.apcprd03.prod.outlook.com>
@ 2022-09-12 12:44 ` Mark Pearson
0 siblings, 0 replies; 12+ messages in thread
From: Mark Pearson @ 2022-09-12 12:44 UTC (permalink / raw)
To: Jason A. Donenfeld
Cc: Sebastian Reichel, linux-pm, stable, Rafael J . Wysocki
<Note - hope this works - moved to my more opensource friendly email
account>
On 2022-09-12 08:20, Mark Pearson wrote:
>
> --------------------------------------------------------------------------------
> *From:* Jason A. Donenfeld <Jason@zx2c4.com>
> *Sent:* September 12, 2022 6:56
> *To:* Sebastian Reichel <sebastian.reichel@collabora.com>; Mark Pearson
> <mpearson@lenovo.com>
> *Cc:* linux-pm@vger.kernel.org <linux-pm@vger.kernel.org>;
> stable@vger.kernel.org <stable@vger.kernel.org>; Rafael J . Wysocki
> <rafael@kernel.org>
> *Subject:* [External] Re: [PATCH RESEND] power: supply: avoid nullptr deref in
> __power_supply_is_system_supplied
> CC+ Mark Pearson from Lenovo
> Full thread is here:
> https://lore.kernel.org/all/YwDsy3ZUgTtlKH9r@zx2c4.com/ <https://lore.kernel.org/all/YwDsy3ZUgTtlKH9r@zx2c4.com/>>
> On Mon, Sep 12, 2022 at 11:48 AM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
>>
>> Ah another thing:
>>
>> On Mon, Sep 12, 2022 at 11:45 AM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
>> > My machine went through three changes I know about between the threshold
>> > of "not crashing" and "crashing":
>> > - Upgraded to 5.19 and then 6.0-rc1.
>> > - I used my laptop on batteries for a prolonged period of time for the
>> > first time in a while.
>> > - I updated KDE, whose power management UI elements may or may not make
>> > frequent calls to this subsystem to update some visual representation.
>>
>> - Updated my BIOS.
>
> GASP! The plot thickens.
>
> It appears that the BIOS update I applied has been removed from
> https://pcsupport.lenovo.com/fr/en/downloads/ds551052-bios-update-utility-bootable-cd-for-windows-10-64-bit-and-linux-thinkpad-p1-gen-4-x1-extreme-gen-4 <https://pcsupport.lenovo.com/fr/en/downloads/ds551052-bios-update-utility-bootable-cd-for-windows-10-64-bit-and-linux-thinkpad-p1-gen-4-x1-extreme-gen-4>
> and now it only shows the 1.16 version. I updated from 1.16 to 1.18.
>
> The missing release notes are still online if you futz with the URL:
> https://download.lenovo.com/pccbbs/mobiles/n40ur14w.txt
> <https://download.lenovo.com/pccbbs/mobiles/n40ur14w.txt>
> https://download.lenovo.com/pccbbs/mobiles/n40ur15w.txt
> <https://download.lenovo.com/pccbbs/mobiles/n40ur15w.txt>
>
> One of the items for 1.17 says:
>> - (Fix) Fixed an issue where it took a long time to update the battery FW.
>
> So maybe something was happening here...
>
> I'm CC'ing Mark from Lenovo to see if he has any insight as to why
> this BIOS update was pulled.
>
> Maybe the battery was appearing and disappearing rapidly. If that's
> correct, then it'd indicate that this bandaid patch is *wrong* and
> what actually is needed is some kind of reference counting or RCU
> around that sysfs interface (and maybe others).
>
> Jason
Hi Jason,
I'll have to check with the FW team but looking at the internal notes I
think the FW was pulled because of a graphics display regression.
Version 36W was fixing a brightness control issue in discrete mode and
37W (not yet released) is fixing external display - so my guess is
something about the fix in 36W has a side effect
More interesting is the EC FW updates. There isn't a new version posted
but there are fixes in the previous version (EC 33W) for a fix for a
'suspected EC-Battery communication transaction failure'. Is that
potentially related to this patch in some way? I can go and ask for more
details if we think it's related. I'll also see if I can repro on my
P1G4 - but I hadn't seen any other reports so it might be HW specific.
Can you confirm which FW you have from the BIOS setup screen (F1 during
early boot)? BIOS and EC please.
Mark
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH RESEND] power: supply: avoid nullptr deref in __power_supply_is_system_supplied
2022-09-12 10:56 ` Jason A. Donenfeld
[not found] ` <TYZPR03MB599467EF3E0F17E72DF47E86BD449@TYZPR03MB5994.apcprd03.prod.outlook.com>
@ 2022-09-12 17:12 ` Sebastian Reichel
1 sibling, 0 replies; 12+ messages in thread
From: Sebastian Reichel @ 2022-09-12 17:12 UTC (permalink / raw)
To: Jason A. Donenfeld; +Cc: Mark Pearson, linux-pm, stable, Rafael J . Wysocki
[-- Attachment #1: Type: text/plain, Size: 2947 bytes --]
Hi,
On Mon, Sep 12, 2022 at 11:56:43AM +0100, Jason A. Donenfeld wrote:
> AFAIK, I'm just using the normal ACPI one. Really nothing fancy.
> Thinkpad X1 Extreme Gen 4.
All ACPI drivers setup get_property method in their power-supply
devices.
> Maybe get_property was being set and unset during some kind of
> initialization/deinitialization that was happening in response to some
> other event? Not sure, except that I managed to trigger it twice before
> patching my kernel so my laptop would keep working.
The function is not intended to be changed during the lifetime of
the device and AFAIK no mainline drivers does this.
> On Mon, Sep 12, 2022 at 11:48 AM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> > On Mon, Sep 12, 2022 at 11:45 AM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> > > My machine went through three changes I know about between the threshold
> > > of "not crashing" and "crashing":
> > > - Upgraded to 5.19 and then 6.0-rc1.
> > > - I used my laptop on batteries for a prolonged period of time for the
> > > first time in a while.
> > > - I updated KDE, whose power management UI elements may or may not make
> > > frequent calls to this subsystem to update some visual representation.
> >
> > - Updated my BIOS.
>
> GASP! The plot thickens.
>
> It appears that the BIOS update I applied has been removed from
> https://pcsupport.lenovo.com/fr/en/downloads/ds551052-bios-update-utility-bootable-cd-for-windows-10-64-bit-and-linux-thinkpad-p1-gen-4-x1-extreme-gen-4
> and now it only shows the 1.16 version. I updated from 1.16 to 1.18.
>
> The missing release notes are still online if you futz with the URL:
> https://download.lenovo.com/pccbbs/mobiles/n40ur14w.txt
> https://download.lenovo.com/pccbbs/mobiles/n40ur15w.txt
>
> One of the items for 1.17 says:
> > - (Fix) Fixed an issue where it took a long time to update the battery FW.
>
> So maybe something was happening here...
>
> I'm CC'ing Mark from Lenovo to see if he has any insight as to why
> this BIOS update was pulled.
>
> Maybe the battery was appearing and disappearing rapidly.
>
> If that's correct, then it'd indicate that this bandaid patch is
> *wrong* and what actually is needed is some kind of reference
> counting or RCU around that sysfs interface (and maybe others).
Device create/remove is the only time that is supposed to touch
the get_property callback. So I suppose a race condition in that
path would be a sensible root cause. Considering systems usually
registers the device once and keeps it until shutdown would also
explain why this has not been noticed earlier.
The function you modified is only called by
power_supply_is_system_supplied(), which is an in-kernel function to
figure out if the system is running on battery.
Can you trigger this easy enough to figure out a few more details
about the state of the problematic device?
-- Sebastian
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2022-09-12 17:12 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-20 14:16 null ptr deref in __power_supply_is_system_supplied Jason A. Donenfeld
2022-08-22 16:44 ` Rafael J. Wysocki
2022-08-22 17:26 ` Jason A. Donenfeld
2022-08-29 15:28 ` [PATCH] power: supply: avoid nullptr " Jason A. Donenfeld
2022-08-29 15:46 ` Rafael J. Wysocki
2022-09-05 17:24 ` [PATCH RESEND] " Jason A. Donenfeld
2022-09-11 12:33 ` Sebastian Reichel
2022-09-12 10:45 ` Jason A. Donenfeld
2022-09-12 10:48 ` Jason A. Donenfeld
2022-09-12 10:56 ` Jason A. Donenfeld
[not found] ` <TYZPR03MB599467EF3E0F17E72DF47E86BD449@TYZPR03MB5994.apcprd03.prod.outlook.com>
2022-09-12 12:44 ` Mark Pearson
2022-09-12 17:12 ` Sebastian Reichel
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.