On Wed, Nov 13, 2024 at 10:23:55PM -0500, Paul Moore wrote: > > And while we are at it, > > parentlen = parentlen == AUDIT_NAME_FULL ? parent_len(path) : parentlen; > > is a bloody awful way to spell > > if (parentlen == AUDIT_NAME_FULL) > > parentlen = parent_len(path); > > What's more, parent_len(path) starts with *yet* *another* strlen(path), > > followed by really awful crap - we trim the trailing slashes (if any), > > then search for the last slash before that... is that really worth > > the chance to skip that strncmp()? > > Pretty much all of the audit code is awkward at best Al, you should know > that.
Do I ever... > We're not going to fix it all in one patch, and considering the nature > of this patch effort, I think there is going to be a lot of value in keeping > the initial fix patch to a minimum to ease backporting. We can improve on > some of those other issues in a second patch or at a later time. > > As a reminder to everyone, patches are always welcome. Fixing things is a > great way to channel disgust into something much more useful. > > > > > + if (p[pathlen - 1] == '/') > > > + pathlen--; > > > + > > > + if (pathlen != dlen) > > > + return 1; > > > > > > return strncmp(p, dname->name, dlen); > > > > ... which really should've been memcmp(), at that. > > Agreed. See above. OK, my primary interest here is to separate struct filename from that stuff as much as possible. So we will end up stomping on the same ground for a while here. FWIW, my current filename-related pile is in #next.filename; there will be a lot more on the VFS side, but one of the obvious targets is ->aname, so __audit_inode() and its vicinity will get affected. We'll need to coordinate that stuff. Anyway, regarding audit_compare_dname_path(), handling just the last '/' is not enough - there might be any number of trailing slashes, not just one. Another fun issue with looking for matches is this: mkdir /tmp/foo mkdir /tmp/foo/bar mkdir /tmp/blah ln -s ../foo/bar/baz /tmp/blah/barf echo crap > /tmp/blah/barf The last one will create a regular file "baz" in /tmp/foo/bar and write "crap\n" into it. With the only pathname passed to open(2) being "/tmp/blah/barf". And there may be a longer chain of symlinks like that. What do you want to see reported in such case?
