public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Al Viro <viro@zeniv.linux.org.uk>
To: Aditya Pakki <pakki001@umn.edu>
Cc: Vivek Goyal <vgoyal@redhat.com>,
	Stefan Hajnoczi <stefanha@redhat.com>,
	Miklos Szeredi <miklos@szeredi.hu>,
	virtualization@lists.linux-foundation.org,
	linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] fuse: Avoid potential use after free
Date: Thu, 22 Apr 2021 00:44:08 +0000	[thread overview]
Message-ID: <YIDG2MOSITJxJBqd@zeniv-ca.linux.org.uk> (raw)
In-Reply-To: <20210406235332.2206460-1-pakki001@umn.edu>

On Tue, Apr 06, 2021 at 06:53:32PM -0500, Aditya Pakki wrote:
> In virtio_fs_get_tree, after fm is freed, it is again freed in case
> s_root is NULL and virtio_fs_fill_super() returns an error. To avoid
> a double free, set fm to NULL.
> 
> Signed-off-by: Aditya Pakki <pakki001@umn.edu>
> ---
>  fs/fuse/virtio_fs.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/fs/fuse/virtio_fs.c b/fs/fuse/virtio_fs.c
> index 4ee6f734ba83..a7484c1539bf 100644
> --- a/fs/fuse/virtio_fs.c
> +++ b/fs/fuse/virtio_fs.c
> @@ -1447,6 +1447,7 @@ static int virtio_fs_get_tree(struct fs_context *fsc)
>  	if (fsc->s_fs_info) {
>  		fuse_conn_put(fc);
>  		kfree(fm);
> +		fm = NULL;
>  	}
>  	if (IS_ERR(sb))
>  		return PTR_ERR(sb);

NAK.  The only cases when sget_fc() returns without having ->s_fs_info
zeroed are when it has successfull grabbed a reference to existing live
superblock or when it has failed.  In the former case we proceed straight
to
        fsc->root = dget(sb->s_root);
	return 0;
and in the latter we bugger off on IS_ERR(sb).  No double-free in either
case.  Said that, the logics in there (especially around the cleanups
on virtio_fs_fill_super() failures) is bloody convoluted, but sorting
that out would take a lot more RTFS than I'm willing to start right now.

In any case, this patch does not fix any bugs and does not make the
thing easier to follow, so...

NAKed-by: Al Viro <viro@zeniv.linux.org.uk>

      parent reply	other threads:[~2021-04-22  0:44 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-06 23:53 [PATCH] fuse: Avoid potential use after free Aditya Pakki
2021-04-07 15:50 ` Vivek Goyal
2021-04-21 13:28   ` Krzysztof Kozlowski
2021-04-22  0:44 ` Al Viro [this message]

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=YIDG2MOSITJxJBqd@zeniv-ca.linux.org.uk \
    --to=viro@zeniv.linux.org.uk \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=miklos@szeredi.hu \
    --cc=pakki001@umn.edu \
    --cc=stefanha@redhat.com \
    --cc=vgoyal@redhat.com \
    --cc=virtualization@lists.linux-foundation.org \
    /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