LKML Archive mirror
 help / color / mirror / Atom feed
From: libaokun@huaweicloud.com
To: netfs@lists.linux.dev
Cc: dhowells@redhat.com, jlayton@kernel.org, zhujia.zj@bytedance.com,
	jefflexu@linux.alibaba.com, linux-erofs@lists.ozlabs.org,
	linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
	libaokun@huaweicloud.com, Baokun Li <libaokun1@huawei.com>,
	Hou Tao <houtao1@huawei.com>
Subject: [PATCH 09/12] cachefiles: defer exposing anon_fd until after copy_to_user() succeeds
Date: Wed, 24 Apr 2024 11:39:13 +0800	[thread overview]
Message-ID: <20240424033916.2748488-10-libaokun@huaweicloud.com> (raw)
In-Reply-To: <20240424033916.2748488-1-libaokun@huaweicloud.com>

From: Baokun Li <libaokun1@huawei.com>

After installing the anonymous fd, we can now see it in userland and close
it. However, at this point we may not have gotten the reference count of
the cache, but we will put it during colse fd, so this may cause a cache
UAF.

To avoid this, we will make the anonymous fd accessible to the userland by
executing fd_install() after copy_to_user() has succeeded, and by this
point we must have already grabbed the reference count of the cache.

Suggested-by: Hou Tao <houtao1@huawei.com>
Signed-off-by: Baokun Li <libaokun1@huawei.com>
---
 fs/cachefiles/ondemand.c | 53 +++++++++++++++++++++++++---------------
 1 file changed, 33 insertions(+), 20 deletions(-)

diff --git a/fs/cachefiles/ondemand.c b/fs/cachefiles/ondemand.c
index 0cf63bfedc9e..7c2d43104120 100644
--- a/fs/cachefiles/ondemand.c
+++ b/fs/cachefiles/ondemand.c
@@ -4,6 +4,11 @@
 #include <linux/uio.h>
 #include "internal.h"
 
+struct anon_file {
+	struct file *file;
+	int fd;
+};
+
 static inline void cachefiles_req_put(struct cachefiles_req *req)
 {
 	if (refcount_dec_and_test(&req->ref))
@@ -244,14 +249,14 @@ int cachefiles_ondemand_restore(struct cachefiles_cache *cache, char *args)
 	return 0;
 }
 
-static int cachefiles_ondemand_get_fd(struct cachefiles_req *req)
+static int cachefiles_ondemand_get_fd(struct cachefiles_req *req,
+				      struct anon_file *anon_file)
 {
 	struct cachefiles_object *object;
 	struct cachefiles_cache *cache;
 	struct cachefiles_open *load;
-	struct file *file;
 	u32 object_id;
-	int ret, fd;
+	int ret;
 
 	object = cachefiles_grab_object(req->object,
 			cachefiles_obj_get_ondemand_fd);
@@ -263,16 +268,16 @@ static int cachefiles_ondemand_get_fd(struct cachefiles_req *req)
 	if (ret < 0)
 		goto err;
 
-	fd = get_unused_fd_flags(O_WRONLY);
-	if (fd < 0) {
-		ret = fd;
+	anon_file->fd = get_unused_fd_flags(O_WRONLY);
+	if (anon_file->fd < 0) {
+		ret = anon_file->fd;
 		goto err_free_id;
 	}
 
-	file = anon_inode_getfile("[cachefiles]", &cachefiles_ondemand_fd_fops,
-				  object, O_WRONLY);
-	if (IS_ERR(file)) {
-		ret = PTR_ERR(file);
+	anon_file->file = anon_inode_getfile("[cachefiles]",
+				&cachefiles_ondemand_fd_fops, object, O_WRONLY);
+	if (IS_ERR(anon_file->file)) {
+		ret = PTR_ERR(anon_file->file);
 		goto err_put_fd;
 	}
 
@@ -281,15 +286,14 @@ static int cachefiles_ondemand_get_fd(struct cachefiles_req *req)
 		spin_unlock(&object->ondemand->lock);
 		ret = -EEXIST;
 		/* Avoid performing cachefiles_ondemand_fd_release(). */
-		file->private_data = NULL;
+		anon_file->file->private_data = NULL;
 		goto err_put_file;
 	}
 
-	file->f_mode |= FMODE_PWRITE | FMODE_LSEEK;
-	fd_install(fd, file);
+	anon_file->file->f_mode |= FMODE_PWRITE | FMODE_LSEEK;
 
 	load = (void *)req->msg.data;
-	load->fd = fd;
+	load->fd = anon_file->fd;
 	object->ondemand->ondemand_id = object_id;
 	spin_unlock(&object->ondemand->lock);
 
@@ -298,9 +302,11 @@ static int cachefiles_ondemand_get_fd(struct cachefiles_req *req)
 	return 0;
 
 err_put_file:
-	fput(file);
+	fput(anon_file->file);
+	anon_file->file = NULL;
 err_put_fd:
-	put_unused_fd(fd);
+	put_unused_fd(anon_file->fd);
+	anon_file->fd = ret;
 err_free_id:
 	xa_erase(&cache->ondemand_ids, object_id);
 err:
@@ -357,6 +363,7 @@ ssize_t cachefiles_ondemand_daemon_read(struct cachefiles_cache *cache,
 	struct cachefiles_msg *msg;
 	size_t n;
 	int ret = 0;
+	struct anon_file anon_file;
 	XA_STATE(xas, &cache->reqs, cache->req_id_next);
 
 	xa_lock(&cache->reqs);
@@ -390,7 +397,7 @@ ssize_t cachefiles_ondemand_daemon_read(struct cachefiles_cache *cache,
 	xa_unlock(&cache->reqs);
 
 	if (msg->opcode == CACHEFILES_OP_OPEN) {
-		ret = cachefiles_ondemand_get_fd(req);
+		ret = cachefiles_ondemand_get_fd(req, &anon_file);
 		if (ret)
 			goto out;
 	}
@@ -398,10 +405,16 @@ ssize_t cachefiles_ondemand_daemon_read(struct cachefiles_cache *cache,
 	msg->msg_id = xas.xa_index;
 	msg->object_id = req->object->ondemand->ondemand_id;
 
-	if (copy_to_user(_buffer, msg, n) != 0) {
+	if (copy_to_user(_buffer, msg, n) != 0)
 		ret = -EFAULT;
-		if (msg->opcode == CACHEFILES_OP_OPEN)
-			close_fd(((struct cachefiles_open *)msg->data)->fd);
+
+	if (msg->opcode == CACHEFILES_OP_OPEN) {
+		if (ret < 0) {
+			fput(anon_file.file);
+			put_unused_fd(anon_file.fd);
+			goto out;
+		}
+		fd_install(anon_file.fd, anon_file.file);
 	}
 out:
 	cachefiles_put_object(req->object, cachefiles_obj_put_read_req);
-- 
2.39.2


  parent reply	other threads:[~2024-04-24  3:48 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   ` [External] " Jia Zhu
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 ` libaokun [this message]
2024-05-06  3:24   ` [PATCH 09/12] cachefiles: defer exposing anon_fd until after copy_to_user() succeeds 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=20240424033916.2748488-10-libaokun@huaweicloud.com \
    --to=libaokun@huaweicloud.com \
    --cc=dhowells@redhat.com \
    --cc=houtao1@huawei.com \
    --cc=jefflexu@linux.alibaba.com \
    --cc=jlayton@kernel.org \
    --cc=libaokun1@huawei.com \
    --cc=linux-erofs@lists.ozlabs.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netfs@lists.linux.dev \
    --cc=zhujia.zj@bytedance.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).