Re: strange interaction between fuse + pidns

[Miklos Szeredi <miklos@xxxxxxxxxx>]

On Mon, 11 Jul 2022 at 12:35, Miklos Szeredi <miklos@xxxxxxxxxx> wrote:
>
> Can you try the attached untested patch?

Updated patch to avoid use after free on req->args.

Still mostly untested.

Thanks,
Miklos

---
 fs/fuse/dev.c    |   23 +++++++++++++++++------
 fs/fuse/file.c   |    1 +
 fs/fuse/fuse_i.h |    3 +++
 3 files changed, 21 insertions(+), 6 deletions(-)

--- a/fs/fuse/dev.c
+++ b/fs/fuse/dev.c
@@ -397,6 +397,12 @@ static void request_wait_answer(struct f
 			req->out.h.error = -EINTR;
 			return;
 		}
+		if (req->args->killable) {
+			req->out.h.error = -EINTR;
+			/* fuse_request_end() will drop final ref */
+			spin_unlock(&fiq->lock);
+			return;
+		}
 		spin_unlock(&fiq->lock);
 	}
 
@@ -478,6 +484,8 @@ static void fuse_args_to_req(struct fuse
 	req->args = args;
 	if (args->end)
 		__set_bit(FR_ASYNC, &req->flags);
+	if (!args->out_numargs)
+		__set_bit(FR_NOOUTARG, &req->flags);
 }
 
 ssize_t fuse_simple_request(struct fuse_mount *fm, struct fuse_args *args)
@@ -486,6 +494,8 @@ ssize_t fuse_simple_request(struct fuse_
 	struct fuse_req *req;
 	ssize_t ret;
 
+	WARN_ON(args->killable && args->out_numargs);
+
 	if (args->force) {
 		atomic_inc(&fc->num_waiting);
 		req = fuse_request_alloc(fm, GFP_KERNEL | __GFP_NOFAIL);
@@ -494,7 +504,8 @@ ssize_t fuse_simple_request(struct fuse_
 			fuse_force_creds(req);
 
 		__set_bit(FR_WAITING, &req->flags);
-		__set_bit(FR_FORCE, &req->flags);
+		if (!args->killable)
+			__set_bit(FR_FORCE, &req->flags);
 	} else {
 		WARN_ON(args->nocreds);
 		req = fuse_get_req(fm, false);
@@ -1913,13 +1924,13 @@ static ssize_t fuse_dev_do_write(struct
 	set_bit(FR_LOCKED, &req->flags);
 	spin_unlock(&fpq->lock);
 	cs->req = req;
-	if (!req->args->page_replace)
-		cs->move_pages = 0;
-
-	if (oh.error)
+	if (oh.error || test_bit(FR_NOOUTARG, &req->flags)) {
 		err = nbytes != sizeof(oh) ? -EINVAL : 0;
-	else
+	} else {
+		if (!req->args->page_replace)
+			cs->move_pages = 0;
 		err = copy_out_args(cs, req->args, nbytes);
+	}
 	fuse_copy_finish(cs);
 
 	spin_lock(&fpq->lock);
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -504,6 +504,7 @@ static int fuse_flush(struct file *file,
 	args.in_args[0].size = sizeof(inarg);
 	args.in_args[0].value = &inarg;
 	args.force = true;
+	args.killable = true;
 
 	err = fuse_simple_request(fm, &args);
 	if (err == -ENOSYS) {
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -261,6 +261,7 @@ struct fuse_args {
 	bool page_zeroing:1;
 	bool page_replace:1;
 	bool may_block:1;
+	bool killable:1;
 	struct fuse_in_arg in_args[3];
 	struct fuse_arg out_args[2];
 	void (*end)(struct fuse_mount *fm, struct fuse_args *args, int error);
@@ -314,6 +315,7 @@ struct fuse_io_priv {
  * FR_FINISHED:		request is finished
  * FR_PRIVATE:		request is on private list
  * FR_ASYNC:		request is asynchronous
+ * FR_NOOUTARG:		reply is only header
  */
 enum fuse_req_flag {
 	FR_ISREPLY,
@@ -328,6 +330,7 @@ enum fuse_req_flag {
 	FR_FINISHED,
 	FR_PRIVATE,
 	FR_ASYNC,
+	FR_NOOUTARG,
 };
 
 /**