Christian Brauner <[email protected]> writes: > generic_ci_d_hash() and generic_ci_d_compare() read sb->s_encoding and > dereference the unicode map while a pathwalk in rcu mode holds nothing > but rcu_read_lock(). The filesystems that use them, ext4, f2fs and > tmpfs, drop the map with utf8_unload() from ->put_super(), which runs > when the last reference to the mount goes away. A lookup that is still > inside such a filesystem at that point reads a freed map. > > With a pathwalk held up in utf8_casefold_hash() while the last reference > to a casefolded tmpfs goes away: > > BUG: KASAN: slab-use-after-free in utf8nlookup+0x74c/0x950 > Read of size 8 at addr ff110001030fe658 by task walker/98 > Call Trace: > <TASK> > utf8nlookup+0x74c/0x950 > utf8byte+0x160/0x1560 > utf8_casefold_hash+0x167/0x230 > generic_ci_d_hash+0xdd/0x1c0 > link_path_walk+0x11ad/0x1d30 > path_lookupat+0x63/0xb00 > filename_lookup+0x1f1/0x5b0 > vfs_statx+0xd7/0x3b0 > [...] > Allocated by task 94: > __kmalloc_cache_noprof+0x249/0x560 > utf8_load+0x4e/0x570 > shmem_parse_opt_casefold.isra.0+0x92/0x270 > [...] > Freed by task 97: > kfree+0x1db/0x5b0 > shmem_put_super+0x72/0x170 > generic_shutdown_super+0x14c/0x530 > kill_anon_super+0x3a/0x60 > deactivate_locked_super+0x9c/0xf0 > cleanup_mnt+0x9f/0x130 > > So far that has been hard to hit. umount(2) waits for a grace period > in namespace_unlock() before it drops the mount, so only a mount whose > last reference goes away elsewhere, like the close of a file or a chdir > out of a lazily unmounted directory, can race a walker. So it is > reachable. > > Delay the free until a grace period has passed. The map and the data > table it holds a reference to then outlive any pathwalk that found the > superblock before the mount went away. The reference on the data table > module is dropped from the rcu callback as well. utf8-core is built in > whenever CONFIG_UNICODE is set, so there is no module exit that would > have to wait for the callbacks. > > Overlayfs points the s_encoding of an overlay at the map of its first > layer without taking a reference of its own. That stays correct. The > layer is held by the overlay, so its map is dropped only after the > overlay is gone. > > Fixes: b886ee3e778e ("ext4: Support case-insensitive file name lookups") > Cc: [email protected] > Signed-off-by: Christian Brauner (Amutable) <[email protected]>
Hey, Christian! Interesting find and looks good to me. Feel free to add: Reviewed-by: Gabriel Krisman Bertazi <[email protected]> thanks, -- Gabriel Krisman Bertazi
