From: libaokun@huaweicloud.com
To: netfs@lists.linux.dev, dhowells@redhat.com, jlayton@kernel.org
Cc: libaokun@huaweicloud.com, yangerkun@huawei.com,
linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
hsiangkao@linux.alibaba.com, linux-erofs@lists.ozlabs.org,
yukuai3@huawei.com
Subject: [PATCH v2 08/12] cachefiles: never get a new anonymous fd if ondemand_id is valid
Date: Wed, 15 May 2024 16:45:57 +0800 [thread overview]
Message-ID: <20240515084601.3240503-9-libaokun@huaweicloud.com> (raw)
In-Reply-To: <20240515084601.3240503-1-libaokun@huaweicloud.com>
From: Baokun Li <libaokun1@huawei.com>
Now every time the daemon reads an open request, it gets a new anonymous fd
and ondemand_id. With the introduction of "restore", it is possible to read
the same open request more than once, and therefore an object can have more
than one anonymous fd.
If the anonymous fd is not unique, the following concurrencies will result
in an fd leak:
t1 | t2 | t3
------------------------------------------------------------
cachefiles_ondemand_init_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
cachefiles_ondemand_get_fd
load->fd = fd0
ondemand_id = object_id0
------ restore ------
cachefiles_ondemand_restore
// restore REQ_A
cachefiles_daemon_read
cachefiles_ondemand_daemon_read
REQ_A = cachefiles_ondemand_select_req
cachefiles_ondemand_get_fd
load->fd = fd1
ondemand_id = object_id1
process_open_req(REQ_A)
write(devfd, ("copen %u,%llu", msg->msg_id, size))
cachefiles_ondemand_copen
xa_erase(&cache->reqs, id)
complete(&REQ_A->done)
kfree(REQ_A)
process_open_req(REQ_A)
// copen fails due to no req
// daemon close(fd1)
cachefiles_ondemand_fd_release
// set object closed
-- umount --
cachefiles_withdraw_cookie
cachefiles_ondemand_clean_object
cachefiles_ondemand_init_close_req
if (!cachefiles_ondemand_object_is_open(object))
return -ENOENT;
// The fd0 is not closed until the daemon exits.
However, the anonymous fd holds the reference count of the object and the
object holds the reference count of the cookie. So even though the cookie
has been relinquished, it will not be unhashed and freed until the daemon
exits.
In fscache_hash_cookie(), when the same cookie is found in the hash list,
if the cookie is set with the FSCACHE_COOKIE_RELINQUISHED bit, then the new
cookie waits for the old cookie to be unhashed, while the old cookie is
waiting for the leaked fd to be closed, if the daemon does not exit in time
it will trigger a hung task.
To avoid this, allocate a new anonymous fd only if no anonymous fd has
been allocated (ondemand_id == 0) or if the previously allocated anonymous
fd has been closed (ondemand_id == -1). Moreover, returns an error if
ondemand_id is valid, letting the daemon know that the current userland
restore logic is abnormal and needs to be checked.
Fixes: c8383054506c ("cachefiles: notify the user daemon when looking up cookie")
Signed-off-by: Baokun Li <libaokun1@huawei.com>
---
fs/cachefiles/ondemand.c | 34 ++++++++++++++++++++++++++++------
1 file changed, 28 insertions(+), 6 deletions(-)
diff --git a/fs/cachefiles/ondemand.c b/fs/cachefiles/ondemand.c
index d04ddc6576e3..d2d4e27fca6f 100644
--- a/fs/cachefiles/ondemand.c
+++ b/fs/cachefiles/ondemand.c
@@ -14,11 +14,18 @@ static int cachefiles_ondemand_fd_release(struct inode *inode,
struct file *file)
{
struct cachefiles_object *object = file->private_data;
- struct cachefiles_cache *cache = object->volume->cache;
- struct cachefiles_ondemand_info *info = object->ondemand;
+ struct cachefiles_cache *cache;
+ struct cachefiles_ondemand_info *info;
int object_id;
struct cachefiles_req *req;
- XA_STATE(xas, &cache->reqs, 0);
+ XA_STATE(xas, NULL, 0);
+
+ if (!object)
+ return 0;
+
+ info = object->ondemand;
+ cache = object->volume->cache;
+ xas.xa = &cache->reqs;
xa_lock(&cache->reqs);
spin_lock(&info->lock);
@@ -288,22 +295,39 @@ static int cachefiles_ondemand_get_fd(struct cachefiles_req *req)
goto err_put_fd;
}
+ spin_lock(&object->ondemand->lock);
+ if (object->ondemand->ondemand_id > 0) {
+ spin_unlock(&object->ondemand->lock);
+ /* Pair with check in cachefiles_ondemand_fd_release(). */
+ file->private_data = NULL;
+ ret = -EEXIST;
+ goto err_put_file;
+ }
+
file->f_mode |= FMODE_PWRITE | FMODE_LSEEK;
fd_install(fd, file);
load = (void *)req->msg.data;
load->fd = fd;
object->ondemand->ondemand_id = object_id;
+ spin_unlock(&object->ondemand->lock);
cachefiles_get_unbind_pincount(cache);
trace_cachefiles_ondemand_open(object, &req->msg, load);
return 0;
+err_put_file:
+ fput(file);
err_put_fd:
put_unused_fd(fd);
err_free_id:
xa_erase(&cache->ondemand_ids, object_id);
err:
+ spin_lock(&object->ondemand->lock);
+ /* Avoid marking an opened object as closed. */
+ if (object->ondemand->ondemand_id <= 0)
+ cachefiles_ondemand_set_object_close(object);
+ spin_unlock(&object->ondemand->lock);
cachefiles_put_object(object, cachefiles_obj_put_ondemand_fd);
return ret;
}
@@ -386,10 +410,8 @@ ssize_t cachefiles_ondemand_daemon_read(struct cachefiles_cache *cache,
if (msg->opcode == CACHEFILES_OP_OPEN) {
ret = cachefiles_ondemand_get_fd(req);
- if (ret) {
- cachefiles_ondemand_set_object_close(req->object);
+ if (ret)
goto out;
- }
}
msg->msg_id = xas.xa_index;
--
2.39.2
next prev parent reply other threads:[~2024-05-15 8:57 UTC|newest]
Thread overview: 38+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-15 8:45 [PATCH v2 00/12] cachefiles: some bugfixes and cleanups for ondemand requests libaokun
2024-05-15 8:45 ` [PATCH v2 01/12] cachefiles: remove request from xarry during flush requests libaokun
2024-05-20 2:20 ` Gao Xiang
2024-05-20 4:11 ` Baokun Li
2024-05-20 7:09 ` Jingbo Xu
2024-05-15 8:45 ` [PATCH v2 02/12] cachefiles: remove err_put_fd tag in cachefiles_ondemand_daemon_read() libaokun
2024-05-20 2:23 ` Gao Xiang
2024-05-20 4:15 ` Baokun Li
2024-05-15 8:45 ` [PATCH v2 03/12] cachefiles: fix slab-use-after-free in cachefiles_ondemand_get_fd() libaokun
2024-05-20 7:24 ` Jingbo Xu
2024-05-20 8:38 ` Baokun Li
2024-05-20 8:45 ` Gao Xiang
2024-05-20 9:10 ` Jingbo Xu
2024-05-20 9:19 ` Baokun Li
2024-05-20 12:22 ` Baokun Li
2024-05-20 8:06 ` Jingbo Xu
2024-05-20 9:10 ` Baokun Li
2024-05-15 8:45 ` [PATCH v2 04/12] cachefiles: fix slab-use-after-free in cachefiles_ondemand_daemon_read() libaokun
2024-05-20 7:36 ` Jingbo Xu
2024-05-20 8:56 ` Baokun Li
2024-05-15 8:45 ` [PATCH v2 05/12] cachefiles: add output string to cachefiles_obj_[get|put]_ondemand_fd libaokun
2024-05-20 7:40 ` Jingbo Xu
2024-05-20 9:02 ` Baokun Li
2024-05-15 8:45 ` [PATCH v2 06/12] cachefiles: add consistency check for copen/cread libaokun
2024-05-15 8:45 ` [PATCH v2 07/12] cachefiles: add spin_lock for cachefiles_ondemand_info libaokun
2024-05-15 8:45 ` libaokun [this message]
2024-05-20 8:43 ` [PATCH v2 08/12] cachefiles: never get a new anonymous fd if ondemand_id is valid Jingbo Xu
2024-05-20 9:07 ` Baokun Li
2024-05-20 9:24 ` Jingbo Xu
2024-05-20 11:14 ` Baokun Li
2024-05-20 11:24 ` Gao Xiang
2024-05-15 8:45 ` [PATCH v2 09/12] cachefiles: defer exposing anon_fd until after copy_to_user() succeeds libaokun
2024-05-20 9:39 ` Jingbo Xu
2024-05-20 11:36 ` Baokun Li
2024-05-15 8:45 ` [PATCH v2 10/12] cachefiles: Set object to close if ondemand_id < 0 in copen libaokun
2024-05-15 8:46 ` [PATCH v2 11/12] cachefiles: flush all requests after setting CACHEFILES_DEAD libaokun
2024-05-15 8:46 ` [PATCH v2 12/12] cachefiles: make on-demand read killable libaokun
2024-05-19 10:56 ` [PATCH v2 00/12] cachefiles: some bugfixes and cleanups for ondemand requests Jeff Layton
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=20240515084601.3240503-9-libaokun@huaweicloud.com \
--to=libaokun@huaweicloud.com \
--cc=dhowells@redhat.com \
--cc=hsiangkao@linux.alibaba.com \
--cc=jlayton@kernel.org \
--cc=linux-erofs@lists.ozlabs.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=netfs@lists.linux.dev \
--cc=yangerkun@huawei.com \
--cc=yukuai3@huawei.com \
/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).