mbox series

[SRU,M/J,0/1] CVE-2024-26893

Message ID 20240507193149.16501-1-bethany.jamison@canonical.com
Headers show
Series CVE-2024-26893 | expand

Message

Bethany Jamison May 7, 2024, 7:31 p.m. UTC
[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(+)

Comments

Stefan Bader May 10, 2024, 1:07 p.m. UTC | #1
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>
Tim Gardner May 13, 2024, 2:52 p.m. UTC | #2
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>
Bethany Jamison May 28, 2024, 1:54 p.m. UTC | #3
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.
Cengiz Can May 31, 2024, 9:15 a.m. UTC | #4
NACK'ing this since we don't have consensus on Jammy yet.