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

Message-ID: <be783cfb-ef65-4546-81eb-5eb6a18d4ef5@redhat.com>
Date: Thu, 5 Feb 2026 23:19:56 -0500
From: Waiman Long <llong@...hat.com>
To: Waiman Long <llong@...hat.com>, Al Viro <viro@...iv.linux.org.uk>
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>
Subject: Re: [PATCH v2] audit: Avoid excessive dput/dget in audit_context
 setup and reset paths

On 2/5/26 11:11 PM, Waiman Long wrote:
> On 2/5/26 8:20 PM, Waiman Long wrote:
>> On 2/5/26 6:53 PM, Al Viro wrote:
>>> On Wed, Feb 04, 2026 at 11:45:17PM -0500, Waiman Long wrote:
>>>
>>>> @@ -70,6 +74,8 @@ void chroot_fs_refs(const struct path *old_root, 
>>>> const
>>>> struct>
>>>>                                  count++;
>>>>                                  path_get(new_root);
>>>>                          }
>>>> +                       count += fs->pwd_xrefs;
>>>> +                       fs->pwd_xrefs = 0;
>>>>                          write_sequnlock(&fs->seq);
>>> Nope - you only need that for threads that have ->pwd equal to 
>>> old_root.
>>> Incidentally, I'd forgotten about that sucker - it kills the idea of
>>> fdget-like tricks dead, more's the pity.  Third-party modification of
>>> task->fs->pwd (under task->lock and task->fs->seq), possible even with
>>> task->fs->users == 1.
>>
>> Yes, I am aware of that when I took a further look at the patch that 
>> I sent out yesterday. I am testing the updated patch now and is 
>> trying to figure out why I get a warning from 
>> mntput_no_expire_slowpath() with a count of -1 when doing an umount. 
>> It is off by 1 somewhere. I will post the patch once I resolve this bug. 
>
> I now know why there are warnings. The problem is in the copy_mnt_ns() 
> function in fs/namespace.c:
>
> __latent_entropy
> struct mnt_namespace *copy_mnt_ns(u64 flags, struct mnt_namespace *ns,
>                 struct user_namespace *user_ns, struct fs_struct *new_fs)
> {
>   :
>                 if (new_fs) {
>                         if (&p->mnt == new_fs->root.mnt) {
>                                 new_fs->root.mnt = mntget(&q->mnt);
>                                 rootmnt = &p->mnt;
>                         }
>                         if (&p->mnt == new_fs->pwd.mnt) {
>                                 new_fs->pwd.mnt = mntget(&q->mnt);
>                                 pwdmnt = &p->mnt;
>                         }
>                 }
>
> It is replacing the fs->pwd.mnt with a new one while pwd_refs is 1. I 
> can make this work with the new fs_struct field. I do have one 
> question though. Do we need to acquire write_seqlock(&new_fs->seq) if 
> we are changing root or pwd here or if the new_fs are in such a state 
> that it will never change when this copying operation is in progress? 

Ah, there are comment above saying

         /*
          * Second pass: switch the tsk->fs->* elements and mark new 
vfsmounts
          * as belonging to new namespace.  We have already acquired a 
private
          * fs_struct, so tsk->fs->lock is not needed.
          */

So I guess it is safe to make change without taking the lock.

Cheers,
Longman