ACK[M]/Cmnt[J]: [SRU][M/J][PATCH 0/1] CVE-2024-26893

[Stefan Bader]

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 at canonical.com>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_0xE8675DEECBEECEA3.asc
Type: application/pgp-keys
Size: 48643 bytes
Desc: OpenPGP public key
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20240510/e3a5e822/attachment-0001.key>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20240510/e3a5e822/attachment-0001.sig>