Linux-Fsdevel Archive on lore.kernel.org
 help / color / Atom feed
From: Vivek Goyal <vgoyal@redhat.com>
To: Miklos Szeredi <miklos@szeredi.hu>
Cc: Michael Stapelberg <michael+lkml@stapelberg.ch>,
	fuse-devel <fuse-devel@lists.sourceforge.net>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Kyle Sanderson <kyle.leet@gmail.com>,
	linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: Still a pretty bad time on 5.4.6 with fuse_request_end.
Date: Tue, 10 Mar 2020 14:07:19 -0400
Message-ID: <20200310180719.GB38440@redhat.com> (raw)
In-Reply-To: <CAJfpegvBguKcNZk-p7sAtSuNH_7HfdCyYvo8Wh7X6P=hT=kPrA@mail.gmail.com>

On Wed, Feb 12, 2020 at 08:36:11PM +0100, Miklos Szeredi wrote:
> On Wed, Feb 12, 2020 at 10:38 AM Michael Stapelberg
> <michael+lkml@stapelberg.ch> wrote:
> >
> > Unfortunately not: when I change the code like so:
> >
> >     bool async;
> >     uint32_t opcode_early = req->args->opcode;
> >
> >     if (test_and_set_bit(FR_FINISHED, &req->flags))
> >         goto put_request;
> >
> >     async = req->args->end;
> >
> > …gdb only reports:
> >
> > (gdb) bt
> > #0  0x000000a700000001 in ?? ()
> > #1  0xffffffff8137fc99 in fuse_copy_finish (cs=0x20000ffffffff) at
> > fs/fuse/dev.c:681
> > Backtrace stopped: previous frame inner to this frame (corrupt stack?)
> >
> > But maybe that’s a hint in and of itself?
> 
> Yep, it's a stack use after return bug.   Attached patch should fix
> it, though I haven't tested it.

I think I have noticed couple of crashes in fuse_request_end() while
it was trying to call req->args->end() and I suspect this patch might
fix the issue.

Just that I have not been able to reproduce it reliably to be able test
it.

Vivek

> 
> Thanks,
> Miklos

> ---
>  fs/fuse/dev.c    |    6 +++---
>  fs/fuse/fuse_i.h |    2 ++
>  2 files changed, 5 insertions(+), 3 deletions(-)
> 
> --- a/fs/fuse/dev.c
> +++ b/fs/fuse/dev.c
> @@ -276,12 +276,10 @@ static void flush_bg_queue(struct fuse_c
>  void fuse_request_end(struct fuse_conn *fc, struct fuse_req *req)
>  {
>  	struct fuse_iqueue *fiq = &fc->iq;
> -	bool async;
>  
>  	if (test_and_set_bit(FR_FINISHED, &req->flags))
>  		goto put_request;
>  
> -	async = req->args->end;
>  	/*
>  	 * test_and_set_bit() implies smp_mb() between bit
>  	 * changing and below intr_entry check. Pairs with
> @@ -324,7 +322,7 @@ void fuse_request_end(struct fuse_conn *
>  		wake_up(&req->waitq);
>  	}
>  
> -	if (async)
> +	if (test_bit(FR_ASYNC, &req->flags))
>  		req->args->end(fc, req->args, req->out.h.error);
>  put_request:
>  	fuse_put_request(fc, req);
> @@ -471,6 +469,8 @@ static void fuse_args_to_req(struct fuse
>  	req->in.h.opcode = args->opcode;
>  	req->in.h.nodeid = args->nodeid;
>  	req->args = args;
> +	if (args->end)
> +		set_bit(FR_ASYNC, &req->flags);
>  }
>  
>  ssize_t fuse_simple_request(struct fuse_conn *fc, struct fuse_args *args)
> --- a/fs/fuse/fuse_i.h
> +++ b/fs/fuse/fuse_i.h
> @@ -301,6 +301,7 @@ struct fuse_io_priv {
>   * FR_SENT:		request is in userspace, waiting for an answer
>   * FR_FINISHED:		request is finished
>   * FR_PRIVATE:		request is on private list
> + * FR_ASYNC:		request is asynchronous
>   */
>  enum fuse_req_flag {
>  	FR_ISREPLY,
> @@ -314,6 +315,7 @@ enum fuse_req_flag {
>  	FR_SENT,
>  	FR_FINISHED,
>  	FR_PRIVATE,
> +	FR_ASYNC,
>  };
>  
>  /**


  parent reply index

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-05  1:15 Kyle Sanderson
2020-01-05  8:08 ` Greg KH
2020-01-16 11:06 ` Miklos Szeredi
2020-02-09  8:09   ` michael+lkml
2020-02-11 10:55     ` Miklos Szeredi
2020-02-12  7:58       ` Michael Stapelberg
2020-02-12  8:34         ` Miklos Szeredi
2020-02-12  9:38           ` Michael Stapelberg
2020-02-12 19:36             ` Miklos Szeredi
2020-02-13  7:23               ` Michael Stapelberg
2020-03-10 18:07               ` Vivek Goyal [this message]
2020-02-22 19:29 Kyle Sanderson
2020-02-23  9:58 ` Greg KH
2020-03-06  9:31 Bruno Thomsen
2020-03-09 19:03 ` Greg KH

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=20200310180719.GB38440@redhat.com \
    --to=vgoyal@redhat.com \
    --cc=fuse-devel@lists.sourceforge.net \
    --cc=gregkh@linuxfoundation.org \
    --cc=kyle.leet@gmail.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=michael+lkml@stapelberg.ch \
    --cc=miklos@szeredi.hu \
    /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

Linux-Fsdevel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-fsdevel/0 linux-fsdevel/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-fsdevel linux-fsdevel/ https://lore.kernel.org/linux-fsdevel \
		linux-fsdevel@vger.kernel.org
	public-inbox-index linux-fsdevel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-fsdevel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git