All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jeff Layton <jlayton@kernel.org>
To: Tao Cui <cui.tao@linux.dev>,
	linux-fsdevel@vger.kernel.org, cel@kernel.org
Cc: alex.aring@gmail.com, viro@zeniv.linux.org.uk,
	brauner@kernel.org,  linux-kernel@vger.kernel.org,
	avagin@openvz.org, khorenko@virtuozzo.com,  bcodding@redhat.com,
	Tao Cui <cuitao@kylinos.cn>
Subject: Re: [PATCH v2] fs/locks: filter OFD locks from /proc/locks for foreign pid namespaces
Date: Wed, 02 Sep 2026 09:03:50 -0400	[thread overview]
Message-ID: <ab253791f92c329865168c4734c9bfe2f8537a5e.camel@kernel.org> (raw)
In-Reply-To: <20260902125139.975253-1-cui.tao@linux.dev>

On Wed, 2026-09-02 at 20:51 +0800, Tao Cui wrote:
> From: Tao Cui <cuitao@kylinos.cn>
> 
> Since Linux 4.9, /proc/locks only shows locks whose owning process is
> visible in the reader's pid namespace.  The check cannot see the
> owner of an OFD lock, though: locks_translate_pid() always returns
> -1 for FL_OFDLCK, so an OFD lock held outside of that namespace
> bypasses the filter and is shown with its device/inode numbers and
> byte range, e.g.
> 
>     4: OFDLCK ADVISORY  WRITE -1 fd:00:70536961 0 EOF
> 
> This was observed on a Kubernetes node: a pod reading /proc/locks
> listed the OFD write lock of an unrelated host process, while the
> POSIX locks of the very same process were correctly hidden.  The
> device/inode pair identifies which host file is locked, the byte
> range discloses where it is actively written, and the
> appearance/disappearance of entries reflects host task activity,
> contrary to the documented per-pidns visibility of /proc/locks
> (proc_locks(5)).
> 
> OFD locks record the owner tgid in flc_pid, so use it for the
> visibility decision in locks_show(), mirroring what
> locks_translate_pid() does for POSIX locks.  The pid column is still
> reported as -1 for OFD locks; only the filter decision changes.
> Remote locks keep their negative flc_pid and stay visible as before.
> 
> Verified with an OFD and a POSIX write lock held in the initial pid
> namespace while a process in a fresh pid namespace reads /proc/locks:
> the OFD entry is visible without this patch and hidden with it, the
> POSIX entry is hidden in both cases.
> 
> Signed-off-by: Tao Cui <cuitao@kylinos.cn>
> 
> ---
> 
> Changes since v1: keep remote OFD locks (negative flc_pid) visible in
> non-initial pid namespaces, as locks_translate_pid() does for remote
> POSIX locks.
> ---
>  fs/locks.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/fs/locks.c b/fs/locks.c
> index 6e4ff7fcec05..4af1385682e4 100644
> --- a/fs/locks.c
> +++ b/fs/locks.c
> @@ -3022,6 +3022,25 @@ static int locks_show(struct seq_file *f, void *v)
>  
>  	cur = hlist_entry(v, struct file_lock_core, flc_link);
>  
> +	/*
> +	 * OFD locks are reported with pid -1, so the filter below cannot see
> +	 * their owner; flc_pid holds the owner tgid, so filter on it.
> +	 * Remote locks keep a negative flc_pid and stay visible as before.
> +	 */
> +	if ((cur->flc_flags & FL_OFDLCK) && cur->flc_pid > 0 &&
> +	    proc_pidns != &init_pid_ns) {
> +		struct pid *pid;
> +		bool visible = false;
> +
> +		rcu_read_lock();
> +		pid = find_pid_ns(cur->flc_pid, &init_pid_ns);
> +		if (pid)
> +			visible = pid_nr_ns(pid, proc_pidns) != 0;
> +		rcu_read_unlock();
> +		if (!visible)
> +			return 0;
> +	}
> +
>  	if (locks_translate_pid(cur, proc_pidns) == 0)
>  		return 0;
>  

First: I think this may be the wrong place to do this. Why not fold
this change into locks_translate_pid()? It seems like we'd have
inconsistent results wrt lock visibility between /proc/locks and
F_GETLK if you do this here.

Thinking about this some more though, I wonder if trying to hide these
locks is the right thing to do. These locks do exist and they do block
you from acquiring a lock. If you go to look at /proc/locks and don't
see them, that seems confusing.

Would we be better off showing all the locks and reporting the pid as a
negative value for ones acquired in foreign namespaces, like we do for
remote locks?
-- 
Jeff Layton <jlayton@kernel.org>

  reply	other threads:[~2026-09-02 13:03 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-02 12:51 [PATCH v2] fs/locks: filter OFD locks from /proc/locks for foreign pid namespaces Tao Cui
2026-09-02 13:03 ` Jeff Layton [this message]
2026-09-03  2:10   ` Tao Cui
2026-09-03 19:06     ` Jeff Layton

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=ab253791f92c329865168c4734c9bfe2f8537a5e.camel@kernel.org \
    --to=jlayton@kernel.org \
    --cc=alex.aring@gmail.com \
    --cc=avagin@openvz.org \
    --cc=bcodding@redhat.com \
    --cc=brauner@kernel.org \
    --cc=cel@kernel.org \
    --cc=cui.tao@linux.dev \
    --cc=cuitao@kylinos.cn \
    --cc=khorenko@virtuozzo.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=viro@zeniv.linux.org.uk \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.