From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 42EF933E37C for ; Wed, 4 Feb 2026 18:16:21 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=170.10.129.124 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1770228981; cv=none; b=sFt8KInnyI2PFSau6cVNYRVVZD2tL6JodN6OgR/h4R9s5Wd1cb+iSOMxNcOTqki5yP4BhImYQYM3yUvRgDK4s9IQWrFlQ/jRS3bpjCTPzGWaQGs96F/xd6Q69kThEyb1hOU9uJoCl55UCzcBiickz5uFi2zC4ceGLFdQ3Y1l78k= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1770228981; c=relaxed/simple; bh=jxUGSbafBVOeo4YVIEroTpfWhgu9nLcAcQdyGA5hHbs=; h=From:Message-ID:Date:MIME-Version:Subject:To:Cc:References: In-Reply-To:Content-Type; b=U8MR6S6Q9O79QHV+p5aDR5RHk66NOo3Ahpn3hNt5o5TUI/d1w+FF5ViBXsH4eNhQLOW9ci9d78wXdfJUgVhgY00UHxO2v92etHAMZi/FRwbwwvsM7pwgKJyMsDq+itv9j0JkGN4bDhmT8EfAsG351F2/zNDjGHbLiAUBmGXiVpo= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com; spf=pass smtp.mailfrom=redhat.com; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b=aTh10WJH; dkim=pass (2048-bit key) header.d=redhat.com header.i=@redhat.com header.b=lqBDv/LD; arc=none smtp.client-ip=170.10.129.124 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=redhat.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="aTh10WJH"; dkim=pass (2048-bit key) header.d=redhat.com header.i=@redhat.com header.b="lqBDv/LD" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1770228980; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=zCL3Cm+Z0d+kZcWyu1dMmuNDXI8bD0nxr3EWpTI/qh8=; b=aTh10WJHMJ+znet3ggiriJp9jRBbKUvc+lMJLQueqPhpWPyCYRXJChAOYhZgaCA1x7BqtC MBLOqi/Kho2EUk4I5Az0dlpAUaxX5uGz5DeNWY4v4UPsaL5xIjZPJvkONshpsg3wbVaW9u Ayvn3DISfDSz1dvLRYk+xNNWnFt76YU= Received: from mail-qt1-f199.google.com (mail-qt1-f199.google.com [209.85.160.199]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-635-uid9K1zkMcekHptMLO_AaA-1; Wed, 04 Feb 2026 13:16:19 -0500 X-MC-Unique: uid9K1zkMcekHptMLO_AaA-1 X-Mimecast-MFC-AGG-ID: uid9K1zkMcekHptMLO_AaA_1770228979 Received: by mail-qt1-f199.google.com with SMTP id d75a77b69052e-5033c483b74so93541cf.2 for ; Wed, 04 Feb 2026 10:16:19 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=google; t=1770228978; x=1770833778; darn=vger.kernel.org; h=content-transfer-encoding:in-reply-to:content-language:references :cc:to:subject:user-agent:mime-version:date:message-id:from:from:to :cc:subject:date:message-id:reply-to; bh=zCL3Cm+Z0d+kZcWyu1dMmuNDXI8bD0nxr3EWpTI/qh8=; b=lqBDv/LDqU0Z/oOnmnPlZx0zDKocc+r7PXDpP17QvrsROyLuubiYWNkyJ/UOJ0LdfF ccP7giR8bmw90xCe5au1mw01Ig4OFJAhX1vEWsPfIkc9HlMutB2xzrMCOrFshKb2m2bX XEtLHArDt0kaS2iSrfqzMct3KnSVZIMQzJS2dCY4Hhbodr4f72s1Vk1HuHIC339/iOCC zPa4M+9fkh88SyVGh8wUouuBB8s0WoRYv8rSjkLzFaa5f9BeuCeJcQs7Ugvsf2XbhFD3 CUYeJ0YUEcV8OTDjs9c/ynDd4K8UqkqGoNPc6a4aoalO9lYnT0WED5YawAfFOMn2HeCy 244A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1770228978; x=1770833778; h=content-transfer-encoding:in-reply-to:content-language:references :cc:to:subject:user-agent:mime-version:date:message-id:from:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=zCL3Cm+Z0d+kZcWyu1dMmuNDXI8bD0nxr3EWpTI/qh8=; b=T5w4GiG8uy4my2DYnmGRIQBaTfCq2UbECULALanrPi2Tt3yLA52fzG7m5P6JXQHdA5 iB/P7pPdqv1TB/+Wah5Xsu3M8VhwReXlDQZLbOP+7Aw+WD9xbvqo6WkSu8/EZCQlxmoo 9zIQvhDQP0g661cEjVgZWsN5jibUZSgOzRAtV1s5KYi3eivobNAcG1gvvr/U8blGX4u3 59JJFy09lnMZClv7CERjvXBDMAKVDlKEb4JzjClkPyviHtRq5HVuWvKfgFyOYAHG3q56 gp8dXoLlZ/sRumqnyB9Va4XgBDh1CLEJkY7gdFagwybEP+SAWON7mJ877at4JHCLljRK aR+w== X-Forwarded-Encrypted: i=1; AJvYcCWkAeV/Tr5o7H/Gf40m5cT2BydCvHem/Q5a5FpbW3EGEXGOFdZ2PBa1ky+p9W8Z2gIQmQhxhfO/y3UGMbk=@vger.kernel.org X-Gm-Message-State: AOJu0Yz9U444sTUUF3g4E/AcBnQsN5oxFshYK/hcgJbNrdfbmc79AUVQ r1+yTjlBgmWjPYTrO7js+un0v62NLQtiYH+2gAgEYG4ZSntrJsvopVT2lYmbxLGQFMeImkpYTX+ bo7NBpSalhCwW84Yr8OOAqsrLht2z7/+W9gW5WiR1FzccXSgUdK0CMaiAYstj4f3Kub4YwMgEbg == X-Gm-Gg: AZuq6aJNFS8tX9DSUFj379krscvL2C/eUvRK0kc5G5cV4vCyQBTKA94F54R18BkMNn3 cD0lH2SgouVgvlPNA2PcEtYRqMBBVJY5skTveREwONrEcGIQ0f1c98AtfYdzRxLNkjN415kPphn EfzzEB/Pzu4JMLIb1e1NOUE8mM8+P8DKkJjLS6A++yLgItrDFRtGMoEd0iquCfiZ6khEjJbNkDL +wq73Rr01wItVJLWUbGIGvnFFfs+8netz5X5HW9Z50LXZr4w5qok4kVQPZuTm0bXUkBkq4Y4HPF bHsFjlPsSsYklfQslF+PtC6H7gwASPqF4DuvQrcssJyKS0ZpvNAVxBWzBfy+9SN4kMSTPY5yevZ +OlGk5ieBimdrAluNdTdECtfo0v/+nOhfCuV+nNHHLqbf3TNNLJgepbu5 X-Received: by 2002:ac8:7f8a:0:b0:502:a28c:f195 with SMTP id d75a77b69052e-5061c0b41a5mr49630531cf.13.1770228977603; Wed, 04 Feb 2026 10:16:17 -0800 (PST) X-Received: by 2002:ac8:7f8a:0:b0:502:a28c:f195 with SMTP id d75a77b69052e-5061c0b41a5mr49629881cf.13.1770228977043; Wed, 04 Feb 2026 10:16:17 -0800 (PST) Received: from ?IPV6:2601:188:c102:b180:1f8b:71d0:77b1:1f6e? ([2601:188:c102:b180:1f8b:71d0:77b1:1f6e]) by smtp.gmail.com with ESMTPSA id d75a77b69052e-5061c1e9971sm22097291cf.21.2026.02.04.10.16.15 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 04 Feb 2026 10:16:16 -0800 (PST) From: Waiman Long X-Google-Original-From: Waiman Long Message-ID: <46d5c480-87d0-4f6a-bcc2-6c936c87e216@redhat.com> Date: Wed, 4 Feb 2026 13:16:15 -0500 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v2] audit: Avoid excessive dput/dget in audit_context setup and reset paths To: Al Viro , Waiman Long Cc: Paul Moore , Eric Paris , Christian Brauner , linux-kernel@vger.kernel.org, audit@vger.kernel.org, Richard Guy Briggs , Ricardo Robaina References: <20260203194433.1738162-1-longman@redhat.com> <20260203200505.GH3183987@ZenIV> <590a36e6-8d11-411a-8fcd-d93eef96f0e9@redhat.com> <20260203215002.GI3183987@ZenIV> <20260203232634.GJ3183987@ZenIV> <6661f966-5235-49ca-bf1f-d1ae2ae32f0d@redhat.com> <20260204062614.GK3183987@ZenIV> Content-Language: en-US In-Reply-To: <20260204062614.GK3183987@ZenIV> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit On 2/4/26 1:26 AM, Al Viro wrote: > On Tue, Feb 03, 2026 at 11:21:23PM -0500, Waiman Long wrote: > >> Interesting. So are you thinking about a reference on the pwd inside the >> fs_struct? > No. fs_struct has pwd pinned - both dentry and mount. > > Each thread has a reference to fs_struct instance in its task_struct (task->fs). > > It is possible for several threads to have their task->fs pointing to the > same instance. > > A thread may modify its fs_struct pointer, making it point to a different > fs_struct instance; that happens on unshare(2). In that case new instance > is an identical copy of the original one; reference counts of mounts and dentries > (both for pwd and root) are incremented to account for additional references > in the copy. That assignment to current->fs happens under task_lock(current). > > A child gets either a reference to identical copy of parent's fs_struct or > an extra reference to parent's fs_struct. In either case child->fs is > set before the child gets to run; as the matter of fact, that happens before > anyone besides the parent might observe the task_struct of child. > > That's it - no other stores to task_struct.fs are ever allowed; no other > thread can change your reference under you. > > > Other than initializing a new copy, *all* stores to fs_struct.pwd are done > to current->fs.pwd; access to any other fs_struct instances is read-only. > The only way another thread might change your pwd is if that thread shares > fs_struct with you. They can observe its contents, as long as they take > care to hold task_lock(your_thread), but no more than that. > > What's more, if current->fs is the sole reference to fs_struct instance, > it will remain such until you spawn a child and set it to share your > instance (CLONE_FS). No other thread can gain extra references to it, > etc. > > > What it means is that if you are holding the sole reference to current->fs, > current->fs->pwd contents will remain unchanged (and pinning the same mount > and dentry) through the entire syscall with very few exceptions: > 1) you spawn a child with CLONE_FS - that has to be either > clone(2) or io_uring_setup(2) (the latter - via worker threads). > 2) you explicitly modify your pwd - in chdir(2), fchdir(2), > pivot_root(2), setns(2) (with CLONE_NEWNS; it switches you to the root of > namespace you've joined) > > As long as these exceptions are accounted for, audit could check > current->fs->users *and* skip incrementing refcounts if it's equal to 1. > It would need to remember whether these reference had been taken - > rechecking condition won't work, since other threads might by gone > by the time we leave the syscall turning "shared" to "not shared". > > That way we can avoid grabbing any references in a fairly common > case. Thanks for the detailed explanation. I am thinking about something like the code diff below. Of course, there are other corner cases like unshare(2) that still needs to be handled. Do you think something like this is viable? Thanks, Longman =========================[ Cut here ]=================================== diff --git a/fs/fs_struct.c b/fs/fs_struct.c index b8c46c5a38a0..09c97059776d 100644 --- a/fs/fs_struct.c +++ b/fs/fs_struct.c @@ -25,6 +25,23 @@ void set_fs_root(struct fs_struct *fs, const struct path *pa>                 path_put(&old_root);  } +static void unshare_fs_pwd_locked(struct fs_struct *fs) +{ +       get_task_struct(current); +       fs->pwd_waiter = current; +retry: +       spin_unlock(&fs->seq.lock); +       /* Sleep until pwd_refs reaches 0 */ +       set_current_state(TASK_UNINTERRUPTIBLE); +       schedule(); +       spin_lock(&fs->seq.lock); +       if (fs->pwd_refs) +               goto retry; +       __set_current_state(TASK_RUNNING); +       fs->pwd_waiter = NULL; +       put_task_struct(current); +} +  /*   * Replace the fs->{pwdmnt,pwd} with {mnt,dentry}. Put the old values.   * It can block. @@ -34,7 +51,15 @@ void set_fs_pwd(struct fs_struct *fs, const struct path *pat>         struct path old_pwd;         path_get(path); -       write_seqlock(&fs->seq); +       /* +        * As the pwd may be shared with other tasks, we need to break down +        * the write_seqlock() call to its component spin_lock() and +        * do_write_seqcount_begin() calls. +        */ +       spin_lock(&fs->seq.lock); +       if (unlikely(fs->pwd_refs)) +               unshare_fs_pwd_locked(fs); +  do_write_seqcount_begin(&fs->seq.seqcount.seqcount);         old_pwd = fs->pwd;         fs->pwd = *path;         write_sequnlock(&fs->seq); diff --git a/include/linux/fs_struct.h b/include/linux/fs_struct.h index 0070764b790a..3848189893c7 100644 --- a/include/linux/fs_struct.h +++ b/include/linux/fs_struct.h @@ -13,6 +13,8 @@ struct fs_struct {         int umask;         int in_exec;         struct path root, pwd; +       int pwd_refs; +       struct task_struct *pwd_waiter; /* set_fs_pwd() waiter */  } __randomize_layout;  extern struct kmem_cache *fs_cachep; @@ -40,6 +42,43 @@ static inline void get_fs_pwd(struct fs_struct *fs, struct p>         read_sequnlock_excl(&fs->seq);  } +/* + * The get_fs_pwd_share/put_fs_pwd_share APIs should only be used by callers + * that need to keep the pwd references for a short time to avoid blocking + * set_fs_pwd() caller for long time. + */ +static inline bool get_fs_pwd_share(struct fs_struct *fs, struct path *pwd) +{ +       bool share = true; + +       read_seqlock_excl(&fs->seq); +       *pwd = fs->pwd; +       share = !fs->pwd_waiter; +       if (likely(share)) +               fs->pwd_refs++; +       else +               path_get(pwd); +       read_sequnlock_excl(&fs->seq); +       return share; +} + +static inline void put_fs_pwd_share(struct fs_struct *fs, struct path *pwd, bo> +{ +       struct task_struct *wakeup_task = NULL; + +       if (!share) { +               path_put(pwd); +               return; +       } +       read_seqlock_excl(&fs->seq); +       fs->pwd_refs--; +       if (fs->pwd_waiter && !fs->pwd_refs) +               wakeup_task = fs->pwd_waiter; +       read_sequnlock_excl(&fs->seq); +       if (wakeup_task) +               wake_up_process(wakeup_task); +} +  extern bool current_chrooted(void);  static inline int current_umask(void)