linux-kernel - Re: [PATCH v2] audit: Avoid excessive dput/dget in audit

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] [day] [month] [year] [list]

Message-ID: <20260205052202.GQ3183987@ZenIV>
Date: Thu, 5 Feb 2026 05:22:02 +0000
From: Al Viro <viro@...iv.linux.org.uk>
To: Waiman Long <llong@...hat.com>
Cc: Paul Moore <paul@...l-moore.com>, Eric Paris <eparis@...hat.com>,
	Christian Brauner <brauner@...nel.org>,
	linux-kernel@...r.kernel.org, audit@...r.kernel.org,
	Richard Guy Briggs <rgb@...hat.com>,
	Ricardo Robaina <rrobaina@...hat.com>,
	Mateusz Guzik <mjguzik@...il.com>
Subject: Re: [PATCH v2] audit: Avoid excessive dput/dget in audit_context
 setup and reset paths

On Wed, Feb 04, 2026 at 10:03:33PM -0500, Waiman Long wrote:

> Now I realize that there is indeed a deadlock problem. Scrap that. Now I
> have a simpler idea that shouldn't have this type of deadlock problem. So
> what do you think about the sample code below?

That it's rather bizarre, TBH.  Basically, you are allowing to park
a number of (identical) references in there instead of dropping them,
with your 'xrefs' being the count of skipped drops.  get_share either
clones a reference or uses up one of those skipped drops; put_share parks
the reference if possible.  And set discards everything not used up...

It could be made to work, but... ouch.  It looks like a special-cased
variant of something fairly generic, with really confusing calling
conventions.  Let me poke around and see if we have any other candidates
for something similar; if nothing else, current->fs->root is interesting
and not just for audit pathologies...

Note, BTW, that there's chroot_fs_refs() to deal with, along with
free_fs_struct() (at least).  This stuff is encapsulated in
fs/fs_struct.c and include/linux/fs_struct.h...  Oh, hell.

"fs: inline current_umask() and move it to fs_struct.h" had been a bad
idea; I'd missed it when it had been posted, but...  exposing the damn
thing all over the place *and* bringing sched.h into it?  Microoptimizations
are fine, and this one might have a measurable effect, but it grows
the subset of the kernel we need to audit when we deal with changing
the damn object by at least a couple of orders of magnitude ;-/  Sigh...

Mateusz: *is* there a measurable effect?  Because if there isn't, I'm
very tempted to simply revert that thing.  "Churn of adding fs_struct.h
as needed" is not the problem - try "exposing the object internals to
far larger subset of the kernel".  We had interesting bugs with weird
shit deciding to poke in there, locking and refcounting be damned.
Encapsulation is really called for in some cases...  And yes, I ought to
have caught that one back in November and asked _then_; mea culpa.