ACK: [SRU][M][PATCH v2 0/1] CVE-2024-24857 CVE-2024-24858

[Guoqing Jiang]

On 6/15/24 01:20, Bethany Jamison wrote:
> [Impact]
>
> Bluetooth: Fix TOCTOU in HCI debugfs implementation
>
> struct hci_dev members conn_info_max_age, conn_info_min_age,
> le_conn_max_interval, le_conn_min_interval, le_adv_max_interval,
> and le_adv_min_interval can be modified from the HCI core code, as well
> through debugfs.
>
> The debugfs implementation, that's only available to privileged users,
> will check for boundaries, making sure that the minimum value being set
> is strictly above the maximum value that already exists, and vice-versa.
>
> However, as both minimum and maximum values can be changed concurrently
> to us modifying them, we need to make sure that the value we check is
> the value we end up using.
>
> For example, with ->conn_info_max_age set to 10, conn_info_min_age_set()
> gets called from vfs handlers to set conn_info_min_age to 8.
>
> In conn_info_min_age_set(), this goes through:
> 	if (val == 0 || val > hdev->conn_info_max_age)
> 		return -EINVAL;
>
> Concurrently, conn_info_max_age_set() gets called to set to set the
> conn_info_max_age to 7:
> 	if (val == 0 || val > hdev->conn_info_max_age)
> 		return -EINVAL;
> That check will also pass because we used the old value (10) for
> conn_info_max_age.
>
> After those checks that both passed, the struct hci_dev access
> is mutex-locked, disabling concurrent access, but that does not matter
> because the invalid value checks both passed, and we'll end up with
> conn_info_min_age = 8 and conn_info_max_age = 7
>
> To fix this problem, we need to lock the structure access before so the
> check and assignment are not interrupted.
>
> This fix was originally devised by the BassCheck[1] team, and
> considered the problem to be an atomicity one. This isn't the case as
> there aren't any concerns about the variable changing while we check it,
> but rather after we check it parallel to another change.
>
> This patch fixes CVE-2024-24858 and CVE-2024-24857.
>
> [1] https://sites.google.com/view/basscheck/
>
> [Fix]
>
> Both CVE-2024-24857 and CVE-2024-24858 are fixed by the same commit and
> are sharing this patch.
>
> Noble:	pending
> Mantic:	Clean cherry-pick
> Jammy:	pending
> Focal:	fixed via stable
> Bionic:	fix sent to esm ML
> Xenial:	fix sent to esm ML
> Trusty:	not-affected
>
> [Test Case]
>
> Compile and boot tested.
>
> [Where problems could occur]
>
> This fix affects those who use bluetooth, an issue with this fix
> would be visible to the user via unexpected system behavior.
>
> v2: 	Initially I sent this patch out for just CVE-2024-24857, but this
> 	patch can also be used to completely fix CVE-2024-24858. The only
> 	changes between v1 and v2 are the labeling in the emails and the
> 	commit message.
>
> Bastien Nocera (1):
>    Bluetooth: Fix TOCTOU in HCI debugfs implementation
>
>   net/bluetooth/hci_debugfs.c | 48 ++++++++++++++++++++++++-------------
>   1 file changed, 32 insertions(+), 16 deletions(-)

Acked-by: Guoqing Jiang <guoqing.jiang at canonical.com>