From: syzbot <syzbot+df038d463cca332e8414@syzkaller.appspotmail.com>
To: dhowells@redhat.com
Cc: asmadeus@codewreck.org, brauner@kernel.org, dhowells@redhat.com,
ericvh@kernel.org, jlayton@kernel.org,
linux-afs@lists.infradead.org, linux-cifs@vger.kernel.org,
linux-kernel@vger.kernel.org, linux_oss@crudebyte.com,
lucho@ionkov.net, netfs@lists.linux.dev, sfrench@samba.org,
syzkaller-bugs@googlegroups.com, v9fs@lists.linux.dev
Subject: Re: [syzbot] [v9fs?] KASAN: slab-use-after-free Write in v9fs_free_request
Date: Wed, 22 May 2024 14:09:14 -0700 [thread overview]
Message-ID: <000000000000bf1a9806191158b5@google.com> (raw)
In-Reply-To: <463668.1716412143@warthog.procyon.org.uk>
> #syz test: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
This crash does not have a reproducer. I cannot test it.
>
> netfs, 9p: Fix race between umount and async request completion
>
> There's a problem in 9p's interaction with netfslib whereby a crash occurs
> because the 9p_fid structs get forcibly destroyed during client teardown
> (without paying attention to their refcounts) before netfslib has finished
> with them. However, it's not a simple case of deferring the clunking that
> p9_fid_put() does as that requires the client.
>
> The problem is that netfslib has to unlock pages and clear the IN_PROGRESS
> flag before destroying the objects involved - including the pid - and, in
> any case, nothing checks to see if writeback completed barring looking at
> the page flags.
>
> Fix this by keeping a count of outstanding I/O requests (of any type) and
> waiting for it to quiesce during inode eviction.
>
> Signed-off-by: David Howells <dhowells@redhat.com>
> cc: Eric Van Hensbergen <ericvh@kernel.org>
> cc: Latchesar Ionkov <lucho@ionkov.net>
> cc: Dominique Martinet <asmadeus@codewreck.org>
> cc: Christian Schoenebeck <linux_oss@crudebyte.com>
> cc: Jeff Layton <jlayton@kernel.org>
> cc: Steve French <sfrench@samba.org>
> cc: v9fs@lists.linux.dev
> cc: linux-afs@lists.infradead.org
> cc: linux-cifs@vger.kernel.org
> cc: netfs@lists.linux.dev
> cc: linux-fsdevel@vger.kernel.org
> ---
> fs/9p/vfs_inode.c | 5 ++++-
> fs/afs/inode.c | 1 +
> fs/netfs/objects.c | 5 +++++
> fs/smb/client/cifsfs.c | 1 +
> include/linux/netfs.h | 18 ++++++++++++++++++
> 5 files changed, 29 insertions(+), 1 deletion(-)
>
> diff --git a/fs/9p/vfs_inode.c b/fs/9p/vfs_inode.c
> index 8c9a896d691e..57cfa9f65046 100644
> --- a/fs/9p/vfs_inode.c
> +++ b/fs/9p/vfs_inode.c
> @@ -354,6 +354,7 @@ void v9fs_evict_inode(struct inode *inode)
> version = cpu_to_le32(v9inode->qid.version);
> netfs_clear_inode_writeback(inode, &version);
>
> + netfs_wait_for_outstanding_io(inode);
> clear_inode(inode);
> filemap_fdatawrite(&inode->i_data);
>
> @@ -361,8 +362,10 @@ void v9fs_evict_inode(struct inode *inode)
> if (v9fs_inode_cookie(v9inode))
> fscache_relinquish_cookie(v9fs_inode_cookie(v9inode), false);
> #endif
> - } else
> + } else {
> + netfs_wait_for_outstanding_io(inode);
> clear_inode(inode);
> + }
> }
>
> struct inode *
> diff --git a/fs/afs/inode.c b/fs/afs/inode.c
> index 94fc049aff58..c831e711a4ac 100644
> --- a/fs/afs/inode.c
> +++ b/fs/afs/inode.c
> @@ -652,6 +652,7 @@ void afs_evict_inode(struct inode *inode)
>
> afs_set_cache_aux(vnode, &aux);
> netfs_clear_inode_writeback(inode, &aux);
> + netfs_wait_for_outstanding_io(inode);
> clear_inode(inode);
>
> while (!list_empty(&vnode->wb_keys)) {
> diff --git a/fs/netfs/objects.c b/fs/netfs/objects.c
> index c90d482b1650..f4a642727479 100644
> --- a/fs/netfs/objects.c
> +++ b/fs/netfs/objects.c
> @@ -72,6 +72,7 @@ struct netfs_io_request *netfs_alloc_request(struct address_space *mapping,
> }
> }
>
> + atomic_inc(&ctx->io_count);
> trace_netfs_rreq_ref(rreq->debug_id, 1, netfs_rreq_trace_new);
> netfs_proc_add_rreq(rreq);
> netfs_stat(&netfs_n_rh_rreq);
> @@ -124,6 +125,7 @@ static void netfs_free_request(struct work_struct *work)
> {
> struct netfs_io_request *rreq =
> container_of(work, struct netfs_io_request, work);
> + struct netfs_inode *ictx = netfs_inode(rreq->inode);
> unsigned int i;
>
> trace_netfs_rreq(rreq, netfs_rreq_trace_free);
> @@ -142,6 +144,9 @@ static void netfs_free_request(struct work_struct *work)
> }
> kvfree(rreq->direct_bv);
> }
> +
> + if (atomic_dec_and_test(&ictx->io_count))
> + wake_up_var(&ictx->io_count);
> call_rcu(&rreq->rcu, netfs_free_request_rcu);
> }
>
> diff --git a/fs/smb/client/cifsfs.c b/fs/smb/client/cifsfs.c
> index ec5b639f421a..21c9e173ea9a 100644
> --- a/fs/smb/client/cifsfs.c
> +++ b/fs/smb/client/cifsfs.c
> @@ -435,6 +435,7 @@ cifs_evict_inode(struct inode *inode)
> if (inode->i_state & I_PINNING_NETFS_WB)
> cifs_fscache_unuse_inode_cookie(inode, true);
> cifs_fscache_release_inode_cookie(inode);
> + netfs_wait_for_outstanding_io(inode);
> clear_inode(inode);
> }
>
> diff --git a/include/linux/netfs.h b/include/linux/netfs.h
> index d2d291a9cdad..3ca3906bb8da 100644
> --- a/include/linux/netfs.h
> +++ b/include/linux/netfs.h
> @@ -68,6 +68,7 @@ struct netfs_inode {
> loff_t remote_i_size; /* Size of the remote file */
> loff_t zero_point; /* Size after which we assume there's no data
> * on the server */
> + atomic_t io_count; /* Number of outstanding reqs */
> unsigned long flags;
> #define NETFS_ICTX_ODIRECT 0 /* The file has DIO in progress */
> #define NETFS_ICTX_UNBUFFERED 1 /* I/O should not use the pagecache */
> @@ -474,6 +475,7 @@ static inline void netfs_inode_init(struct netfs_inode *ctx,
> ctx->remote_i_size = i_size_read(&ctx->inode);
> ctx->zero_point = LLONG_MAX;
> ctx->flags = 0;
> + atomic_set(&ctx->io_count, 0);
> #if IS_ENABLED(CONFIG_FSCACHE)
> ctx->cache = NULL;
> #endif
> @@ -517,4 +519,20 @@ static inline struct fscache_cookie *netfs_i_cookie(struct netfs_inode *ctx)
> #endif
> }
>
> +/**
> + * netfs_wait_for_outstanding_io - Wait for outstanding I/O to complete
> + * @ctx: The netfs inode to wait on
> + *
> + * Wait for outstanding I/O requests of any type to complete. This is intended
> + * to be called from inode eviction routines. This makes sure that any
> + * resources held by those requests are cleaned up before we let the inode get
> + * cleaned up.
> + */
> +static inline void netfs_wait_for_outstanding_io(struct inode *inode)
> +{
> + struct netfs_inode *ictx = netfs_inode(inode);
> +
> + wait_var_event(&ictx->io_count, atomic_read(&ictx->io_count) == 0);
> +}
> +
> #endif /* _LINUX_NETFS_H */
>
next prev parent reply other threads:[~2024-05-22 21:09 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-12 19:42 [syzbot] [v9fs?] KASAN: slab-use-after-free Write in v9fs_free_request syzbot
2024-05-20 7:32 ` asmadeus
2024-05-21 14:24 ` David Howells
2024-05-22 10:56 ` David Howells
2024-05-22 21:09 ` David Howells
2024-05-22 21:09 ` syzbot [this message]
-- strict thread matches above, loose matches on Subject: below --
2024-05-13 11:50 lee bruce
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=000000000000bf1a9806191158b5@google.com \
--to=syzbot+df038d463cca332e8414@syzkaller.appspotmail.com \
--cc=asmadeus@codewreck.org \
--cc=brauner@kernel.org \
--cc=dhowells@redhat.com \
--cc=ericvh@kernel.org \
--cc=jlayton@kernel.org \
--cc=linux-afs@lists.infradead.org \
--cc=linux-cifs@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux_oss@crudebyte.com \
--cc=lucho@ionkov.net \
--cc=netfs@lists.linux.dev \
--cc=sfrench@samba.org \
--cc=syzkaller-bugs@googlegroups.com \
--cc=v9fs@lists.linux.dev \
/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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).