public inbox for io-uring@vger.kernel.org
 help / color / mirror / Atom feed
From: Paul Moore <paul@paul-moore.com>
To: Al Viro <viro@zeniv.linux.org.uk>
Cc: linux-fsdevel@vger.kernel.org, torvalds@linux-foundation.org,
	 brauner@kernel.org, jack@suse.cz, mjguzik@gmail.com,
	axboe@kernel.dk,  audit@vger.kernel.org,
	io-uring@vger.kernel.org
Subject: Re: [RFC][PATCH 11/13] allow incomplete imports of filenames
Date: Mon, 10 Nov 2025 19:45:22 -0500	[thread overview]
Message-ID: <CAHC9VhQroZriXXeG=iYhQJ3_BwdSbn+wsk0hUeBSK3k+V0q=uQ@mail.gmail.com> (raw)
In-Reply-To: <20251109063745.2089578-12-viro@zeniv.linux.org.uk>

On Sun, Nov 9, 2025 at 1:38 AM Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> There are two filename-related problems in io_uring and its
> interplay with audit.
>
> Filenames are imported when request is submitted and used when
> it is processed.  Unfortunately, the latter may very well
> happen in a different thread.  In that case the reference to
> filename is put into the wrong audit_context - that of submitting
> thread, not the processing one.  Audit logics is called by
> the latter, and it really wants to be able to find the names
> in audit_context current (== processing) thread.
>
> Another related problem is the headache with refcounts -
> normally all references to given struct filename are visible
> only to one thread (the one that uses that struct filename).
> io_uring violates that - an extra reference is stashed in
> audit_context of submitter.  It gets dropped when submitter
> returns to userland, which can happen simultaneously with
> processing thread deciding to drop the reference it got.
>
> We paper over that by making refcount atomic, but that means
> pointless headache for everyone.
>
> Solution: the notion of partially imported filenames.  Namely,
> already copied from userland, but *not* exposed to audit yet.
>
> io_uring can create that in submitter thread, and complete the
> import (obtaining the usual reference to struct filename) in
> processing thread.
>
> Object: struct delayed_filename.
>
> Primitives for working with it:
>
> delayed_getname(&delayed_filename, user_string) - copies the name
> from userland, returning 0 and stashing the address of (still incomplete)
> struct filename in delayed_filename on success and returning -E... on
> error.
>
> delayed_getname_uflags(&delayed_filename, user_string, atflags) - similar,
> in the same relation to delayed_getname() as getname_uflags() is to getname()
>
> complete_getname(&delayed_getname) - completes the import of filename stashed
> in delayed_getname and returns struct filename to caller, emptying delayed_getname.
>
> dismiss_delayed_getname(&delayed_getname) - destructor; drops whatever
> might be stashed in delayed_getname, emptying it.
>
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---
>  fs/namei.c           |  45 +++++++++++++++++--
>  include/linux/fs.h   |  10 +++++
>  io_uring/fs.c        | 101 +++++++++++++++++++++++--------------------
>  io_uring/openclose.c |  16 +++----
>  io_uring/statx.c     |  17 +++-----
>  io_uring/xattr.c     |  30 +++++--------
>  6 files changed, 129 insertions(+), 90 deletions(-)

I don't have any patches to share for this yet, but as a FYI, I've
started working on some audit patches to deal with this issue in a
more general way; the io_uring approach of splitting processing
between a prep and work stage causes audit problems beyond just
filenames.  My current thought is to setup an audit_context in the
io_uring prep stage for those ops that need auditing, and then carry
around the context in the io_kiocb for later use, swapping it to
current->audit_context when the work is performed.  Still plenty of
hand wavy stuff to sort out, and it's entirely possible that it
doesn't work out, or is too ugly to see posted, but it seemed relevant
to the patch.

Regardless, I think this approach is reasonable; best case it is a
stopgap until we have something more general, worst case it becomes a
more permanent fix.  I can live with either outcome.

Acked-by: Paul Moore <paul@paul-moore.com>

