netdev - Re: [PATCH 04/16] backing-file: use credential guards for writes

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAOQ4uxhW2FiVe6XjQDT_aXhzJDyT5yuna9CVaWOLyzU1J99hFg@mail.gmail.com>
Date: Mon, 3 Nov 2025 14:24:13 +0100
From: Amir Goldstein <amir73il@...il.com>
To: Christian Brauner <brauner@...nel.org>
Cc: Linus Torvalds <torvalds@...ux-foundation.org>, linux-fsdevel@...r.kernel.org, 
	linux-kernel@...r.kernel.org, linux-aio@...ck.org, 
	linux-unionfs@...r.kernel.org, linux-erofs@...ts.ozlabs.org, 
	linux-nfs@...r.kernel.org, linux-cifs@...r.kernel.org, 
	samba-technical@...ts.samba.org, cgroups@...r.kernel.org, 
	netdev@...r.kernel.org
Subject: Re: [PATCH 04/16] backing-file: use credential guards for writes

On Mon, Nov 3, 2025 at 12:30 PM Christian Brauner <brauner@...nel.org> wrote:
>
> Use credential guards for scoped credential override with automatic
> restoration on scope exit.
>
> Signed-off-by: Christian Brauner <brauner@...nel.org>
> ---
>  fs/backing-file.c | 74 +++++++++++++++++++++++++++++--------------------------
>  1 file changed, 39 insertions(+), 35 deletions(-)
>
> diff --git a/fs/backing-file.c b/fs/backing-file.c
> index 4cb7276e7ead..9bea737d5bef 100644
> --- a/fs/backing-file.c
> +++ b/fs/backing-file.c
> @@ -210,11 +210,47 @@ ssize_t backing_file_read_iter(struct file *file, struct iov_iter *iter,
>  }
>  EXPORT_SYMBOL_GPL(backing_file_read_iter);
>
> +static int do_backing_file_write_iter(struct file *file, struct iov_iter *iter,
> +                                     struct kiocb *iocb, int flags,
> +                                     void (*end_write)(struct kiocb *, ssize_t))
> +{
> +       struct backing_aio *aio;
> +       int ret;
> +
> +       if (is_sync_kiocb(iocb)) {
> +               rwf_t rwf = iocb_to_rw_flags(flags);
> +
> +               ret = vfs_iter_write(file, iter, &iocb->ki_pos, rwf);
> +               if (end_write)
> +                       end_write(iocb, ret);
> +               return ret;
> +       }
> +
> +       ret = backing_aio_init_wq(iocb);
> +       if (ret)
> +               return ret;
> +
> +       aio = kmem_cache_zalloc(backing_aio_cachep, GFP_KERNEL);
> +       if (!aio)
> +               return -ENOMEM;
> +
> +       aio->orig_iocb = iocb;
> +       aio->end_write = end_write;
> +       kiocb_clone(&aio->iocb, iocb, get_file(file));
> +       aio->iocb.ki_flags = flags;
> +       aio->iocb.ki_complete = backing_aio_queue_completion;
> +       refcount_set(&aio->ref, 2);
> +       ret = vfs_iocb_iter_write(file, &aio->iocb, iter);
> +       backing_aio_put(aio);
> +       if (ret != -EIOCBQUEUED)
> +               backing_aio_cleanup(aio, ret);
> +       return ret;
> +}
> +
>  ssize_t backing_file_write_iter(struct file *file, struct iov_iter *iter,
>                                 struct kiocb *iocb, int flags,
>                                 struct backing_file_ctx *ctx)
>  {
> -       const struct cred *old_cred;
>         ssize_t ret;
>
>         if (WARN_ON_ONCE(!(file->f_mode & FMODE_BACKING)))
> @@ -237,40 +273,8 @@ ssize_t backing_file_write_iter(struct file *file, struct iov_iter *iter,
>          */
>         flags &= ~IOCB_DIO_CALLER_COMP;
>
> -       old_cred = override_creds(ctx->cred);
> -       if (is_sync_kiocb(iocb)) {
> -               rwf_t rwf = iocb_to_rw_flags(flags);
> -
> -               ret = vfs_iter_write(file, iter, &iocb->ki_pos, rwf);
> -               if (ctx->end_write)
> -                       ctx->end_write(iocb, ret);
> -       } else {
> -               struct backing_aio *aio;
> -
> -               ret = backing_aio_init_wq(iocb);
> -               if (ret)
> -                       goto out;
> -
> -               ret = -ENOMEM;
> -               aio = kmem_cache_zalloc(backing_aio_cachep, GFP_KERNEL);
> -               if (!aio)
> -                       goto out;
> -
> -               aio->orig_iocb = iocb;
> -               aio->end_write = ctx->end_write;
> -               kiocb_clone(&aio->iocb, iocb, get_file(file));
> -               aio->iocb.ki_flags = flags;
> -               aio->iocb.ki_complete = backing_aio_queue_completion;
> -               refcount_set(&aio->ref, 2);
> -               ret = vfs_iocb_iter_write(file, &aio->iocb, iter);
> -               backing_aio_put(aio);
> -               if (ret != -EIOCBQUEUED)
> -                       backing_aio_cleanup(aio, ret);
> -       }
> -out:
> -       revert_creds(old_cred);
> -
> -       return ret;
> +       with_creds(ctx->cred);
> +       return do_backing_file_write_iter(file, iter, iocb, flags, ctx->end_write);
>  }

Pointing out the obvious that do_backing_file_write_iter() feels
unnecessary here.

But I am fine with keeping it for symmetry with
do_backing_file_read_iter() and in case we will want to call the sync
end_write() callback outside of creds override context as we do in the
read case.

Thanks,
Amir.