* [PATCH (resend)] fuse: Fix oops at process_init_reply().
@ 2018-07-18 10:07 Tetsuo Handa
2018-07-18 10:44 ` Al Viro
0 siblings, 1 reply; 5+ messages in thread
From: Tetsuo Handa @ 2018-07-18 10:07 UTC (permalink / raw)
To: Miklos Szeredi; +Cc: linux-fsdevel, Tetsuo Handa, syzbot
syzbot is hitting NULL pointer dereference at process_init_reply() [1].
This is because deactivate_locked_super() is called before response for
initial request is processed. Fix this by protecting process_init_reply()
using fc->killsb.
[1] https://syzkaller.appspot.com/bug?id=d363046088dc26030e146e92102f965bf4623a50
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Reported-by: syzbot <syzbot+b62f08f4d5857755e3bc@syzkaller.appspotmail.com>
---
fs/fuse/inode.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index a24df88..2c9495e 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -868,7 +868,8 @@ static void process_init_reply(struct fuse_conn *fc, struct fuse_req *req)
{
struct fuse_init_out *arg = &req->misc.init_out;
- if (req->out.h.error || arg->major != FUSE_KERNEL_VERSION)
+ down_read(&fc->killsb);
+ if (req->out.h.error || arg->major != FUSE_KERNEL_VERSION || !fc->sb)
fc->conn_error = 1;
else {
unsigned long ra_pages;
@@ -938,6 +939,7 @@ static void process_init_reply(struct fuse_conn *fc, struct fuse_req *req)
}
fuse_set_initialized(fc);
wake_up_all(&fc->blocked_waitq);
+ up_read(&fc->killsb);
}
static void fuse_send_init(struct fuse_conn *fc, struct fuse_req *req)
--
2.7.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH (resend)] fuse: Fix oops at process_init_reply().
2018-07-18 10:07 [PATCH (resend)] fuse: Fix oops at process_init_reply() Tetsuo Handa
@ 2018-07-18 10:44 ` Al Viro
2018-07-18 10:51 ` Tetsuo Handa
2018-07-18 11:35 ` Miklos Szeredi
0 siblings, 2 replies; 5+ messages in thread
From: Al Viro @ 2018-07-18 10:44 UTC (permalink / raw)
To: Tetsuo Handa; +Cc: Miklos Szeredi, linux-fsdevel, syzbot
On Wed, Jul 18, 2018 at 07:07:18PM +0900, Tetsuo Handa wrote:
> syzbot is hitting NULL pointer dereference at process_init_reply() [1].
> This is because deactivate_locked_super() is called before response for
> initial request is processed. Fix this by protecting process_init_reply()
> using fc->killsb.
IDGI... why is FUSE_INIT asynchronous in the first place? What's the point
returning a superblock before FUSE_INIT completes, seeing that things like
fuse_get_req() block until that one is over?
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH (resend)] fuse: Fix oops at process_init_reply().
2018-07-18 10:44 ` Al Viro
@ 2018-07-18 10:51 ` Tetsuo Handa
2018-07-20 8:41 ` Miklos Szeredi
2018-07-18 11:35 ` Miklos Szeredi
1 sibling, 1 reply; 5+ messages in thread
From: Tetsuo Handa @ 2018-07-18 10:51 UTC (permalink / raw)
To: Al Viro; +Cc: Miklos Szeredi, linux-fsdevel, syzbot
On 2018/07/18 19:44, Al Viro wrote:
> On Wed, Jul 18, 2018 at 07:07:18PM +0900, Tetsuo Handa wrote:
>> syzbot is hitting NULL pointer dereference at process_init_reply() [1].
>> This is because deactivate_locked_super() is called before response for
>> initial request is processed. Fix this by protecting process_init_reply()
>> using fc->killsb.
>
> IDGI... why is FUSE_INIT asynchronous in the first place? What's the point
> returning a superblock before FUSE_INIT completes, seeing that things like
> fuse_get_req() block until that one is over?
>
I don't know...
What we can say is that async initialization is prone to races like
https://syzkaller.appspot.com/bug?id=b61716c2020c98e885af88c7de5896425947313f .
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH (resend)] fuse: Fix oops at process_init_reply().
2018-07-18 10:51 ` Tetsuo Handa
@ 2018-07-20 8:41 ` Miklos Szeredi
0 siblings, 0 replies; 5+ messages in thread
From: Miklos Szeredi @ 2018-07-20 8:41 UTC (permalink / raw)
To: Tetsuo Handa; +Cc: Al Viro, linux-fsdevel, syzbot
On Wed, Jul 18, 2018 at 12:51 PM, Tetsuo Handa
<penguin-kernel@i-love.sakura.ne.jp> wrote:
> On 2018/07/18 19:44, Al Viro wrote:
>> On Wed, Jul 18, 2018 at 07:07:18PM +0900, Tetsuo Handa wrote:
>>> syzbot is hitting NULL pointer dereference at process_init_reply() [1].
>>> This is because deactivate_locked_super() is called before response for
>>> initial request is processed. Fix this by protecting process_init_reply()
>>> using fc->killsb.
>>
>> IDGI... why is FUSE_INIT asynchronous in the first place? What's the point
>> returning a superblock before FUSE_INIT completes, seeing that things like
>> fuse_get_req() block until that one is over?
>>
> I don't know...
>
> What we can say is that async initialization is prone to races like
> https://syzkaller.appspot.com/bug?id=b61716c2020c98e885af88c7de5896425947313f .
Yep, turns out to be a can of worms when looking closely: it is
assumed that all requests are finished by the time fuse_abort_conn()
returns. Turns out to be not true...
Thanks,
Miklos
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH (resend)] fuse: Fix oops at process_init_reply().
2018-07-18 10:44 ` Al Viro
2018-07-18 10:51 ` Tetsuo Handa
@ 2018-07-18 11:35 ` Miklos Szeredi
1 sibling, 0 replies; 5+ messages in thread
From: Miklos Szeredi @ 2018-07-18 11:35 UTC (permalink / raw)
To: Al Viro; +Cc: Tetsuo Handa, linux-fsdevel, syzbot
On Wed, Jul 18, 2018 at 12:44 PM, Al Viro <viro@zeniv.linux.org.uk> wrote:
> On Wed, Jul 18, 2018 at 07:07:18PM +0900, Tetsuo Handa wrote:
>> syzbot is hitting NULL pointer dereference at process_init_reply() [1].
>> This is because deactivate_locked_super() is called before response for
>> initial request is processed. Fix this by protecting process_init_reply()
>> using fc->killsb.
>
> IDGI... why is FUSE_INIT asynchronous in the first place? What's the point
> returning a superblock before FUSE_INIT completes, seeing that things like
> fuse_get_req() block until that one is over?
Very very old story. Basically one of the design decisions was to
make usrespace fs initialization be completely serial like this:
fd = open("/dev/fuse", ...);
mount(..., "fuse", ...);
read(fd, request_buf, ...);
/* First request is always going to be FUSE_INIT */
write(fd, reply_buf, ...);
...
In hindsight it was a bad decision, but we are pretty much stuck with
it at this point, at least for backward compatibility with all current
fuse userspace code.
Thanks,
Miklos
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-07-20 9:29 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-18 10:07 [PATCH (resend)] fuse: Fix oops at process_init_reply() Tetsuo Handa
2018-07-18 10:44 ` Al Viro
2018-07-18 10:51 ` Tetsuo Handa
2018-07-20 8:41 ` Miklos Szeredi
2018-07-18 11:35 ` Miklos Szeredi
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 NNTP newsgroup(s).