| Message ID | 20241203200010.2821132-1-lucas.demarchi@intel.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 33ead7e538183b1348ba60af90027240a10de751 |
| Headers | show |
| Series | usb: typec: ucsi: Fix connector status writing past buffer size | expand |
Hi, On Tue, Dec 03, 2024 at 12:00:10PM -0800, Lucas De Marchi wrote: > Similar to commit 65c4c9447bfc ("usb: typec: ucsi: Fix a missing bits to > bytes conversion in ucsi_init()"), there was a missing conversion from > bits to bytes. Here the outcome is worse though: since the value is > lower than UCSI_MAX_DATA_LENGTH, instead of bailing out with an error, > it writes past the buffer size. > > The error is then seen in other places like below: > > Oops: general protection fault, probably for non-canonical address 0x891e812cd0ed968: 0000 [#1] PREEMPT SMP NOPTI > CPU: 3 UID: 110 PID: 906 Comm: prometheus-node Not tainted 6.13.0-rc1-xe #1 > Hardware name: Intel Corporation Lunar Lake Client Platform/LNL-M LP5 RVP1, BIOS LNLMFWI1.R00.3222.D84.2410171025 10/17/2024 > RIP: 0010:power_supply_get_property+0x3e/0xe0 > Code: 85 c0 7e 4f 4c 8b 07 89 f3 49 89 d4 49 8b 48 20 48 85 c9 74 72 49 8b 70 18 31 d2 31 c0 eb 0b 83 c2 01 48 63 c2 48 39 c8 73 5d <3b> 1c 86 75 f0 49 8b 40 28 4c 89 e2 89 de ff d0 0f 1f 00 5b 41 5c > RSP: 0018:ffffc900017dfa50 EFLAGS: 00010246 > RAX: 0000000000000000 RBX: 0000000000000011 RCX: c963b02c06092008 > RDX: 0000000000000000 RSI: 0891e812cd0ed968 RDI: ffff888121dd6800 > RBP: ffffc900017dfa68 R08: ffff88810a4024b8 R09: 0000000000000000 > R10: 0000000000000000 R11: 0000000000000000 R12: ffffc900017dfa78 > R13: ffff888121dd6800 R14: ffff888138ad2c00 R15: ffff88810c57c528 > FS: 00007713a2ffd6c0(0000) GS:ffff88846f380000(0000) knlGS:0000000000000000 > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > CR2: 000000c0004b1000 CR3: 0000000121ce8003 CR4: 0000000000f72ef0 > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 > DR3: 0000000000000000 DR6: 00000000ffff07f0 DR7: 0000000000000400 > PKRU: 55555554 > Call Trace: > <TASK> > ? show_regs+0x6c/0x80 > ? die_addr+0x37/0xa0 > ? exc_general_protection+0x1c1/0x440 > ? asm_exc_general_protection+0x27/0x30 > ? power_supply_get_property+0x3e/0xe0 > power_supply_hwmon_read+0x50/0xe0 > hwmon_attr_show+0x46/0x170 > dev_attr_show+0x1a/0x70 > sysfs_kf_seq_show+0xaa/0x120 > kernfs_seq_show+0x41/0x60 > > Just use the buffer size as argument to fix it. > > Fixes: 226ff2e681d0 ("usb: typec: ucsi: Convert connector specific commands to bitmaps") > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com> Also you are missing at some tags: Reported-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com> Closes: https://lore.kernel.org/all/SJ1PR11MB6129CCD82CD78D8EE6E27EF4B9362@SJ1PR11MB6129.namprd11.prod.outlook.com/ and considering he figured out the root cause (and there is no better tag AFAIK): Suggested-by: Thomas Weißschuh <linux@weissschuh.net> Greetings, -- Sebastian > --- > drivers/usb/typec/ucsi/ucsi.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c > index c435c0835744a..69caae84879e9 100644 > --- a/drivers/usb/typec/ucsi/ucsi.c > +++ b/drivers/usb/typec/ucsi/ucsi.c > @@ -651,7 +651,8 @@ static void ucsi_unregister_altmodes(struct ucsi_connector *con, u8 recipient) > static int ucsi_get_connector_status(struct ucsi_connector *con, bool conn_ack) > { > u64 command = UCSI_GET_CONNECTOR_STATUS | UCSI_CONNECTOR_NUMBER(con->num); > - size_t size = min(UCSI_GET_CONNECTOR_STATUS_SIZE, UCSI_MAX_DATA_LENGTH(con->ucsi)); > + size_t size = min(sizeof(con->status), > + UCSI_MAX_DATA_LENGTH(con->ucsi)); > int ret; > > ret = ucsi_send_command_common(con->ucsi, command, &con->status, size, conn_ack); > -- > 2.47.0 > >
> -----Original Message----- > From: De Marchi, Lucas <lucas.demarchi@intel.com> > Sent: Wednesday, December 4, 2024 1:30 AM > To: linux-usb@vger.kernel.org > Cc: linux-kernel@vger.kernel.org; Greg Kroah-Hartman > <gregkh@linuxfoundation.org>; Heikki Krogerus > <heikki.krogerus@linux.intel.com>; intel-xe@lists.freedesktop.org; intel- > gfx@lists.freedesktop.org; Thomas Weißschuh <linux@weissschuh.net>; > Borah, Chaitanya Kumar <chaitanya.kumar.borah@intel.com>; Luca Coelho > <luca@coelho.fi>; De Marchi, Lucas <lucas.demarchi@intel.com> > Subject: [PATCH] usb: typec: ucsi: Fix connector status writing past buffer size > > Similar to commit 65c4c9447bfc ("usb: typec: ucsi: Fix a missing bits to bytes > conversion in ucsi_init()"), there was a missing conversion from bits to bytes. > Here the outcome is worse though: since the value is lower than > UCSI_MAX_DATA_LENGTH, instead of bailing out with an error, it writes past > the buffer size. > > The error is then seen in other places like below: > > Oops: general protection fault, probably for non-canonical address > 0x891e812cd0ed968: 0000 [#1] PREEMPT SMP NOPTI > CPU: 3 UID: 110 PID: 906 Comm: prometheus-node Not tainted > 6.13.0-rc1-xe #1 > Hardware name: Intel Corporation Lunar Lake Client Platform/LNL-M > LP5 RVP1, BIOS LNLMFWI1.R00.3222.D84.2410171025 10/17/2024 > RIP: 0010:power_supply_get_property+0x3e/0xe0 > Code: 85 c0 7e 4f 4c 8b 07 89 f3 49 89 d4 49 8b 48 20 48 85 c9 74 72 > 49 8b 70 18 31 d2 31 c0 eb 0b 83 c2 01 48 63 c2 48 39 c8 73 5d <3b> 1c 86 75 > f0 49 8b 40 28 4c 89 e2 89 de ff d0 0f 1f 00 5b 41 5c > RSP: 0018:ffffc900017dfa50 EFLAGS: 00010246 > RAX: 0000000000000000 RBX: 0000000000000011 RCX: > c963b02c06092008 > RDX: 0000000000000000 RSI: 0891e812cd0ed968 RDI: > ffff888121dd6800 > RBP: ffffc900017dfa68 R08: ffff88810a4024b8 R09: 0000000000000000 > R10: 0000000000000000 R11: 0000000000000000 R12: > ffffc900017dfa78 > R13: ffff888121dd6800 R14: ffff888138ad2c00 R15: ffff88810c57c528 > FS: 00007713a2ffd6c0(0000) GS:ffff88846f380000(0000) > knlGS:0000000000000000 > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > CR2: 000000c0004b1000 CR3: 0000000121ce8003 CR4: > 0000000000f72ef0 > DR0: 0000000000000000 DR1: 0000000000000000 DR2: > 0000000000000000 > DR3: 0000000000000000 DR6: 00000000ffff07f0 DR7: > 0000000000000400 > PKRU: 55555554 > Call Trace: > <TASK> > ? show_regs+0x6c/0x80 > ? die_addr+0x37/0xa0 > ? exc_general_protection+0x1c1/0x440 > ? asm_exc_general_protection+0x27/0x30 > ? power_supply_get_property+0x3e/0xe0 > power_supply_hwmon_read+0x50/0xe0 > hwmon_attr_show+0x46/0x170 > dev_attr_show+0x1a/0x70 > sysfs_kf_seq_show+0xaa/0x120 > kernfs_seq_show+0x41/0x60 > > Just use the buffer size as argument to fix it. > > Fixes: 226ff2e681d0 ("usb: typec: ucsi: Convert connector specific commands > to bitmaps") > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> Tested-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com> > --- > drivers/usb/typec/ucsi/ucsi.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c index > c435c0835744a..69caae84879e9 100644 > --- a/drivers/usb/typec/ucsi/ucsi.c > +++ b/drivers/usb/typec/ucsi/ucsi.c > @@ -651,7 +651,8 @@ static void ucsi_unregister_altmodes(struct > ucsi_connector *con, u8 recipient) static int ucsi_get_connector_status(struct > ucsi_connector *con, bool conn_ack) { > u64 command = UCSI_GET_CONNECTOR_STATUS | > UCSI_CONNECTOR_NUMBER(con->num); > - size_t size = min(UCSI_GET_CONNECTOR_STATUS_SIZE, > UCSI_MAX_DATA_LENGTH(con->ucsi)); > + size_t size = min(sizeof(con->status), > + UCSI_MAX_DATA_LENGTH(con->ucsi)); > int ret; > > ret = ucsi_send_command_common(con->ucsi, command, &con- > >status, size, conn_ack); > -- > 2.47.0
On 2024-12-03 12:00:10-0800, Lucas De Marchi wrote: > Similar to commit 65c4c9447bfc ("usb: typec: ucsi: Fix a missing bits to > bytes conversion in ucsi_init()"), there was a missing conversion from > bits to bytes. Here the outcome is worse though: since the value is > lower than UCSI_MAX_DATA_LENGTH, instead of bailing out with an error, > it writes past the buffer size. > > The error is then seen in other places like below: > > Oops: general protection fault, probably for non-canonical address 0x891e812cd0ed968: 0000 [#1] PREEMPT SMP NOPTI > CPU: 3 UID: 110 PID: 906 Comm: prometheus-node Not tainted 6.13.0-rc1-xe #1 > Hardware name: Intel Corporation Lunar Lake Client Platform/LNL-M LP5 RVP1, BIOS LNLMFWI1.R00.3222.D84.2410171025 10/17/2024 > RIP: 0010:power_supply_get_property+0x3e/0xe0 > Code: 85 c0 7e 4f 4c 8b 07 89 f3 49 89 d4 49 8b 48 20 48 85 c9 74 72 49 8b 70 18 31 d2 31 c0 eb 0b 83 c2 01 48 63 c2 48 39 c8 73 5d <3b> 1c 86 75 f0 49 8b 40 28 4c 89 e2 89 de ff d0 0f 1f 00 5b 41 5c > RSP: 0018:ffffc900017dfa50 EFLAGS: 00010246 > RAX: 0000000000000000 RBX: 0000000000000011 RCX: c963b02c06092008 > RDX: 0000000000000000 RSI: 0891e812cd0ed968 RDI: ffff888121dd6800 > RBP: ffffc900017dfa68 R08: ffff88810a4024b8 R09: 0000000000000000 > R10: 0000000000000000 R11: 0000000000000000 R12: ffffc900017dfa78 > R13: ffff888121dd6800 R14: ffff888138ad2c00 R15: ffff88810c57c528 > FS: 00007713a2ffd6c0(0000) GS:ffff88846f380000(0000) knlGS:0000000000000000 > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > CR2: 000000c0004b1000 CR3: 0000000121ce8003 CR4: 0000000000f72ef0 > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 > DR3: 0000000000000000 DR6: 00000000ffff07f0 DR7: 0000000000000400 > PKRU: 55555554 > Call Trace: > <TASK> > ? show_regs+0x6c/0x80 > ? die_addr+0x37/0xa0 > ? exc_general_protection+0x1c1/0x440 > ? asm_exc_general_protection+0x27/0x30 > ? power_supply_get_property+0x3e/0xe0 > power_supply_hwmon_read+0x50/0xe0 > hwmon_attr_show+0x46/0x170 > dev_attr_show+0x1a/0x70 > sysfs_kf_seq_show+0xaa/0x120 > kernfs_seq_show+0x41/0x60 > > Just use the buffer size as argument to fix it. > > Fixes: 226ff2e681d0 ("usb: typec: ucsi: Convert connector specific commands to bitmaps") > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> > --- > drivers/usb/typec/ucsi/ucsi.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c > index c435c0835744a..69caae84879e9 100644 > --- a/drivers/usb/typec/ucsi/ucsi.c > +++ b/drivers/usb/typec/ucsi/ucsi.c > @@ -651,7 +651,8 @@ static void ucsi_unregister_altmodes(struct ucsi_connector *con, u8 recipient) > static int ucsi_get_connector_status(struct ucsi_connector *con, bool conn_ack) > { > u64 command = UCSI_GET_CONNECTOR_STATUS | UCSI_CONNECTOR_NUMBER(con->num); > - size_t size = min(UCSI_GET_CONNECTOR_STATUS_SIZE, UCSI_MAX_DATA_LENGTH(con->ucsi)); > + size_t size = min(sizeof(con->status), > + UCSI_MAX_DATA_LENGTH(con->ucsi)); Why the linebreak? The previous line was longer and all other lines are longer, too. In any case: Reviewed-by: Thomas Weißschuh <linux@weissschuh.net> > int ret; > > ret = ucsi_send_command_common(con->ucsi, command, &con->status, size, conn_ack); > -- > 2.47.0 >
On Tue, Dec 03, 2024 at 12:00:10PM -0800, Lucas De Marchi wrote: > Similar to commit 65c4c9447bfc ("usb: typec: ucsi: Fix a missing bits to > bytes conversion in ucsi_init()"), there was a missing conversion from > bits to bytes. Here the outcome is worse though: since the value is > lower than UCSI_MAX_DATA_LENGTH, instead of bailing out with an error, > it writes past the buffer size. > > The error is then seen in other places like below: > > Oops: general protection fault, probably for non-canonical address 0x891e812cd0ed968: 0000 [#1] PREEMPT SMP NOPTI > CPU: 3 UID: 110 PID: 906 Comm: prometheus-node Not tainted 6.13.0-rc1-xe #1 > Hardware name: Intel Corporation Lunar Lake Client Platform/LNL-M LP5 RVP1, BIOS LNLMFWI1.R00.3222.D84.2410171025 10/17/2024 > RIP: 0010:power_supply_get_property+0x3e/0xe0 > Code: 85 c0 7e 4f 4c 8b 07 89 f3 49 89 d4 49 8b 48 20 48 85 c9 74 72 49 8b 70 18 31 d2 31 c0 eb 0b 83 c2 01 48 63 c2 48 39 c8 73 5d <3b> 1c 86 75 f0 49 8b 40 28 4c 89 e2 89 de ff d0 0f 1f 00 5b 41 5c > RSP: 0018:ffffc900017dfa50 EFLAGS: 00010246 > RAX: 0000000000000000 RBX: 0000000000000011 RCX: c963b02c06092008 > RDX: 0000000000000000 RSI: 0891e812cd0ed968 RDI: ffff888121dd6800 > RBP: ffffc900017dfa68 R08: ffff88810a4024b8 R09: 0000000000000000 > R10: 0000000000000000 R11: 0000000000000000 R12: ffffc900017dfa78 > R13: ffff888121dd6800 R14: ffff888138ad2c00 R15: ffff88810c57c528 > FS: 00007713a2ffd6c0(0000) GS:ffff88846f380000(0000) knlGS:0000000000000000 > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > CR2: 000000c0004b1000 CR3: 0000000121ce8003 CR4: 0000000000f72ef0 > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 > DR3: 0000000000000000 DR6: 00000000ffff07f0 DR7: 0000000000000400 > PKRU: 55555554 > Call Trace: > <TASK> > ? show_regs+0x6c/0x80 > ? die_addr+0x37/0xa0 > ? exc_general_protection+0x1c1/0x440 > ? asm_exc_general_protection+0x27/0x30 > ? power_supply_get_property+0x3e/0xe0 > power_supply_hwmon_read+0x50/0xe0 > hwmon_attr_show+0x46/0x170 > dev_attr_show+0x1a/0x70 > sysfs_kf_seq_show+0xaa/0x120 > kernfs_seq_show+0x41/0x60 > > Just use the buffer size as argument to fix it. > > Fixes: 226ff2e681d0 ("usb: typec: ucsi: Convert connector specific commands to bitmaps") > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> > --- > drivers/usb/typec/ucsi/ucsi.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c > index c435c0835744a..69caae84879e9 100644 > --- a/drivers/usb/typec/ucsi/ucsi.c > +++ b/drivers/usb/typec/ucsi/ucsi.c > @@ -651,7 +651,8 @@ static void ucsi_unregister_altmodes(struct ucsi_connector *con, u8 recipient) > static int ucsi_get_connector_status(struct ucsi_connector *con, bool conn_ack) > { > u64 command = UCSI_GET_CONNECTOR_STATUS | UCSI_CONNECTOR_NUMBER(con->num); > - size_t size = min(UCSI_GET_CONNECTOR_STATUS_SIZE, UCSI_MAX_DATA_LENGTH(con->ucsi)); > + size_t size = min(sizeof(con->status), > + UCSI_MAX_DATA_LENGTH(con->ucsi)); > int ret; > > ret = ucsi_send_command_common(con->ucsi, command, &con->status, size, conn_ack); > -- > 2.47.0
Hi all, I opened a bugzilla here about this issue, this commit broke all our lunar lake machines in boot (5 in total): https://bugzilla.kernel.org/show_bug.cgi?id=219562 I have tested this patch and it appears to have fixed things. I'm about to test on all machines with a full run of stress testing of S2idle to be absolutely sure. But you can add me as tested by. Tested-By: Brandt, Todd E <todd.e.brandt@linux.intel.com> On Tue, 2024-12-03 at 12:00 -0800, Lucas De Marchi wrote: > Similar to commit 65c4c9447bfc ("usb: typec: ucsi: Fix a missing bits > to > bytes conversion in ucsi_init()"), there was a missing conversion > from > bits to bytes. Here the outcome is worse though: since the value is > lower than UCSI_MAX_DATA_LENGTH, instead of bailing out with an > error, > it writes past the buffer size. > > The error is then seen in other places like below: > > Oops: general protection fault, probably for non-canonical > address 0x891e812cd0ed968: 0000 [#1] PREEMPT SMP NOPTI > CPU: 3 UID: 110 PID: 906 Comm: prometheus-node Not tainted > 6.13.0-rc1-xe #1 > Hardware name: Intel Corporation Lunar Lake Client > Platform/LNL-M LP5 RVP1, BIOS LNLMFWI1.R00.3222.D84.2410171025 > 10/17/2024 > RIP: 0010:power_supply_get_property+0x3e/0xe0 > Code: 85 c0 7e 4f 4c 8b 07 89 f3 49 89 d4 49 8b 48 20 48 85 > c9 74 72 49 8b 70 18 31 d2 31 c0 eb 0b 83 c2 01 48 63 c2 48 39 c8 73 > 5d <3b> 1c 86 75 f0 49 8b 40 28 4c 89 e2 89 de ff d0 0f 1f 00 5b 41 > 5c > RSP: 0018:ffffc900017dfa50 EFLAGS: 00010246 > RAX: 0000000000000000 RBX: 0000000000000011 RCX: > c963b02c06092008 > RDX: 0000000000000000 RSI: 0891e812cd0ed968 RDI: > ffff888121dd6800 > RBP: ffffc900017dfa68 R08: ffff88810a4024b8 R09: > 0000000000000000 > R10: 0000000000000000 R11: 0000000000000000 R12: > ffffc900017dfa78 > R13: ffff888121dd6800 R14: ffff888138ad2c00 R15: > ffff88810c57c528 > FS: 00007713a2ffd6c0(0000) GS:ffff88846f380000(0000) > knlGS:0000000000000000 > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > CR2: 000000c0004b1000 CR3: 0000000121ce8003 CR4: > 0000000000f72ef0 > DR0: 0000000000000000 DR1: 0000000000000000 DR2: > 0000000000000000 > DR3: 0000000000000000 DR6: 00000000ffff07f0 DR7: > 0000000000000400 > PKRU: 55555554 > Call Trace: > <TASK> > ? show_regs+0x6c/0x80 > ? die_addr+0x37/0xa0 > ? exc_general_protection+0x1c1/0x440 > ? asm_exc_general_protection+0x27/0x30 > ? power_supply_get_property+0x3e/0xe0 > power_supply_hwmon_read+0x50/0xe0 > hwmon_attr_show+0x46/0x170 > dev_attr_show+0x1a/0x70 > sysfs_kf_seq_show+0xaa/0x120 > kernfs_seq_show+0x41/0x60 > > Just use the buffer size as argument to fix it. > > Fixes: 226ff2e681d0 ("usb: typec: ucsi: Convert connector specific > commands to bitmaps") > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> > --- > drivers/usb/typec/ucsi/ucsi.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/usb/typec/ucsi/ucsi.c > b/drivers/usb/typec/ucsi/ucsi.c > index c435c0835744a..69caae84879e9 100644 > --- a/drivers/usb/typec/ucsi/ucsi.c > +++ b/drivers/usb/typec/ucsi/ucsi.c > @@ -651,7 +651,8 @@ static void ucsi_unregister_altmodes(struct > ucsi_connector *con, u8 recipient) > static int ucsi_get_connector_status(struct ucsi_connector *con, > bool conn_ack) > { > u64 command = UCSI_GET_CONNECTOR_STATUS | > UCSI_CONNECTOR_NUMBER(con->num); > - size_t size = min(UCSI_GET_CONNECTOR_STATUS_SIZE, > UCSI_MAX_DATA_LENGTH(con->ucsi)); > + size_t size = min(sizeof(con->status), > + UCSI_MAX_DATA_LENGTH(con->ucsi)); > int ret; > > ret = ucsi_send_command_common(con->ucsi, command, &con- > >status, size, conn_ack);
On 12/3/2024 14:00, Lucas De Marchi wrote: > Similar to commit 65c4c9447bfc ("usb: typec: ucsi: Fix a missing bits to > bytes conversion in ucsi_init()"), there was a missing conversion from > bits to bytes. Here the outcome is worse though: since the value is > lower than UCSI_MAX_DATA_LENGTH, instead of bailing out with an error, > it writes past the buffer size. > > The error is then seen in other places like below: > > Oops: general protection fault, probably for non-canonical address 0x891e812cd0ed968: 0000 [#1] PREEMPT SMP NOPTI > CPU: 3 UID: 110 PID: 906 Comm: prometheus-node Not tainted 6.13.0-rc1-xe #1 > Hardware name: Intel Corporation Lunar Lake Client Platform/LNL-M LP5 RVP1, BIOS LNLMFWI1.R00.3222.D84.2410171025 10/17/2024 > RIP: 0010:power_supply_get_property+0x3e/0xe0 > Code: 85 c0 7e 4f 4c 8b 07 89 f3 49 89 d4 49 8b 48 20 48 85 c9 74 72 49 8b 70 18 31 d2 31 c0 eb 0b 83 c2 01 48 63 c2 48 39 c8 73 5d <3b> 1c 86 75 f0 49 8b 40 28 4c 89 e2 89 de ff d0 0f 1f 00 5b 41 5c > RSP: 0018:ffffc900017dfa50 EFLAGS: 00010246 > RAX: 0000000000000000 RBX: 0000000000000011 RCX: c963b02c06092008 > RDX: 0000000000000000 RSI: 0891e812cd0ed968 RDI: ffff888121dd6800 > RBP: ffffc900017dfa68 R08: ffff88810a4024b8 R09: 0000000000000000 > R10: 0000000000000000 R11: 0000000000000000 R12: ffffc900017dfa78 > R13: ffff888121dd6800 R14: ffff888138ad2c00 R15: ffff88810c57c528 > FS: 00007713a2ffd6c0(0000) GS:ffff88846f380000(0000) knlGS:0000000000000000 > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > CR2: 000000c0004b1000 CR3: 0000000121ce8003 CR4: 0000000000f72ef0 > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 > DR3: 0000000000000000 DR6: 00000000ffff07f0 DR7: 0000000000000400 > PKRU: 55555554 > Call Trace: > <TASK> > ? show_regs+0x6c/0x80 > ? die_addr+0x37/0xa0 > ? exc_general_protection+0x1c1/0x440 > ? asm_exc_general_protection+0x27/0x30 > ? power_supply_get_property+0x3e/0xe0 > power_supply_hwmon_read+0x50/0xe0 > hwmon_attr_show+0x46/0x170 > dev_attr_show+0x1a/0x70 > sysfs_kf_seq_show+0xaa/0x120 > kernfs_seq_show+0x41/0x60 > > Just use the buffer size as argument to fix it. > > Fixes: 226ff2e681d0 ("usb: typec: ucsi: Convert connector specific commands to bitmaps") > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> I've got a laptop that can't boot 6.13-rc1 or 6.13-rc2 without this fix too. Tested-by: Mario Limonciello <mario.limonciello@amd.com> Closes: https://lore.kernel.org/linux-pm/93978193-1273-4954-893a-f27528ed1d0e@kernel.org/ > --- > drivers/usb/typec/ucsi/ucsi.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c > index c435c0835744a..69caae84879e9 100644 > --- a/drivers/usb/typec/ucsi/ucsi.c > +++ b/drivers/usb/typec/ucsi/ucsi.c > @@ -651,7 +651,8 @@ static void ucsi_unregister_altmodes(struct ucsi_connector *con, u8 recipient) > static int ucsi_get_connector_status(struct ucsi_connector *con, bool conn_ack) > { > u64 command = UCSI_GET_CONNECTOR_STATUS | UCSI_CONNECTOR_NUMBER(con->num); > - size_t size = min(UCSI_GET_CONNECTOR_STATUS_SIZE, UCSI_MAX_DATA_LENGTH(con->ucsi)); > + size_t size = min(sizeof(con->status), > + UCSI_MAX_DATA_LENGTH(con->ucsi)); > int ret; > > ret = ucsi_send_command_common(con->ucsi, command, &con->status, size, conn_ack);
diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c index c435c0835744a..69caae84879e9 100644 --- a/drivers/usb/typec/ucsi/ucsi.c +++ b/drivers/usb/typec/ucsi/ucsi.c @@ -651,7 +651,8 @@ static void ucsi_unregister_altmodes(struct ucsi_connector *con, u8 recipient) static int ucsi_get_connector_status(struct ucsi_connector *con, bool conn_ack) { u64 command = UCSI_GET_CONNECTOR_STATUS | UCSI_CONNECTOR_NUMBER(con->num); - size_t size = min(UCSI_GET_CONNECTOR_STATUS_SIZE, UCSI_MAX_DATA_LENGTH(con->ucsi)); + size_t size = min(sizeof(con->status), + UCSI_MAX_DATA_LENGTH(con->ucsi)); int ret; ret = ucsi_send_command_common(con->ucsi, command, &con->status, size, conn_ack);
Similar to commit 65c4c9447bfc ("usb: typec: ucsi: Fix a missing bits to bytes conversion in ucsi_init()"), there was a missing conversion from bits to bytes. Here the outcome is worse though: since the value is lower than UCSI_MAX_DATA_LENGTH, instead of bailing out with an error, it writes past the buffer size. The error is then seen in other places like below: Oops: general protection fault, probably for non-canonical address 0x891e812cd0ed968: 0000 [#1] PREEMPT SMP NOPTI CPU: 3 UID: 110 PID: 906 Comm: prometheus-node Not tainted 6.13.0-rc1-xe #1 Hardware name: Intel Corporation Lunar Lake Client Platform/LNL-M LP5 RVP1, BIOS LNLMFWI1.R00.3222.D84.2410171025 10/17/2024 RIP: 0010:power_supply_get_property+0x3e/0xe0 Code: 85 c0 7e 4f 4c 8b 07 89 f3 49 89 d4 49 8b 48 20 48 85 c9 74 72 49 8b 70 18 31 d2 31 c0 eb 0b 83 c2 01 48 63 c2 48 39 c8 73 5d <3b> 1c 86 75 f0 49 8b 40 28 4c 89 e2 89 de ff d0 0f 1f 00 5b 41 5c RSP: 0018:ffffc900017dfa50 EFLAGS: 00010246 RAX: 0000000000000000 RBX: 0000000000000011 RCX: c963b02c06092008 RDX: 0000000000000000 RSI: 0891e812cd0ed968 RDI: ffff888121dd6800 RBP: ffffc900017dfa68 R08: ffff88810a4024b8 R09: 0000000000000000 R10: 0000000000000000 R11: 0000000000000000 R12: ffffc900017dfa78 R13: ffff888121dd6800 R14: ffff888138ad2c00 R15: ffff88810c57c528 FS: 00007713a2ffd6c0(0000) GS:ffff88846f380000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 000000c0004b1000 CR3: 0000000121ce8003 CR4: 0000000000f72ef0 DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 DR3: 0000000000000000 DR6: 00000000ffff07f0 DR7: 0000000000000400 PKRU: 55555554 Call Trace: <TASK> ? show_regs+0x6c/0x80 ? die_addr+0x37/0xa0 ? exc_general_protection+0x1c1/0x440 ? asm_exc_general_protection+0x27/0x30 ? power_supply_get_property+0x3e/0xe0 power_supply_hwmon_read+0x50/0xe0 hwmon_attr_show+0x46/0x170 dev_attr_show+0x1a/0x70 sysfs_kf_seq_show+0xaa/0x120 kernfs_seq_show+0x41/0x60 Just use the buffer size as argument to fix it. Fixes: 226ff2e681d0 ("usb: typec: ucsi: Convert connector specific commands to bitmaps") Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> --- drivers/usb/typec/ucsi/ucsi.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)