On 2018-12-14 17:02, Paul Moore wrote: > On Fri, Dec 14, 2018 at 11:27 AM Richard Guy Briggs <rgb@xxxxxxxxxx> wrote: > > On 2018-12-12 08:03, Paul Moore wrote: > > > On Fri, Nov 16, 2018 at 12:34 PM Richard Guy Briggs <rgb@xxxxxxxxxx> wrote: > > > > On user and remote filesystems, a forced umount can still hang due to > > > > attemting to fetch the fcaps of a mounted filesystem that is no longer > > > > available. > > > > > > > > These two patches take different approaches to address this, one by > > > > avoiding the lookup when the MNT_FORCE flag is included, the other by > > > > providing a method to filter out auditing specified types of filesystems. > > > > > > > > This can happen on ceph, cifs, 9p, lustre, fuse (gluster) or NFS. > > > > > > > > Arguably the better way to address this issue is to disable auditing > > > > processes that touch removable filesystems. > > > > Please see the github issue tracker > > > > https://github.com/linux-audit/audit-kernel/issues/100 > > > > > > > > Richard Guy Briggs (2): > > > > audit: avoid fcaps on MNT_FORCE > > > > audit: moar filter PATH records keyed on filesystem magic > > > > > > > > fs/namei.c | 2 +- > > > > fs/namespace.c | 3 +++ > > > > include/linux/audit.h | 8 ++++++-- > > > > kernel/audit.c | 5 +++-- > > > > kernel/audit.h | 2 +- > > > > kernel/auditsc.c | 29 ++++++++++++++++++++++++++--- > > > > 6 files changed, 40 insertions(+), 9 deletions(-) > > > > > > Just to get this out of the way, don't use "moar", spell it properly. > > > > > > Beyond that, it's not clear to me from your cover letter if you are > > > proposing these patches as an "or" or as an "and"; assuming the > > > patch(es) are reasonable, do you want us to merge both of these > > > patches, or only the one we like the most? > > > > I would like each one to be considered on its own merits. > > > > The second was discussed back when the logs were flooded with "(null)" > > PATH records due to debugfs and tracefs noise. Do you agree with the > > general concept or not? > > I believe I was in favor of this back then, and I think it is still a > reasonable feature to add independent of the umount hang problem. I wasn't so keen then, but see more use for it now. > One possible enhancement might be to also support filesystem names and > not just magic numbers. This could either be done in userspace or the > kernel via AUDIT_FSNAME, or similar. If you do take the _FSNAME > approach you should have the kernel convert that to a magic number > when it translates the rule (performance reasons). I had looked at filesystem names previously with Steve and I seem to recall that was much better left to userspace to convert. The biggest challenge I see here is that in-kernel filsystems have all their magic numbers listed in the kernel, whereas discovering the magic numbers for fuse and other remote filesystems is a bit harder. This was one of the reasons I wanted to include the magic number in the name= field in patch #1 for ghak8 so it was easy to figure out what type of filesystem was involved. > > The first is being picked apart (rightfully) due to assumptions and > > choices made long ago in the audit system. So while it is still in far > > more flux than the second patch, I think it has the potential to fix the > > problem more correctly and permanently but in the process may challenge > > our rules about the format and invariability of audit records. The > > basic premise is to prevent audit from trying to get information (fcaps) > > from a filesystem that is likely to be far more ephemeral than local > > on-disk kernel filesystems or to fail to do so more gracefully. > > There is one minor nit: use "flags" instead of "lflags" in the > audit_inode parameter list, the local "flags" variable can be changed > to something else; the parameters are what callers see, make them > simple and familiar. Noted. > However, beyond that I think the general approach of not recording > fcaps is reasonable if we can't reliably do it. What do the fcaps > entries look like in this particular case, are they "0" or "?"? I > would suggest "?" is the correct answer here. I'd agree "?" would be the best option to make it clear it is not available rather than just zero. > paul moore - RGB -- Richard Guy Briggs <rgb@xxxxxxxxxx> Sr. S/W Engineer, Kernel Security, Base Operating Systems Remote, Ottawa, Red Hat Canada IRC: rgb, SunRaycer Voice: +1.647.777.2635, Internal: (81) 32635
