Linux network filesystem support library
 help / color / mirror / Atom feed
From: Al Viro <viro@zeniv.linux.org.uk>
To: David Howells <dhowells@redhat.com>
Cc: Jeffle Xu <jefflexu@linux.alibaba.com>,
	Gao Xiang <hsiangkao@linux.alibaba.com>,
	netfs@lists.linux.dev, linux-fsdevel@vger.kernel.org
Subject: [PATCH] get rid of close_fd() misuse in cachefiles
Date: Mon, 3 Jun 2024 01:11:28 +0100	[thread overview]
Message-ID: <20240603001128.GG1629371@ZenIV> (raw)

	fd_install() can't be undone by close_fd().  Just delay it
until the last failure exit - have cachefiles_ondemand_get_fd()
return the file on success (and ERR_PTR() on error) and let the
caller do fd_install() after successful copy_to_user()

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
diff --git a/fs/cachefiles/ondemand.c b/fs/cachefiles/ondemand.c
index 4ba42f1fa3b4..b5da26ef2d45 100644
--- a/fs/cachefiles/ondemand.c
+++ b/fs/cachefiles/ondemand.c
@@ -205,7 +205,7 @@ int cachefiles_ondemand_restore(struct cachefiles_cache *cache, char *args)
 	return 0;
 }
 
-static int cachefiles_ondemand_get_fd(struct cachefiles_req *req)
+static struct file *cachefiles_ondemand_get_fd(struct cachefiles_req *req)
 {
 	struct cachefiles_object *object;
 	struct cachefiles_cache *cache;
@@ -238,7 +238,6 @@ static int cachefiles_ondemand_get_fd(struct cachefiles_req *req)
 	}
 
 	file->f_mode |= FMODE_PWRITE | FMODE_LSEEK;
-	fd_install(fd, file);
 
 	load = (void *)req->msg.data;
 	load->fd = fd;
@@ -246,7 +245,7 @@ static int cachefiles_ondemand_get_fd(struct cachefiles_req *req)
 
 	cachefiles_get_unbind_pincount(cache);
 	trace_cachefiles_ondemand_open(object, &req->msg, load);
-	return 0;
+	return file;
 
 err_put_fd:
 	put_unused_fd(fd);
@@ -254,7 +253,7 @@ static int cachefiles_ondemand_get_fd(struct cachefiles_req *req)
 	xa_erase(&cache->ondemand_ids, object_id);
 err:
 	cachefiles_put_object(object, cachefiles_obj_put_ondemand_fd);
-	return ret;
+	return ERR_PTR(ret);
 }
 
 static void ondemand_object_worker(struct work_struct *work)
@@ -299,9 +298,9 @@ ssize_t cachefiles_ondemand_daemon_read(struct cachefiles_cache *cache,
 {
 	struct cachefiles_req *req;
 	struct cachefiles_msg *msg;
+	struct file *file = NULL;
 	unsigned long id = 0;
 	size_t n;
-	int ret = 0;
 	XA_STATE(xas, &cache->reqs, cache->req_id_next);
 
 	xa_lock(&cache->reqs);
@@ -335,8 +334,8 @@ ssize_t cachefiles_ondemand_daemon_read(struct cachefiles_cache *cache,
 	id = xas.xa_index;
 
 	if (msg->opcode == CACHEFILES_OP_OPEN) {
-		ret = cachefiles_ondemand_get_fd(req);
-		if (ret) {
+		file = cachefiles_ondemand_get_fd(req);
+		if (IS_ERR(file)) {
 			cachefiles_ondemand_set_object_close(req->object);
 			goto error;
 		}
@@ -346,10 +345,15 @@ ssize_t cachefiles_ondemand_daemon_read(struct cachefiles_cache *cache,
 	msg->object_id = req->object->ondemand->ondemand_id;
 
 	if (copy_to_user(_buffer, msg, n) != 0) {
-		ret = -EFAULT;
-		goto err_put_fd;
+		if (file)
+			fput(file);
+		file = ERR_PTR(-EFAULT);
+		goto error;
 	}
 
+	if (file)
+		fd_install(((struct cachefiles_open *)msg->data)->fd, file);
+
 	/* CLOSE request has no reply */
 	if (msg->opcode == CACHEFILES_OP_CLOSE) {
 		xa_erase(&cache->reqs, id);
@@ -358,14 +362,11 @@ ssize_t cachefiles_ondemand_daemon_read(struct cachefiles_cache *cache,
 
 	return n;
 
-err_put_fd:
-	if (msg->opcode == CACHEFILES_OP_OPEN)
-		close_fd(((struct cachefiles_open *)msg->data)->fd);
 error:
 	xa_erase(&cache->reqs, id);
-	req->error = ret;
+	req->error = PTR_ERR(file);
 	complete(&req->done);
-	return ret;
+	return PTR_ERR(file);
 }
 
 typedef int (*init_req_fn)(struct cachefiles_req *req, void *private);

             reply	other threads:[~2024-06-03  0:11 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-06-03  0:11 Al Viro [this message]
2024-06-03  1:53 ` [PATCH] get rid of close_fd() misuse in cachefiles Gao Xiang
2024-06-03  2:21   ` Al Viro
2024-06-03  2:33     ` Gao Xiang
2024-06-03  3:40       ` Al Viro
2024-06-03  6:23         ` [PATCH] cachefiles: remove unneeded include of <linux/fdtable.h> Gao Xiang
2024-06-03 13:39           ` (subset) " Christian Brauner

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=20240603001128.GG1629371@ZenIV \
    --to=viro@zeniv.linux.org.uk \
    --cc=dhowells@redhat.com \
    --cc=hsiangkao@linux.alibaba.com \
    --cc=jefflexu@linux.alibaba.com \
    --cc=linux-fsdevel@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