linux-kernel - Re: [PATCH v3 1/5] Xarray: Do not return sibling entries from xas_find

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1f8b523e-d68f-4382-8b1e-2475eb47ae81@linux.alibaba.com>
Date: Fri, 13 Dec 2024 14:12:47 +0800
From: Baolin Wang <baolin.wang@...ux.alibaba.com>
To: Kemeng Shi <shikemeng@...weicloud.com>, akpm@...ux-foundation.org,
 willy@...radead.org
Cc: linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org,
 linux-mm@...ck.org
Subject: Re: [PATCH v3 1/5] Xarray: Do not return sibling entries from
 xas_find_marked()



On 2024/12/13 20:25, Kemeng Shi wrote:
> Similar to issue fixed in commit cbc02854331ed ("XArray: Do not return
> sibling entries from xa_load()"), we may return sibling entries from
> xas_find_marked as following:
>      Thread A:               Thread B:
>                              xa_store_range(xa, entry, 6, 7, gfp);
> 			    xa_set_mark(xa, 6, mark)
>      XA_STATE(xas, xa, 6);
>      xas_find_marked(&xas, 7, mark);
>      offset = xas_find_chunk(xas, advance, mark);
>      [offset is 6 which points to a valid entry]
>                              xa_store_range(xa, entry, 4, 7, gfp);
>      entry = xa_entry(xa, node, 6);
>      [entry is a sibling of 4]
>      if (!xa_is_node(entry))
>          return entry;
> 
> Skip sibling entry like xas_find() does to protect caller from seeing
> sibling entry from xas_find_marked() or caller may use sibling entry
> as a valid entry and crash the kernel.
> 
> Besides, load_race() test is modified to catch mentioned issue and modified
> load_race() only passes after this fix is merged.
> 
> Here is an example how this bug could be triggerred in tmpfs which
> enables large folio in mapping:
> Let's take a look at involved racer:
> 1. How pages could be created and dirtied in shmem file.
> write
>   ksys_write
>    vfs_write
>     new_sync_write
>      shmem_file_write_iter
>       generic_perform_write
>        shmem_write_begin
>         shmem_get_folio
>          shmem_allowable_huge_orders
>          shmem_alloc_and_add_folios
>          shmem_alloc_folio
>          __folio_set_locked
>          shmem_add_to_page_cache
>           XA_STATE_ORDER(..., index, order)
>           xax_store()
>        shmem_write_end
>         folio_mark_dirty()
> 
> 2. How dirty pages could be deleted in shmem file.
> ioctl
>   do_vfs_ioctl
>    file_ioctl
>     ioctl_preallocate
>      vfs_fallocate
>       shmem_fallocate
>        shmem_truncate_range
>         shmem_undo_range
>          truncate_inode_folio
>           filemap_remove_folio
>            page_cache_delete
>             xas_store(&xas, NULL);
> 
> 3. How dirty pages could be lockless searched
> sync_file_range
>   ksys_sync_file_range
>    __filemap_fdatawrite_range
>     filemap_fdatawrite_wbc

Seems not a good example, IIUC, tmpfs doesn't support writeback 
(mapping_can_writeback() will return false), right?

>      do_writepages
>       writeback_use_writepage
>        writeback_iter
>         writeback_get_folio
>          filemap_get_folios_tag
>           find_get_entry
>            folio = xas_find_marked()
>            folio_try_get(folio)
>