Commit 522018a0 authored by Baokun Li's avatar Baokun Li Committed by Christian Brauner
Browse files

cachefiles: fix slab-use-after-free in fscache_withdraw_volume() 



We got the following issue in our fault injection stress test:

==================================================================
BUG: KASAN: slab-use-after-free in fscache_withdraw_volume+0x2e1/0x370
Read of size 4 at addr ffff88810680be08 by task ondemand-04-dae/5798

CPU: 0 PID: 5798 Comm: ondemand-04-dae Not tainted 6.8.0-dirty #565
Call Trace:
 kasan_check_range+0xf6/0x1b0
 fscache_withdraw_volume+0x2e1/0x370
 cachefiles_withdraw_volume+0x31/0x50
 cachefiles_withdraw_cache+0x3ad/0x900
 cachefiles_put_unbind_pincount+0x1f6/0x250
 cachefiles_daemon_release+0x13b/0x290
 __fput+0x204/0xa00
 task_work_run+0x139/0x230

Allocated by task 5820:
 __kmalloc+0x1df/0x4b0
 fscache_alloc_volume+0x70/0x600
 __fscache_acquire_volume+0x1c/0x610
 erofs_fscache_register_volume+0x96/0x1a0
 erofs_fscache_register_fs+0x49a/0x690
 erofs_fc_fill_super+0x6c0/0xcc0
 vfs_get_super+0xa9/0x140
 vfs_get_tree+0x8e/0x300
 do_new_mount+0x28c/0x580
 [...]

Freed by task 5820:
 kfree+0xf1/0x2c0
 fscache_put_volume.part.0+0x5cb/0x9e0
 erofs_fscache_unregister_fs+0x157/0x1b0
 erofs_kill_sb+0xd9/0x1c0
 deactivate_locked_super+0xa3/0x100
 vfs_get_super+0x105/0x140
 vfs_get_tree+0x8e/0x300
 do_new_mount+0x28c/0x580
 [...]
==================================================================

Following is the process that triggers the issue:

        mount failed         |         daemon exit
------------------------------------------------------------
 deactivate_locked_super        cachefiles_daemon_release
  erofs_kill_sb
   erofs_fscache_unregister_fs
    fscache_relinquish_volume
     __fscache_relinquish_volume
      fscache_put_volume(fscache_volume, fscache_volume_put_relinquish)
       zero = __refcount_dec_and_test(&fscache_volume->ref, &ref);
                                 cachefiles_put_unbind_pincount
                                  cachefiles_daemon_unbind
                                   cachefiles_withdraw_cache
                                    cachefiles_withdraw_volumes
                                     list_del_init(&volume->cache_link)
       fscache_free_volume(fscache_volume)
        cache->ops->free_volume
         cachefiles_free_volume
          list_del_init(&cachefiles_volume->cache_link);
        kfree(fscache_volume)
                                     cachefiles_withdraw_volume
                                      fscache_withdraw_volume
                                       fscache_volume->n_accesses
                                       // fscache_volume UAF !!!

The fscache_volume in cache->volumes must not have been freed yet, but its
reference count may be 0. So use the new fscache_try_get_volume() helper
function try to get its reference count.

If the reference count of fscache_volume is 0, fscache_put_volume() is
freeing it, so wait for it to be removed from cache->volumes.

If its reference count is not 0, call cachefiles_withdraw_volume() with
reference count protection to avoid the above issue.

Fixes: fe2140e2 ("cachefiles: Implement volume support")
Signed-off-by: default avatarBaokun Li <libaokun1@huawei.com>
Link: https://lore.kernel.org/r/20240628062930.2467993-3-libaokun@huaweicloud.com


Signed-off-by: default avatarChristian Brauner <brauner@kernel.org>
parent 85b08b31

fs/cachefiles/cache.c

+10 −0
Original line number Diff line number Diff line
@@ -8,6 +8,7 @@ 
#include <linux/slab.h>

#include <linux/statfs.h>

#include <linux/namei.h>

#include <trace/events/fscache.h>

#include "internal.h"



/*
@@ -319,12 +320,20 @@ static void cachefiles_withdraw_volumes(struct cachefiles_cache *cache) 
	_enter("");



	for (;;) {

		struct fscache_volume *vcookie = NULL;

		struct cachefiles_volume *volume = NULL;



		spin_lock(&cache->object_list_lock);

		if (!list_empty(&cache->volumes)) {

			volume = list_first_entry(&cache->volumes,

						  struct cachefiles_volume, cache_link);

			vcookie = fscache_try_get_volume(volume->vcookie,

							 fscache_volume_get_withdraw);

			if (!vcookie) {

				spin_unlock(&cache->object_list_lock);

				cpu_relax();

				continue;

			}

			list_del_init(&volume->cache_link);

		}

		spin_unlock(&cache->object_list_lock);
@@ -332,6 +341,7 @@ static void cachefiles_withdraw_volumes(struct cachefiles_cache *cache) 
			break;



		cachefiles_withdraw_volume(volume);

		fscache_put_volume(vcookie, fscache_volume_put_withdraw);

	}



	_leave("");

include/trace/events/fscache.h

+4 −0
Original line number Diff line number Diff line
@@ -35,12 +35,14 @@ enum fscache_volume_trace { 
	fscache_volume_get_cookie,

	fscache_volume_get_create_work,

	fscache_volume_get_hash_collision,

	fscache_volume_get_withdraw,

	fscache_volume_free,

	fscache_volume_new_acquire,

	fscache_volume_put_cookie,

	fscache_volume_put_create_work,

	fscache_volume_put_hash_collision,

	fscache_volume_put_relinquish,

	fscache_volume_put_withdraw,

	fscache_volume_see_create_work,

	fscache_volume_see_hash_wake,

	fscache_volume_wait_create_work,
@@ -120,12 +122,14 @@ enum fscache_access_trace { 
	EM(fscache_volume_get_cookie,		"GET cook ")		\

	EM(fscache_volume_get_create_work,	"GET creat")		\

	EM(fscache_volume_get_hash_collision,	"GET hcoll")		\

	EM(fscache_volume_get_withdraw,		"GET withd")            \

	EM(fscache_volume_free,			"FREE     ")		\

	EM(fscache_volume_new_acquire,		"NEW acq  ")		\

	EM(fscache_volume_put_cookie,		"PUT cook ")		\

	EM(fscache_volume_put_create_work,	"PUT creat")		\

	EM(fscache_volume_put_hash_collision,	"PUT hcoll")		\

	EM(fscache_volume_put_relinquish,	"PUT relnq")		\

	EM(fscache_volume_put_withdraw,		"PUT withd")            \

	EM(fscache_volume_see_create_work,	"SEE creat")		\

	EM(fscache_volume_see_hash_wake,	"SEE hwake")		\

	E_(fscache_volume_wait_create_work,	"WAIT crea")