Commit 400e825f authored by Baokun Li's avatar Baokun Li Committed by Zizhi Wo
Browse files

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

mainline inclusion
from mainline-v6.10-rc4
commit da4a827416066191aafeeccee50a8836a826ba10
category: feature
bugzilla: https://gitee.com/openeuler/kernel/issues/IA6I1T

Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=da4a827416066191aafeeccee50a8836a826ba10



--------------------------------

We got the following issue in a fuzz test of randomly issuing the restore
command:

==================================================================
BUG: KASAN: slab-use-after-free in cachefiles_ondemand_daemon_read+0xb41/0xb60
Read of size 8 at addr ffff888122e84088 by task ondemand-04-dae/963

CPU: 13 PID: 963 Comm: ondemand-04-dae Not tainted 6.8.0-dirty #564
Call Trace:
 kasan_report+0x93/0xc0
 cachefiles_ondemand_daemon_read+0xb41/0xb60
 vfs_read+0x169/0xb50
 ksys_read+0xf5/0x1e0

Allocated by task 116:
 kmem_cache_alloc+0x140/0x3a0
 cachefiles_lookup_cookie+0x140/0xcd0
 fscache_cookie_state_machine+0x43c/0x1230
 [...]

Freed by task 792:
 kmem_cache_free+0xfe/0x390
 cachefiles_put_object+0x241/0x480
 fscache_cookie_state_machine+0x5c8/0x1230
 [...]
==================================================================

Following is the process that triggers the issue:

     mount  |   daemon_thread1    |    daemon_thread2
------------------------------------------------------------
cachefiles_withdraw_cookie
 cachefiles_ondemand_clean_object(object)
  cachefiles_ondemand_send_req
   REQ_A = kzalloc(sizeof(*req) + data_len)
   wait_for_completion(&REQ_A->done)

            cachefiles_daemon_read
             cachefiles_ondemand_daemon_read
              REQ_A = cachefiles_ondemand_select_req
              msg->object_id = req->object->ondemand->ondemand_id
                                  ------ restore ------
                                  cachefiles_ondemand_restore
                                  xas_for_each(&xas, req, ULONG_MAX)
                                   xas_set_mark(&xas, CACHEFILES_REQ_NEW)

                                  cachefiles_daemon_read
                                   cachefiles_ondemand_daemon_read
                                    REQ_A = cachefiles_ondemand_select_req
              copy_to_user(_buffer, msg, n)
               xa_erase(&cache->reqs, id)
               complete(&REQ_A->done)
              ------ close(fd) ------
              cachefiles_ondemand_fd_release
               cachefiles_put_object
 cachefiles_put_object
  kmem_cache_free(cachefiles_object_jar, object)
                                    REQ_A->object->ondemand->ondemand_id
                                     // object UAF !!!

When we see the request within xa_lock, req->object must not have been
freed yet, so grab the reference count of object before xa_unlock to
avoid the above issue.

Fixes: 0a7e54c1959c ("cachefiles: resend an open request if the read request's object is closed")
Signed-off-by: default avatarBaokun Li <libaokun1@huawei.com>
Link: https://lore.kernel.org/r/20240522114308.2402121-5-libaokun@huaweicloud.com


Acked-by: default avatarJeff Layton <jlayton@kernel.org>
Reviewed-by: default avatarJia Zhu <zhujia.zj@bytedance.com>
Reviewed-by: default avatarJingbo Xu <jefflexu@linux.alibaba.com>
Signed-off-by: default avatarChristian Brauner <brauner@kernel.org>
Signed-off-by: default avatarZizhi Wo <wozizhi@huawei.com>
parent 5dd77866

fs/cachefiles/ondemand.c

+3 −0
Original line number Diff line number Diff line
@@ -337,6 +337,7 @@ ssize_t cachefiles_ondemand_daemon_read(struct cachefiles_cache *cache, 
	xas_clear_mark(&xas, CACHEFILES_REQ_NEW);

	cache->req_id_next = xas.xa_index + 1;

	refcount_inc(&req->ref);

	cachefiles_grab_object(req->object, cachefiles_obj_get_read_req);

	xa_unlock(&cache->reqs);



	id = xas.xa_index;
@@ -357,6 +358,7 @@ ssize_t cachefiles_ondemand_daemon_read(struct cachefiles_cache *cache, 
		goto err_put_fd;

	}



	cachefiles_put_object(req->object, cachefiles_obj_put_read_req);

	/* CLOSE request has no reply */

	if (msg->opcode == CACHEFILES_OP_CLOSE) {

		xa_erase(&cache->reqs, id);
@@ -370,6 +372,7 @@ ssize_t cachefiles_ondemand_daemon_read(struct cachefiles_cache *cache, 
	if (msg->opcode == CACHEFILES_OP_OPEN)

		close_fd(((struct cachefiles_open *)msg->data)->fd);

error:

	cachefiles_put_object(req->object, cachefiles_obj_put_read_req);

	xas_reset(&xas);

	xas_lock(&xas);

	if (xas_load(&xas) == req) {

include/trace/events/cachefiles.h

+5 −1
Original line number Diff line number Diff line
@@ -33,6 +33,8 @@ enum cachefiles_obj_ref_trace { 
	cachefiles_obj_see_withdrawal,

	cachefiles_obj_get_ondemand_fd,

	cachefiles_obj_put_ondemand_fd,

	cachefiles_obj_get_read_req,

	cachefiles_obj_put_read_req,

};



enum fscache_why_object_killed {
@@ -129,7 +131,9 @@ enum cachefiles_error_trace { 
	EM(cachefiles_obj_see_withdraw_cookie,	"SEE withdraw_cookie")	\

	EM(cachefiles_obj_see_withdrawal,	"SEE withdrawal")	\

	EM(cachefiles_obj_get_ondemand_fd,      "GET ondemand_fd")	\

	E_(cachefiles_obj_put_ondemand_fd,      "PUT ondemand_fd")

	EM(cachefiles_obj_put_ondemand_fd,      "PUT ondemand_fd")	\

	EM(cachefiles_obj_get_read_req,		"GET read_req")		\

	E_(cachefiles_obj_put_read_req,		"PUT read_req")



#define cachefiles_coherency_traces					\

	EM(cachefiles_coherency_check_aux,	"BAD aux ")		\