Re: [RFC] struct filename, io_uring and audit troubles

[Al Viro <viro@zeniv.linux.org.uk>]

On Mon, Sep 23, 2024 at 08:11:51PM -0400, Paul Moore wrote:
> > Umm...  IIRC, sgrubb had been involved in the spec-related horrors, but
> > that was a long time ago...
> 
> Yep, he was.  Last I spoke to Steve a year or so ago, audit was no
> longer part of his job description; Steve still maintains his
> userspace audit tools, but that is a nights/weekends job as far as I
> understand.
> 
> The last time I was involved in any audit/CC spec related work was
> well over a decade ago now, and all of those CC protection profiles
> have long since expired and been replaced.

Interesting...  I guess eparis would be the next victim^Wpossible source
of information.
 
> >         * get rid of the "repeated getname() on the same address is going to
> > give you the same object" - that can't be relied upon without audit, for one
> > thing and for another... having a syscall that takes two pathnames that gives
> > different audit log (if not predicate evaluation) in cases when those are
> > identical pointers vs. strings with identical contenst is, IMO, somewhat
> > undesirable.  That kills filename->uaddr.
> 
> /uaddr/uptr/ if I'm following you correctly, but yeah, that all seems good.

*nod*

> >         * looking at the users of that stuff, I would probably prefer to
> > separate getname*() from insertion into audit context.  It's not that
> > tricky - __set_nameidata() catches *everything* that uses nd->name (i.e.
> > all that audit_inode() calls in fs/namei.c use).
> 
> That should be a pretty significant simplification, that sounds good to me.
> 
> > ... What remains is
> >         do_symlinkat() for symlink body
> >         fs_index() on the argument (if we want to bother - it's a part
> > of weird Missed'em'V sysfs(2) syscall; I sincerely doubt that there's
> > anybody who'd use it)
> 
> We probably should bother, folks that really care about audit don't
> like blind spots.  Perhaps make it a separate patch if it isn't too
> ugly to split it out.

Heh...  I suggest you to look at the manpage of that thing.

sysfs(1, "ext2") => echo $((`sed -ne "/\text2$/=" </proc/filesystems` - 1))
sysfs(2, 10) => sed -ne "11s/.*\t//p" </proc/filesystems
sysfs(3) => wc -l </proc/filesystems

Yes, really - find position of filesystem type in the list of registered
filesystems by name (0-based numeration), find the name of filesystem
type by position and find the number of registered filesystem types.
Missed'em'V had no synthetic filesystems...

And the string is, of course, not a pathname of any sort, so I'd argue that
spewing PATH record into audit log is a bug.  Not that the number you get
from sysfs(1, something) had been usable for anything other than passing it
to sysfs(2, number) and getting the same string back - you can't pass that
number to mount(2) or anything of that kind.  I suspect that the only
reason this syscall exists is some binary emulation - introduced in 1.1.11,
not supported by glibc at least since 2014 (and almost certainly way before
that).

> >         That's all it takes.  With that done, we can kill ->aname;
> > just look in the ->names_list for the first entry with given ->name -
> > as in, given struct filename * value, no need to look inside.
> 
> Seems reasonable to me.  I can't imagine these special cases being any
> worse than what we have now in fs/namei.c, and if nothing else having
> a single catch point for the bulk of the VFS lookups makes it worth it
> as far as I'm concerned.

Huh?  Right now we allocate audit_names at getname_flags()/getname_kernel()
time; grep for audit_getname() - that's as centralized as it gets.
What I want to do is somewhat _de_centralize it; that way they would not
go anywhere other than audit_context of the thread actually doing the
work.

There is a lot of calls of audit_inode(), but I'm not planning to touch
any of those.