APPLIED: [SRU][F][PATCH 0/1] CVE-2024-26960

[Stefan Bader]

On 05.09.24 08:36, Koichiro Den wrote:
> [Impact]
> 
> mm: swap: fix race between free_swap_and_cache() and swapoff()
> 
> There was previously a theoretical window where swapoff() could run and
> teardown a swap_info_struct while a call to free_swap_and_cache() was
> running in another thread.  This could cause, amongst other bad
> possibilities, swap_page_trans_huge_swapped() (called by
> free_swap_and_cache()) to access the freed memory for swap_map.
> 
> This is a theoretical problem and I haven't been able to provoke it from a
> test case.  But there has been agreement based on code review that this is
> possible (see link below).
> 
> Fix it by using get_swap_device()/put_swap_device(), which will stall
> swapoff().  There was an extra check in _swap_info_get() to confirm that
> the swap entry was not free.  This isn't present in get_swap_device()
> because it doesn't make sense in general due to the race between getting
> the reference and swapoff.  So I've added an equivalent check directly in
> free_swap_and_cache().
> 
> Details of how to provoke one possible issue:
> 
> --8<-----
> 
> CPU0                               CPU1
> ----                               ----
> shmem_undo_range
>    shmem_free_swap
>      xa_cmpxchg_irq
>      free_swap_and_cache
>        __swap_entry_free
>        /* swap_count() become 0 */
>                                     swapoff
>                                       try_to_unuse
>                                         shmem_unuse /* cannot find swap entry */
>                                         find_next_to_unuse
>                                         filemap_get_folio
>                                         folio_free_swap
>                                         /* remove swap cache */
>                                         /* free si->swap_map[] */
>        swap_page_trans_huge_swapped <-- access freed si->swap_map !!!
> 
> --8<-----
> 
> [Backport]
> 
> First and foremost, the follow-up v3 patch [1] offered a clearer description
> of the potential race condition and also removed an incorrect comment.
> However, it did not make it to mm-stable, while the v2 patch was merged.
> Later, the comment was revised in [2], which depends on commit
> a95722a04772 ("swap: comments get_swap_device() with usage rule").
> As this is a CVE backport patch, we typically do not merge such comment
> improvements; hence we are merging [1] only.
> 
> Additionally, there were conflicts due to missing commits:
> - commit 33e16272fe98 ("mm/swapfile.c: __swap_entry_free() always free 1 entry")
> - commit a95722a04772 ("swap: comments get_swap_device() with usage rule")
> 
> I resolved these conflicts by adjusting contexts without merging the commits.
> 
> [1] https://lore.kernel.org/all/20240311084426.447164-1-ying.huang@intel.com/
> [2] https://lkml.kernel.org/r/20240407065450.498821-1-ying.huang@intel.com
> 
> [Fix]
> 
> Noble:  fixed via stable
> Jammy:  fixed via stable
> Focal:  Backport - adjusted contexts due to missing commits, see [Backport]
> Bionic: fix sent to esm ML
> Xenial: not affected
> Trusty: not affected
> 
> [Test case]
> 
> Compile and boot tested.
> 
> Additionally, this backport was stress tested by the following procedure
> on my amd64 kvm setup with 8GB RAM. Despite extensive testing, the issue
> could not be reproduced on the kernel without this backport, likely due
> to the narrow race window and the fact that it's hard to make it happen
> so frequently. Still, this proved the robustness of the backport patch.
> 
>    # Create swap file
>    sudo fallocate -l 4G /swapfile
>    sudo chmod 600 /swapfile
>    sudo mkswap /swapfile
>    sudo mount -o remount,size=7G /dev/shm
> 
>    # Test script
>    $ cat ~/test.sh
>    #!/bin/bash
>    sudo swapon /swapfile
>    cat /dev/urandom > /dev/shm/test
>    # trigger shmem_undo_range -> free_swap_and_cache
>    echo > /dev/shm/test &
>    # try to swapoff
>    while ! sudo swapoff /swapfile; do
>      :
>    done
> 
>    # Run
>    $ while :; do ./test.sh; done
> 
> [Where problem could occur]
> 
> This fix affects all architectures and is generic in nature. It
> addresses a potential UAF that could occur if, during final
> free_swap_and_cache() for shm, a swapoff operation subtly slips in and
> frees the swap_map before swap_page_trans_huge_swapped() is invoked.
> 
> Ryan Roberts (1):
>    mm: swap: fix race between free_swap_and_cache() and swapoff()
> 
>   mm/swapfile.c | 8 +++++++-
>   1 file changed, 7 insertions(+), 1 deletion(-)
> 

Applied to focal:linux/master-next. Thanks.

-Stefan

-------------- 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/20240906/827ea819/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/20240906/827ea819/attachment-0001.sig>