public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Al Viro <viro@zeniv.linux.org.uk>
To: Waiman Long <llong@redhat.com>
Cc: Paul Moore <paul@paul-moore.com>, Eric Paris <eparis@redhat.com>,
	Christian Brauner <brauner@kernel.org>,
	linux-kernel@vger.kernel.org, audit@vger.kernel.org,
	Richard Guy Briggs <rgb@redhat.com>,
	Ricardo Robaina <rrobaina@redhat.com>,
	Mateusz Guzik <mjguzik@gmail.com>
Subject: Re: [PATCH v2] audit: Avoid excessive dput/dget in audit_context setup and reset paths
Date: Thu, 5 Feb 2026 05:22:02 +0000	[thread overview]
Message-ID: <20260205052202.GQ3183987@ZenIV> (raw)
In-Reply-To: <cbaab46e-715d-4d06-80af-320caec7feb7@redhat.com>

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.

  parent reply	other threads:[~2026-02-05  5:20 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-03 19:44 [PATCH v2] audit: Avoid excessive dput/dget in audit_context setup and reset paths Waiman Long
2026-02-03 19:59 ` Al Viro
2026-02-03 20:18   ` Waiman Long
2026-02-03 20:05 ` Al Viro
2026-02-03 20:32   ` Waiman Long
2026-02-03 21:50     ` Al Viro
2026-02-03 23:26       ` Al Viro
2026-02-04  4:21         ` Waiman Long
2026-02-04  6:26           ` Al Viro
2026-02-04 18:16             ` Waiman Long
2026-02-04 20:18               ` Al Viro
2026-02-05  3:03                 ` Waiman Long
2026-02-05  4:45                   ` Waiman Long
2026-02-05 23:53                     ` Al Viro
2026-02-06  1:20                       ` Waiman Long
2026-02-06  4:11                         ` Waiman Long
2026-02-06  4:19                           ` Waiman Long
2026-02-06  5:22                           ` Al Viro
2026-02-06  6:31                             ` Al Viro
2026-02-06  6:38                               ` Al Viro
2026-02-06  7:13                             ` Al Viro
2026-02-06 19:16                             ` Waiman Long
2026-02-06 20:04                               ` Waiman Long
2026-02-06 20:38                                 ` Al Viro
2026-02-07  8:25                                 ` [PATCH][RFC] bug in unshare(2) failure recovery Al Viro
2026-02-07 23:06                                   ` Waiman Long
2026-02-17 12:49                                   ` Christian Brauner
2026-02-17 12:49                                   ` Christian Brauner
2026-02-06 20:29                               ` [PATCH v2] audit: Avoid excessive dput/dget in audit_context setup and reset paths Al Viro
2026-02-06 20:58                                 ` setns(2) vs. pivot_root(2) (was Re: [PATCH v2] audit: Avoid excessive dput/dget in audit_context setup and reset paths) Al Viro
2026-02-06 21:09                                   ` Al Viro
2026-02-17 13:12                                     ` Christian Brauner
2026-02-06  8:15                       ` [PATCH v2] audit: Avoid excessive dput/dget in audit_context setup and reset paths Al Viro
2026-02-05  5:22                   ` Al Viro [this message]
2026-02-05 13:59                     ` Waiman Long
2026-02-05 17:53                     ` Mateusz Guzik
2026-02-17 13:33                       ` Christian Brauner
2026-02-17 13:44                         ` Mateusz Guzik

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=20260205052202.GQ3183987@ZenIV \
    --to=viro@zeniv.linux.org.uk \
    --cc=audit@vger.kernel.org \
    --cc=brauner@kernel.org \
    --cc=eparis@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=llong@redhat.com \
    --cc=mjguzik@gmail.com \
    --cc=paul@paul-moore.com \
    --cc=rgb@redhat.com \
    --cc=rrobaina@redhat.com \
    /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