mbox series

[SRU,N,0/1] CVE-2024-35997

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

Message

Bethany Jamison June 25, 2024, 5:46 p.m. UTC
[Impact]

HID: i2c-hid: remove I2C_HID_READ_PENDING flag to prevent lock-up

The flag I2C_HID_READ_PENDING is used to serialize I2C operations.
However, this is not necessary, because I2C core already has its own
locking for that.

More importantly, this flag can cause a lock-up: if the flag is set in
i2c_hid_xfer() and an interrupt happens, the interrupt handler
(i2c_hid_irq) will check this flag and return immediately without doing
anything, then the interrupt handler will be invoked again in an
infinite loop.

Since interrupt handler is an RT task, it takes over the CPU and the
flag-clearing task never gets scheduled, thus we have a lock-up.

Delete this unnecessary flag.

[Fix]

Noble:	Clean cherry-pick from linux-6.8.y
Jammy:	pending
Focal:	pending
Bionic:	fix sent to esm ML
Xenial: fix sent to esm ML
Trusty:	not going to be fixed by us

[Test Case]

Compile and boot tested.

[Where problems could occur]

This fix affects those who use the HID over I2C protocol 
implementation, an issue with this fix would be visible to the user
via a system freeze or crash.

Nam Cao (1):
  HID: i2c-hid: remove I2C_HID_READ_PENDING flag to prevent lock-up

 drivers/hid/i2c-hid/i2c-hid-core.c | 9 ---------
 1 file changed, 9 deletions(-)

Comments

Thibault Ferrante June 26, 2024, 8:34 a.m. UTC | #1
On 25-06-2024 19:46, Bethany Jamison wrote:
> [Impact]
> 
> HID: i2c-hid: remove I2C_HID_READ_PENDING flag to prevent lock-up
> 
> The flag I2C_HID_READ_PENDING is used to serialize I2C operations.
> However, this is not necessary, because I2C core already has its own
> locking for that.
> 
> More importantly, this flag can cause a lock-up: if the flag is set in
> i2c_hid_xfer() and an interrupt happens, the interrupt handler
> (i2c_hid_irq) will check this flag and return immediately without doing
> anything, then the interrupt handler will be invoked again in an
> infinite loop.
> 
> Since interrupt handler is an RT task, it takes over the CPU and the
> flag-clearing task never gets scheduled, thus we have a lock-up.
> 
> Delete this unnecessary flag.
> 
> [Fix]
> 
> Noble:	Clean cherry-pick from linux-6.8.y
> Jammy:	pending
> Focal:	pending
> Bionic:	fix sent to esm ML
> Xenial: fix sent to esm ML
> Trusty:	not going to be fixed by us
> 
> [Test Case]
> 
> Compile and boot tested.
> 
> [Where problems could occur]
> 
> This fix affects those who use the HID over I2C protocol
> implementation, an issue with this fix would be visible to the user
> via a system freeze or crash.
> 
> Nam Cao (1):
>    HID: i2c-hid: remove I2C_HID_READ_PENDING flag to prevent lock-up
> 
>   drivers/hid/i2c-hid/i2c-hid-core.c | 9 ---------
>   1 file changed, 9 deletions(-)
> 

Acked-by: Thibault Ferrante <thibault.ferrante@canonical.com>

--
Thibault
Manuel Diewald June 26, 2024, 2:25 p.m. UTC | #2
On Tue, Jun 25, 2024 at 12:46:06PM -0500, Bethany Jamison wrote:
> [Impact]
> 
> HID: i2c-hid: remove I2C_HID_READ_PENDING flag to prevent lock-up
> 
> The flag I2C_HID_READ_PENDING is used to serialize I2C operations.
> However, this is not necessary, because I2C core already has its own
> locking for that.
> 
> More importantly, this flag can cause a lock-up: if the flag is set in
> i2c_hid_xfer() and an interrupt happens, the interrupt handler
> (i2c_hid_irq) will check this flag and return immediately without doing
> anything, then the interrupt handler will be invoked again in an
> infinite loop.
> 
> Since interrupt handler is an RT task, it takes over the CPU and the
> flag-clearing task never gets scheduled, thus we have a lock-up.
> 
> Delete this unnecessary flag.
> 
> [Fix]
> 
> Noble:	Clean cherry-pick from linux-6.8.y
> Jammy:	pending
> Focal:	pending
> Bionic:	fix sent to esm ML
> Xenial: fix sent to esm ML
> Trusty:	not going to be fixed by us
> 
> [Test Case]
> 
> Compile and boot tested.
> 
> [Where problems could occur]
> 
> This fix affects those who use the HID over I2C protocol 
> implementation, an issue with this fix would be visible to the user
> via a system freeze or crash.
> 
> Nam Cao (1):
>   HID: i2c-hid: remove I2C_HID_READ_PENDING flag to prevent lock-up
> 
>  drivers/hid/i2c-hid/i2c-hid-core.c | 9 ---------
>  1 file changed, 9 deletions(-)
> 
> -- 
> 2.34.1
> 
> 
> -- 
> kernel-team mailing list
> kernel-team@lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team

Acked-by: Manuel Diewald <manuel.diewald@canonical.com>
Stefan Bader July 4, 2024, 5:33 p.m. UTC | #3
On 25.06.24 19:46, Bethany Jamison wrote:
> [Impact]
> 
> HID: i2c-hid: remove I2C_HID_READ_PENDING flag to prevent lock-up
> 
> The flag I2C_HID_READ_PENDING is used to serialize I2C operations.
> However, this is not necessary, because I2C core already has its own
> locking for that.
> 
> More importantly, this flag can cause a lock-up: if the flag is set in
> i2c_hid_xfer() and an interrupt happens, the interrupt handler
> (i2c_hid_irq) will check this flag and return immediately without doing
> anything, then the interrupt handler will be invoked again in an
> infinite loop.
> 
> Since interrupt handler is an RT task, it takes over the CPU and the
> flag-clearing task never gets scheduled, thus we have a lock-up.
> 
> Delete this unnecessary flag.
> 
> [Fix]
> 
> Noble:	Clean cherry-pick from linux-6.8.y
> Jammy:	pending
> Focal:	pending
> Bionic:	fix sent to esm ML
> Xenial: fix sent to esm ML
> Trusty:	not going to be fixed by us
> 
> [Test Case]
> 
> Compile and boot tested.
> 
> [Where problems could occur]
> 
> This fix affects those who use the HID over I2C protocol
> implementation, an issue with this fix would be visible to the user
> via a system freeze or crash.
> 
> Nam Cao (1):
>    HID: i2c-hid: remove I2C_HID_READ_PENDING flag to prevent lock-up
> 
>   drivers/hid/i2c-hid/i2c-hid-core.c | 9 ---------
>   1 file changed, 9 deletions(-)
> 

Rejected for the following reasons:
Already applied for  Noble update: v6.8.9 upstream stable release. CVE 
number added to commit message.

-Stefan