From: Jia Zhu <zhujia.zj@bytedance.com>
To: libaokun@huaweicloud.com, netfs@lists.linux.dev
Cc: dhowells@redhat.com, jlayton@kernel.org,
jefflexu@linux.alibaba.com, linux-erofs@lists.ozlabs.org,
linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
Baokun Li <libaokun1@huawei.com>,
zhujia.zj@bytedance.com
Subject: Re: [External] [PATCH 04/12] cachefiles: fix slab-use-after-free in cachefiles_ondemand_daemon_read()
Date: Thu, 25 Apr 2024 11:42:28 +0800 [thread overview]
Message-ID: <c359d4d3-5aff-4536-983d-87af3198724d@bytedance.com> (raw)
In-Reply-To: <20240424033916.2748488-5-libaokun@huaweicloud.com>
在 2024/4/24 11:39, libaokun@huaweicloud.com 写道:
> From: Baokun Li <libaokun1@huawei.com>
>
> 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: Baokun Li <libaokun1@huawei.com>
Reviewed-by: Jia Zhu <zhujia.zj@bytedance.com>
> ---
> fs/cachefiles/ondemand.c | 2 ++
> include/trace/events/cachefiles.h | 6 +++++-
> 2 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/fs/cachefiles/ondemand.c b/fs/cachefiles/ondemand.c
> index 56d12fe4bf73..bb94ef6a6f61 100644
> --- a/fs/cachefiles/ondemand.c
> +++ b/fs/cachefiles/ondemand.c
> @@ -336,6 +336,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);
>
> if (msg->opcode == CACHEFILES_OP_OPEN) {
> @@ -355,6 +356,7 @@ ssize_t cachefiles_ondemand_daemon_read(struct cachefiles_cache *cache,
> close_fd(((struct cachefiles_open *)msg->data)->fd);
> }
> out:
> + cachefiles_put_object(req->object, cachefiles_obj_put_read_req);
> /* Remove error request and CLOSE request has no reply */
> if (ret || msg->opcode == CACHEFILES_OP_CLOSE) {
> xas_reset(&xas);
> diff --git a/include/trace/events/cachefiles.h b/include/trace/events/cachefiles.h
> index cf4b98b9a9ed..119a823fb5a0 100644
> --- a/include/trace/events/cachefiles.h
> +++ b/include/trace/events/cachefiles.h
> @@ -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 {
> @@ -127,7 +129,9 @@ enum cachefiles_error_trace {
> EM(cachefiles_obj_see_lookup_cookie, "SEE lookup_cookie") \
> EM(cachefiles_obj_see_lookup_failed, "SEE lookup_failed") \
> EM(cachefiles_obj_see_withdraw_cookie, "SEE withdraw_cookie") \
> - E_(cachefiles_obj_see_withdrawal, "SEE withdrawal")
> + EM(cachefiles_obj_see_withdrawal, "SEE withdrawal") \
> + 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 ") \
next prev parent reply other threads:[~2024-04-25 3:42 UTC|newest]
Thread overview: 35+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-24 3:39 [PATCH 00/12] cachefiles: some bugfixes and cleanups for ondemand requests libaokun
2024-04-24 3:39 ` [PATCH 01/12] cachefiles: remove request from xarry during flush requests libaokun
2024-04-25 3:13 ` Jia Zhu
2024-05-06 3:48 ` Jingbo Xu
2024-05-06 3:57 ` Baokun Li
2024-05-06 5:50 ` Jingbo Xu
2024-05-07 6:52 ` Baokun Li
2024-04-24 3:39 ` [PATCH 02/12] cachefiles: remove err_put_fd tag in cachefiles_ondemand_daemon_read() libaokun
2024-04-25 3:17 ` Jia Zhu
2024-05-06 3:55 ` Jingbo Xu
2024-05-06 4:02 ` Baokun Li
2024-04-24 3:39 ` [PATCH 03/12] cachefiles: fix slab-use-after-free in cachefiles_ondemand_get_fd() libaokun
2024-04-24 14:55 ` Jia Zhu
2024-04-25 1:33 ` Baokun Li
2024-04-25 3:39 ` Jia Zhu
2024-04-24 3:39 ` [PATCH 04/12] cachefiles: fix slab-use-after-free in cachefiles_ondemand_daemon_read() libaokun
2024-04-25 3:42 ` Jia Zhu [this message]
2024-04-24 3:39 ` [PATCH 05/12] cachefiles: add output string to cachefiles_obj_[get|put]_ondemand_fd libaokun
2024-04-24 3:39 ` [PATCH 06/12] cachefiles: add consistency check for copen/cread libaokun
2024-05-06 2:31 ` Jingbo Xu
2024-05-06 3:12 ` Baokun Li
2024-04-24 3:39 ` [PATCH 07/12] cachefiles: add spin_lock for cachefiles_ondemand_info libaokun
2024-05-06 2:55 ` Jingbo Xu
2024-05-06 3:23 ` Baokun Li
2024-04-24 3:39 ` [PATCH 08/12] cachefiles: never get a new anon fd if ondemand_id is valid libaokun
2024-05-06 3:09 ` Jingbo Xu
2024-05-07 9:32 ` Baokun Li
2024-04-24 3:39 ` [PATCH 09/12] cachefiles: defer exposing anon_fd until after copy_to_user() succeeds libaokun
2024-05-06 3:24 ` Jingbo Xu
2024-05-06 3:34 ` Baokun Li
2024-04-24 3:39 ` [PATCH 10/12] cachefiles: Set object to close if ondemand_id < 0 in copen libaokun
2024-04-25 4:56 ` Jia Zhu
2024-04-24 3:39 ` [PATCH 11/12] cachefiles: flush all requests after setting CACHEFILES_DEAD libaokun
2024-04-24 3:39 ` [PATCH 12/12] cachefiles: make on-demand read killable libaokun
2024-04-25 5:15 ` Jia Zhu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=c359d4d3-5aff-4536-983d-87af3198724d@bytedance.com \
--to=zhujia.zj@bytedance.com \
--cc=dhowells@redhat.com \
--cc=jefflexu@linux.alibaba.com \
--cc=jlayton@kernel.org \
--cc=libaokun1@huawei.com \
--cc=libaokun@huaweicloud.com \
--cc=linux-erofs@lists.ozlabs.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=netfs@lists.linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).