From: Amir Goldstein <amir73il@gmail.com>
To: Miklos Szeredi <miklos@szeredi.hu>
Cc: yangerkun <yangerkun@huawei.com>, linux-unionfs@vger.kernel.org
Subject: Re: [PATCH] ovl: fix incorrect fdput() on aio completion
Date: Mon, 4 Sep 2023 18:08:59 +0300 [thread overview]
Message-ID: <CAOQ4uxiuZHhxBMupBPTmkf8rg3QQM7M=7EPg9XmNeGZOR9Eb=Q@mail.gmail.com> (raw)
In-Reply-To: <CAJfpegvP+_ERUU_LvB2b=N13C=vqczYmtTrbM=opjXKYmva4Vw@mail.gmail.com>
On Mon, Sep 4, 2023 at 6:03 PM Miklos Szeredi <miklos@szeredi.hu> wrote:
>
> On Mon, 4 Sept 2023 at 16:47, Amir Goldstein <amir73il@gmail.com> wrote:
> >
> > ovl_{read,write}_iter() always call fdput(real) to put one or zero
> > refcounts of the real file, but for aio, whether it was submitted or not,
> > ovl_aio_put() also calls fdput(), which is not balanced. This is only a
> > problem in the less common case when FDPUT_FPUT flag is set.
> >
> > To fix the problem use get_file() to take file refcount and use fput()
> > instead of fdput() in ovl_aio_put().
> >
> > Fixes: 2406a307ac7d ("ovl: implement async IO routines")
> > Cc: <stable@vger.kernel.org> # v5.6
> > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> > ---
> >
> > Miklos,
> >
> > This is the refcount leak fix that I found during work on backing_fs [1]
> > that deserves to be fast tracked into stable.
> >
> > If it's ok with you, I will prepare a PR after rc1 including this
> > fix and the symlink fileattr fix.
>
> Looks good.
>
> Thanks,
> Miklos
>
>
> >
> > Thanks,
> > Amir.
> >
> > [1] https://lore.kernel.org/r/CAOQ4uxgzYevVCaGBjjckOr1vv0gKvVPYiOAL6E_KQY-YQx_7hg@mail.gmail.com/
> >
> > fs/overlayfs/file.c | 7 +++----
> > 1 file changed, 3 insertions(+), 4 deletions(-)
> >
> > diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c
> > index 3b4cc633d763..c743820e5c61 100644
> > --- a/fs/overlayfs/file.c
> > +++ b/fs/overlayfs/file.c
> > @@ -19,7 +19,6 @@ struct ovl_aio_req {
> > struct kiocb iocb;
> > refcount_t ref;
> > struct kiocb *orig_iocb;
> > - struct fd fd;
> > };
> >
> > static struct kmem_cache *ovl_aio_request_cachep;
> > @@ -280,7 +279,7 @@ static rwf_t ovl_iocb_to_rwf(int ifl)
> > static inline void ovl_aio_put(struct ovl_aio_req *aio_req)
> > {
> > if (refcount_dec_and_test(&aio_req->ref)) {
> > - fdput(aio_req->fd);
> > + fput(aio_req->iocb.ki_filp);
> > kmem_cache_free(ovl_aio_request_cachep, aio_req);
> > }
> > }
> > @@ -342,7 +341,7 @@ static ssize_t ovl_read_iter(struct kiocb *iocb, struct iov_iter *iter)
> > if (!aio_req)
> > goto out;
> >
> > - aio_req->fd = real;
> > + get_file(real.file);
> > real.flags = 0;
> > aio_req->orig_iocb = iocb;
> > kiocb_clone(&aio_req->iocb, iocb, real.file);
>
> It might be clearer to do the get_file() here:
>
> + kiocb_clone(&aio_req->iocb, iocb, get_file(real.file));
>
Right. I will fix and stage.
> Looks good otherwise.
>
I will take it as Reviewed-by ;-)
and will do the same for symlink fileattr fix.
Thanks,
Amir.
prev parent reply other threads:[~2023-09-04 15:09 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-09-04 14:47 [PATCH] ovl: fix incorrect fdput() on aio completion Amir Goldstein
2023-09-04 15:02 ` Miklos Szeredi
2023-09-04 15:08 ` Amir Goldstein [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='CAOQ4uxiuZHhxBMupBPTmkf8rg3QQM7M=7EPg9XmNeGZOR9Eb=Q@mail.gmail.com' \
--to=amir73il@gmail.com \
--cc=linux-unionfs@vger.kernel.org \
--cc=miklos@szeredi.hu \
--cc=yangerkun@huawei.com \
/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).