Re: strange interaction between fuse + pidns

Miklos Szeredi
2022-07-11T13:59:51Z

[-- Attachment #1: Type: text/plain, Size: 210 bytes --]

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

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

Still mostly untested.

Thanks,
Miklos

[-- Attachment #2: fuse-allow-flush-to-be-killed-v2.patch --]
[-- Type: text/x-patch, Size: 2789 bytes --]

---
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,
};

/**