-- 
paul-moore.com

  reply	other threads:[~2025-11-11  0:45 UTC|newest]

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-09  6:37 [RFC][PATCH 00/13] io_uring, struct filename and audit Al Viro
2025-11-09  6:37 ` [RFC][PATCH 01/13] do_faccessat(): import pathname only once Al Viro
2025-11-13 10:11   ` Jan Kara
2025-11-09  6:37 ` [RFC][PATCH 02/13] do_fchmodat(): " Al Viro
2025-11-13 10:12   ` Jan Kara
2025-11-09  6:37 ` [RFC][PATCH 03/13] do_fchownat(): " Al Viro
2025-11-13 10:13   ` Jan Kara
2025-11-09  6:37 ` [RFC][PATCH 04/13] do_utimes_path(): " Al Viro
2025-11-13 10:15   ` Jan Kara
2025-11-09  6:37 ` [RFC][PATCH 05/13] chdir(2): " Al Viro
2025-11-13 10:16   ` Jan Kara
2025-11-09  6:37 ` [RFC][PATCH 06/13] chroot(2): " Al Viro
2025-11-13 10:18   ` Jan Kara
2025-11-09  6:37 ` [RFC][PATCH 07/13] user_statfs(): " Al Viro
2025-11-13 10:18   ` Jan Kara
2025-11-09  6:37 ` [RFC][PATCH 08/13] do_sys_truncate(): " Al Viro
2025-11-13 10:18   ` Jan Kara
2025-11-09  6:37 ` [RFC][PATCH 09/13] do_readlinkat(): " Al Viro
2025-11-13 10:20   ` Jan Kara
2025-11-09  6:37 ` [RFC][PATCH 10/13] get rid of audit_reusename() Al Viro
2025-11-09 19:18   ` Linus Torvalds
2025-11-09 19:55     ` Mateusz Guzik
2025-11-09 20:22       ` Linus Torvalds
2025-11-09 22:18         ` Mateusz Guzik
2025-11-09 22:29           ` Linus Torvalds
2025-11-09 22:33             ` Mateusz Guzik
2025-11-09 22:39               ` Mateusz Guzik
2025-11-09 22:41               ` Linus Torvalds
2025-11-09 22:44                 ` Linus Torvalds
2025-11-09 23:07                   ` Linus Torvalds
2025-11-09 22:18     ` Linus Torvalds
2025-11-10  5:17       ` Al Viro
2025-11-10 16:41         ` Linus Torvalds
2025-11-10 19:58           ` Al Viro
2025-11-10 20:52             ` Linus Torvalds
2025-11-11  1:16               ` Al Viro
2025-11-12  9:26           ` Christian Brauner
2025-11-10  6:05       ` Al Viro
2025-11-10  6:36       ` Al Viro
2025-11-10 16:50         ` Linus Torvalds
2025-11-10 23:13   ` Paul Moore
2025-11-11  0:23     ` Paul Moore
2025-11-13 10:29   ` Jan Kara
2025-11-09  6:37 ` [RFC][PATCH 11/13] allow incomplete imports of filenames Al Viro
2025-11-11  0:45   ` Paul Moore [this message]
2025-11-11 14:41   ` Jens Axboe
2025-11-19  1:12     ` Al Viro
2025-11-19  1:14       ` Al Viro
2025-11-19  5:41         ` Al Viro
2025-11-09  6:37 ` [RFC][PATCH 12/13] fs: touch up predicts in putname() Al Viro
2025-11-09  6:37 ` [RFC][PATCH 13/13] struct filename ->refcnt doesn't need to be atomic Al Viro

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='CAHC9VhQroZriXXeG=iYhQJ3_BwdSbn+wsk0hUeBSK3k+V0q=uQ@mail.gmail.com' \
    --to=paul@paul-moore.com \
    --cc=audit@vger.kernel.org \
    --cc=axboe@kernel.dk \
    --cc=brauner@kernel.org \
    --cc=io-uring@vger.kernel.org \
    --cc=jack@suse.cz \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=mjguzik@gmail.com \
    --cc=torvalds@linux-foundation.org \
    --cc=viro@zeniv.linux.org.uk \
    /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