On Mon, 4 Sept 2023 at 16:47, Amir Goldstein <amir73il@xxxxxxxxx> 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@xxxxxxxxxxxxxxx> # v5.6
> Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx>
> ---
>
> 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@xxxxxxxxxxxxxx/
>
> 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));
Looks good otherwise.
Thanks,
Miklos
