5d8f805789
We got the following issue in our fault injection stress test:
==================================================================
BUG: KASAN: slab-use-after-free in cachefiles_withdraw_cookie+0x4d9/0x600
Read of size 8 at addr ffff888118efc000 by task kworker/u78:0/109
CPU: 13 PID: 109 Comm: kworker/u78:0 Not tainted 6.8.0-dirty #566
Call Trace:
<TASK>
kasan_report+0x93/0xc0
cachefiles_withdraw_cookie+0x4d9/0x600
fscache_cookie_state_machine+0x5c8/0x1230
fscache_cookie_worker+0x91/0x1c0
process_one_work+0x7fa/0x1800
[...]
Allocated by task 117:
kmalloc_trace+0x1b3/0x3c0
cachefiles_acquire_volume+0xf3/0x9c0
fscache_create_volume_work+0x97/0x150
process_one_work+0x7fa/0x1800
[...]
Freed by task 120301:
kfree+0xf1/0x2c0
cachefiles_withdraw_cache+0x3fa/0x920
cachefiles_put_unbind_pincount+0x1f6/0x250
cachefiles_daemon_release+0x13b/0x290
__fput+0x204/0xa00
task_work_run+0x139/0x230
do_exit+0x87a/0x29b0
[...]
==================================================================
Following is the process that triggers the issue:
p1 | p2
------------------------------------------------------------
fscache_begin_lookup
fscache_begin_volume_access
fscache_cache_is_live(fscache_cache)
cachefiles_daemon_release
cachefiles_put_unbind_pincount
cachefiles_daemon_unbind
cachefiles_withdraw_cache
fscache_withdraw_cache
fscache_set_cache_state(cache, FSCACHE_CACHE_IS_WITHDRAWN);
cachefiles_withdraw_objects(cache)
fscache_wait_for_objects(fscache)
atomic_read(&fscache_cache->object_count) == 0
fscache_perform_lookup
cachefiles_lookup_cookie
cachefiles_alloc_object
refcount_set(&object->ref, 1);
object->volume = volume
fscache_count_object(vcookie->cache);
atomic_inc(&fscache_cache->object_count)
cachefiles_withdraw_volumes
cachefiles_withdraw_volume
fscache_withdraw_volume
__cachefiles_free_volume
kfree(cachefiles_volume)
fscache_cookie_state_machine
cachefiles_withdraw_cookie
cache = object->volume->cache;
// cachefiles_volume UAF !!!
After setting FSCACHE_CACHE_IS_WITHDRAWN, wait for all the cookie lookups
to complete first, and then wait for fscache_cache->object_count == 0 to
avoid the cookie exiting after the volume has been freed and triggering
the above issue. Therefore call fscache_withdraw_volume() before calling
cachefiles_withdraw_objects().
This way, after setting FSCACHE_CACHE_IS_WITHDRAWN, only the following two
cases will occur:
1) fscache_begin_lookup fails in fscache_begin_volume_access().
2) fscache_withdraw_volume() will ensure that fscache_count_object() has
been executed before calling fscache_wait_for_objects().
Fixes: fe2140e2f5
("cachefiles: Implement volume support")
Suggested-by: Hou Tao <houtao1@huawei.com>
Signed-off-by: Baokun Li <libaokun1@huawei.com>
Link: https://lore.kernel.org/r/20240628062930.2467993-4-libaokun@huaweicloud.com
Signed-off-by: Christian Brauner <brauner@kernel.org>
139 lines
3.3 KiB
C
139 lines
3.3 KiB
C
// SPDX-License-Identifier: GPL-2.0-or-later
|
|
/* Volume handling.
|
|
*
|
|
* Copyright (C) 2021 Red Hat, Inc. All Rights Reserved.
|
|
* Written by David Howells (dhowells@redhat.com)
|
|
*/
|
|
|
|
#include <linux/fs.h>
|
|
#include <linux/slab.h>
|
|
#include "internal.h"
|
|
#include <trace/events/fscache.h>
|
|
|
|
/*
|
|
* Allocate and set up a volume representation. We make sure all the fanout
|
|
* directories are created and pinned.
|
|
*/
|
|
void cachefiles_acquire_volume(struct fscache_volume *vcookie)
|
|
{
|
|
struct cachefiles_volume *volume;
|
|
struct cachefiles_cache *cache = vcookie->cache->cache_priv;
|
|
const struct cred *saved_cred;
|
|
struct dentry *vdentry, *fan;
|
|
size_t len;
|
|
char *name;
|
|
bool is_new = false;
|
|
int ret, n_accesses, i;
|
|
|
|
_enter("");
|
|
|
|
volume = kzalloc(sizeof(struct cachefiles_volume), GFP_KERNEL);
|
|
if (!volume)
|
|
return;
|
|
volume->vcookie = vcookie;
|
|
volume->cache = cache;
|
|
INIT_LIST_HEAD(&volume->cache_link);
|
|
|
|
cachefiles_begin_secure(cache, &saved_cred);
|
|
|
|
len = vcookie->key[0];
|
|
name = kmalloc(len + 3, GFP_NOFS);
|
|
if (!name)
|
|
goto error_vol;
|
|
name[0] = 'I';
|
|
memcpy(name + 1, vcookie->key + 1, len);
|
|
name[len + 1] = 0;
|
|
|
|
retry:
|
|
vdentry = cachefiles_get_directory(cache, cache->store, name, &is_new);
|
|
if (IS_ERR(vdentry))
|
|
goto error_name;
|
|
volume->dentry = vdentry;
|
|
|
|
if (is_new) {
|
|
if (!cachefiles_set_volume_xattr(volume))
|
|
goto error_dir;
|
|
} else {
|
|
ret = cachefiles_check_volume_xattr(volume);
|
|
if (ret < 0) {
|
|
if (ret != -ESTALE)
|
|
goto error_dir;
|
|
inode_lock_nested(d_inode(cache->store), I_MUTEX_PARENT);
|
|
cachefiles_bury_object(cache, NULL, cache->store, vdentry,
|
|
FSCACHE_VOLUME_IS_WEIRD);
|
|
cachefiles_put_directory(volume->dentry);
|
|
cond_resched();
|
|
goto retry;
|
|
}
|
|
}
|
|
|
|
for (i = 0; i < 256; i++) {
|
|
sprintf(name, "@%02x", i);
|
|
fan = cachefiles_get_directory(cache, vdentry, name, NULL);
|
|
if (IS_ERR(fan))
|
|
goto error_fan;
|
|
volume->fanout[i] = fan;
|
|
}
|
|
|
|
cachefiles_end_secure(cache, saved_cred);
|
|
|
|
vcookie->cache_priv = volume;
|
|
n_accesses = atomic_inc_return(&vcookie->n_accesses); /* Stop wakeups on dec-to-0 */
|
|
trace_fscache_access_volume(vcookie->debug_id, 0,
|
|
refcount_read(&vcookie->ref),
|
|
n_accesses, fscache_access_cache_pin);
|
|
|
|
spin_lock(&cache->object_list_lock);
|
|
list_add(&volume->cache_link, &volume->cache->volumes);
|
|
spin_unlock(&cache->object_list_lock);
|
|
|
|
kfree(name);
|
|
return;
|
|
|
|
error_fan:
|
|
for (i = 0; i < 256; i++)
|
|
cachefiles_put_directory(volume->fanout[i]);
|
|
error_dir:
|
|
cachefiles_put_directory(volume->dentry);
|
|
error_name:
|
|
kfree(name);
|
|
error_vol:
|
|
kfree(volume);
|
|
cachefiles_end_secure(cache, saved_cred);
|
|
}
|
|
|
|
/*
|
|
* Release a volume representation.
|
|
*/
|
|
static void __cachefiles_free_volume(struct cachefiles_volume *volume)
|
|
{
|
|
int i;
|
|
|
|
_enter("");
|
|
|
|
volume->vcookie->cache_priv = NULL;
|
|
|
|
for (i = 0; i < 256; i++)
|
|
cachefiles_put_directory(volume->fanout[i]);
|
|
cachefiles_put_directory(volume->dentry);
|
|
kfree(volume);
|
|
}
|
|
|
|
void cachefiles_free_volume(struct fscache_volume *vcookie)
|
|
{
|
|
struct cachefiles_volume *volume = vcookie->cache_priv;
|
|
|
|
if (volume) {
|
|
spin_lock(&volume->cache->object_list_lock);
|
|
list_del_init(&volume->cache_link);
|
|
spin_unlock(&volume->cache->object_list_lock);
|
|
__cachefiles_free_volume(volume);
|
|
}
|
|
}
|
|
|
|
void cachefiles_withdraw_volume(struct cachefiles_volume *volume)
|
|
{
|
|
cachefiles_set_volume_xattr(volume);
|
|
__cachefiles_free_volume(volume);
|
|
}
|