Message ID | 20240507193149.16501-1-bethany.jamison@canonical.com |
---|---|
Headers | show |
Series | CVE-2024-26893 | expand |
On 07.05.24 21:31, Bethany Jamison wrote: > [Impact] > > In the Linux kernel, the following vulnerability has been resolved: > > firmware: arm_scmi: Fix double free in SMC transport cleanup path > > When the generic SCMI code tears down a channel, it calls the chan_free > callback function, defined by each transport. Since multiple protocols > might share the same transport_info member, chan_free() might want to > clean up the same member multiple times within the given SCMI transport > implementation. In this case, it is SMC transport. This will lead to a NULL > pointer dereference at the second time: > > | scmi_protocol scmi_dev.1: Enabled polling mode TX channel - > prot_id:16 > | arm-scmi firmware:scmi: SCMI Notifications - Core Enabled. > | arm-scmi firmware:scmi: unable to communicate with SCMI > | Unable to handle kernel NULL pointer dereference at virtual address > 0000000000000000 > | Mem abort info: > | ESR = 0x0000000096000004 > | EC = 0x25: DABT (current EL), IL = 32 bits > | SET = 0, FnV = 0 > | EA = 0, S1PTW = 0 > | FSC = 0x04: level 0 translation fault > | Data abort info: > | ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000 > | CM = 0, WnR = 0, TnD = 0, TagAccess = 0 > | GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0 > | user pgtable: 4k pages, 48-bit VAs, pgdp=0000000881ef8000 > | [0000000000000000] pgd=0000000000000000, p4d=0000000000000000 > | Internal error: Oops: 0000000096000004 [#1] PREEMPT SMP > | Modules linked in: > | CPU: 4 PID: 1 Comm: swapper/0 Not tainted > 6.7.0-rc2-00124-g455ef3d016c9-dirty #793 > | Hardware name: FVP Base RevC (DT) > | pstate: 61400009 (nZCv daif +PAN -UAO -TCO +DIT -SSBS BTYPE=--) > | pc : smc_chan_free+0x3c/0x6c > | lr : smc_chan_free+0x3c/0x6c > | Call trace: > | smc_chan_free+0x3c/0x6c > | idr_for_each+0x68/0xf8 > | scmi_cleanup_channels.isra.0+0x2c/0x58 > | scmi_probe+0x434/0x734 > | platform_probe+0x68/0xd8 > | really_probe+0x110/0x27c > | __driver_probe_device+0x78/0x12c > | driver_probe_device+0x3c/0x118 > | __driver_attach+0x74/0x128 > | bus_for_each_dev+0x78/0xe0 > | driver_attach+0x24/0x30 > | bus_add_driver+0xe4/0x1e8 > | driver_register+0x60/0x128 > | __platform_driver_register+0x28/0x34 > | scmi_driver_init+0x84/0xc0 > | do_one_initcall+0x78/0x33c > | kernel_init_freeable+0x2b8/0x51c > | kernel_init+0x24/0x130 > | ret_from_fork+0x10/0x20 > | Code: f0004701 910a0021 aa1403e5 97b91c70 (b9400280) > | ---[ end trace 0000000000000000 ]--- > > Simply check for the struct pointer being NULL before trying to access > its members, to avoid this situation. > > This was found when a transport doesn't really work (for instance no SMC > service), the probe routines then tries to clean up, and triggers a crash. > > [Fix] > > Mantic: Clean cherry-pick from linux-6.6.y > Jammy: Backported - I added the if-statement from the fix commit as is > ignoring the context conflict (missing free_irq if-statement). > Focal: not-affected > Bionic: not-affected > Xenial: not-affected > Trusty: not-affected > > [Test Case] > > Compile and boot tested. > > [Where problems could occur] > > This fix affects those who use the ARM SCMI (System Control and > Management Interface) driver, an issue with this fix would be visable to > the user via a system crash. > > Andre Przywara (1): > firmware: arm_scmi: Fix double free in SMC transport cleanup path > > drivers/firmware/arm_scmi/smc.c | 7 +++++++ > 1 file changed, 7 insertions(+) > For Jammy the context of the patch is not really conclusive and looking at the code I am not sure whether this applies or might even be harmful. In Mantic some irq gets released and apart from that only two pointers get set to NULL. Then the function returns. In Jammy however there is no irq release and the function does not immediately return after having set the pointers. Instead there is a call to scmi_free_channel() which in turn calls idr_remove(). It is rather hard to say whether skipping that is equal to not calling free_irq(). Acked-by: Stefan Bader <stefan.bader@canonical.com>
On 5/7/24 21:31, Bethany Jamison wrote: > [Impact] > > In the Linux kernel, the following vulnerability has been resolved: > > firmware: arm_scmi: Fix double free in SMC transport cleanup path > > When the generic SCMI code tears down a channel, it calls the chan_free > callback function, defined by each transport. Since multiple protocols > might share the same transport_info member, chan_free() might want to > clean up the same member multiple times within the given SCMI transport > implementation. In this case, it is SMC transport. This will lead to a NULL > pointer dereference at the second time: > > | scmi_protocol scmi_dev.1: Enabled polling mode TX channel - > prot_id:16 > | arm-scmi firmware:scmi: SCMI Notifications - Core Enabled. > | arm-scmi firmware:scmi: unable to communicate with SCMI > | Unable to handle kernel NULL pointer dereference at virtual address > 0000000000000000 > | Mem abort info: > | ESR = 0x0000000096000004 > | EC = 0x25: DABT (current EL), IL = 32 bits > | SET = 0, FnV = 0 > | EA = 0, S1PTW = 0 > | FSC = 0x04: level 0 translation fault > | Data abort info: > | ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000 > | CM = 0, WnR = 0, TnD = 0, TagAccess = 0 > | GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0 > | user pgtable: 4k pages, 48-bit VAs, pgdp=0000000881ef8000 > | [0000000000000000] pgd=0000000000000000, p4d=0000000000000000 > | Internal error: Oops: 0000000096000004 [#1] PREEMPT SMP > | Modules linked in: > | CPU: 4 PID: 1 Comm: swapper/0 Not tainted > 6.7.0-rc2-00124-g455ef3d016c9-dirty #793 > | Hardware name: FVP Base RevC (DT) > | pstate: 61400009 (nZCv daif +PAN -UAO -TCO +DIT -SSBS BTYPE=--) > | pc : smc_chan_free+0x3c/0x6c > | lr : smc_chan_free+0x3c/0x6c > | Call trace: > | smc_chan_free+0x3c/0x6c > | idr_for_each+0x68/0xf8 > | scmi_cleanup_channels.isra.0+0x2c/0x58 > | scmi_probe+0x434/0x734 > | platform_probe+0x68/0xd8 > | really_probe+0x110/0x27c > | __driver_probe_device+0x78/0x12c > | driver_probe_device+0x3c/0x118 > | __driver_attach+0x74/0x128 > | bus_for_each_dev+0x78/0xe0 > | driver_attach+0x24/0x30 > | bus_add_driver+0xe4/0x1e8 > | driver_register+0x60/0x128 > | __platform_driver_register+0x28/0x34 > | scmi_driver_init+0x84/0xc0 > | do_one_initcall+0x78/0x33c > | kernel_init_freeable+0x2b8/0x51c > | kernel_init+0x24/0x130 > | ret_from_fork+0x10/0x20 > | Code: f0004701 910a0021 aa1403e5 97b91c70 (b9400280) > | ---[ end trace 0000000000000000 ]--- > > Simply check for the struct pointer being NULL before trying to access > its members, to avoid this situation. > > This was found when a transport doesn't really work (for instance no SMC > service), the probe routines then tries to clean up, and triggers a crash. > > [Fix] > > Mantic: Clean cherry-pick from linux-6.6.y > Jammy: Backported - I added the if-statement from the fix commit as is > ignoring the context conflict (missing free_irq if-statement). > Focal: not-affected > Bionic: not-affected > Xenial: not-affected > Trusty: not-affected > > [Test Case] > > Compile and boot tested. > > [Where problems could occur] > > This fix affects those who use the ARM SCMI (System Control and > Management Interface) driver, an issue with this fix would be visable to > the user via a system crash. > > Andre Przywara (1): > firmware: arm_scmi: Fix double free in SMC transport cleanup path > > drivers/firmware/arm_scmi/smc.c | 7 +++++++ > 1 file changed, 7 insertions(+) > Acked-by: Tim Gardner <tim.gardner@canonical.com>
On 5/10/24 8:07 AM, Stefan Bader wrote: > On 07.05.24 21:31, Bethany Jamison wrote: >> [Impact] >> >> In the Linux kernel, the following vulnerability has been resolved: >> >> firmware: arm_scmi: Fix double free in SMC transport cleanup path >> >> When the generic SCMI code tears down a channel, it calls the >> chan_free >> callback function, defined by each transport. Since multiple protocols >> might share the same transport_info member, chan_free() might want to >> clean up the same member multiple times within the given SCMI >> transport >> implementation. In this case, it is SMC transport. This will lead >> to a NULL >> pointer dereference at the second time: >> >> | scmi_protocol scmi_dev.1: Enabled polling mode TX channel - >> prot_id:16 >> | arm-scmi firmware:scmi: SCMI Notifications - Core Enabled. >> | arm-scmi firmware:scmi: unable to communicate with SCMI >> | Unable to handle kernel NULL pointer dereference at virtual >> address >> 0000000000000000 >> | Mem abort info: >> | ESR = 0x0000000096000004 >> | EC = 0x25: DABT (current EL), IL = 32 bits >> | SET = 0, FnV = 0 >> | EA = 0, S1PTW = 0 >> | FSC = 0x04: level 0 translation fault >> | Data abort info: >> | ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000 >> | CM = 0, WnR = 0, TnD = 0, TagAccess = 0 >> | GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0 >> | user pgtable: 4k pages, 48-bit VAs, pgdp=0000000881ef8000 >> | [0000000000000000] pgd=0000000000000000, p4d=0000000000000000 >> | Internal error: Oops: 0000000096000004 [#1] PREEMPT SMP >> | Modules linked in: >> | CPU: 4 PID: 1 Comm: swapper/0 Not tainted >> 6.7.0-rc2-00124-g455ef3d016c9-dirty #793 >> | Hardware name: FVP Base RevC (DT) >> | pstate: 61400009 (nZCv daif +PAN -UAO -TCO +DIT -SSBS BTYPE=--) >> | pc : smc_chan_free+0x3c/0x6c >> | lr : smc_chan_free+0x3c/0x6c >> | Call trace: >> | smc_chan_free+0x3c/0x6c >> | idr_for_each+0x68/0xf8 >> | scmi_cleanup_channels.isra.0+0x2c/0x58 >> | scmi_probe+0x434/0x734 >> | platform_probe+0x68/0xd8 >> | really_probe+0x110/0x27c >> | __driver_probe_device+0x78/0x12c >> | driver_probe_device+0x3c/0x118 >> | __driver_attach+0x74/0x128 >> | bus_for_each_dev+0x78/0xe0 >> | driver_attach+0x24/0x30 >> | bus_add_driver+0xe4/0x1e8 >> | driver_register+0x60/0x128 >> | __platform_driver_register+0x28/0x34 >> | scmi_driver_init+0x84/0xc0 >> | do_one_initcall+0x78/0x33c >> | kernel_init_freeable+0x2b8/0x51c >> | kernel_init+0x24/0x130 >> | ret_from_fork+0x10/0x20 >> | Code: f0004701 910a0021 aa1403e5 97b91c70 (b9400280) >> | ---[ end trace 0000000000000000 ]--- >> >> Simply check for the struct pointer being NULL before trying to access >> its members, to avoid this situation. >> >> This was found when a transport doesn't really work (for instance >> no SMC >> service), the probe routines then tries to clean up, and triggers a >> crash. >> >> [Fix] >> >> Mantic: Clean cherry-pick from linux-6.6.y >> Jammy: Backported - I added the if-statement from the fix commit >> as is >> ignoring the context conflict (missing free_irq if-statement). >> Focal: not-affected >> Bionic: not-affected >> Xenial: not-affected >> Trusty: not-affected >> >> [Test Case] >> >> Compile and boot tested. >> >> [Where problems could occur] >> >> This fix affects those who use the ARM SCMI (System Control and >> Management Interface) driver, an issue with this fix would be visable to >> the user via a system crash. >> >> Andre Przywara (1): >> firmware: arm_scmi: Fix double free in SMC transport cleanup path >> >> drivers/firmware/arm_scmi/smc.c | 7 +++++++ >> 1 file changed, 7 insertions(+) >> > For Jammy the context of the patch is not really conclusive and > looking at the code I am not sure whether this applies or might even > be harmful. In Mantic some irq gets released and apart from that only > two pointers get set to NULL. Then the function returns. > In Jammy however there is no irq release and the function does not > immediately return after having set the pointers. Instead there is a > call to scmi_free_channel() which in turn calls idr_remove(). It is > rather hard to say whether skipping that is equal to not calling > free_irq(). > > Acked-by: Stefan Bader <stefan.bader@canonical.com> For Jammy - I agree I also had some questions about the right way to apply this - I tracked down a commit (d1ff11d7ad8704f8d615f6446041c221b2d2ec4d) that adds the free_irq section but it doesn't build with cbd - I'm not sure the best way to go about fixing this for Jammy.
NACK'ing this since we don't have consensus on Jammy yet.