linux-kernel - Re: [PATCH] filelock: add file path in lock_get

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: <7ce8cadfc432368924134ae96f8d9710282a15d6.camel@kernel.org>
Date: Tue, 23 Jul 2024 09:16:10 -0400
From: Jeff Layton <jlayton@...nel.org>
To: Yuwei Guan <Yuwei.Guan@...krlife.com>, chuck.lever@...cle.com, 
 alex.aring@...il.com, viro@...iv.linux.org.uk, brauner@...nel.org,
 jack@...e.cz
Cc: linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org, 
	ssawgyw@...il.com, Dashi.Luban@...krlife.com
Subject: Re: [PATCH] filelock: add file path in lock_get_status() to show
 more debug info

On Tue, 2024-07-23 at 20:38 +0800, Yuwei Guan wrote:
> The current lock_get_status() function shows ino, but it’s not
> intuitive enough for debugging. This patch will add the file’s
> path information, making it easier to debug which specific file
> is holding the lock.
> 
> Signed-off-by: Yuwei Guan <Yuwei.Guan@...krlife.com>
> ---
>  fs/locks.c | 25 ++++++++++++++++++++++++-
>  1 file changed, 24 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/locks.c b/fs/locks.c
> index bdd94c32256f..feb0a4427a5b 100644
> --- a/fs/locks.c
> +++ b/fs/locks.c
> @@ -2764,6 +2764,8 @@ static void lock_get_status(struct seq_file *f,
> struct file_lock_core *flc,
>  	struct pid_namespace *proc_pidns = proc_pid_ns(file_inode(f-
> >file)->i_sb);
>  	int type = flc->flc_type;
>  	struct file_lock *fl = file_lock(flc);
> +	struct dentry *dentry = NULL;
> +	char *path, *pathbuf;
>  
>  	pid = locks_translate_pid(flc, proc_pidns);
>  
> @@ -2819,8 +2821,29 @@ static void lock_get_status(struct seq_file
> *f, struct file_lock_core *flc,
>  		seq_printf(f, "%d %02x:%02x:%lu ", pid,
>  				MAJOR(inode->i_sb->s_dev),
>  				MINOR(inode->i_sb->s_dev), inode-
> >i_ino);
> +
> +		pathbuf = __getname();

This won't work. locks_show is called under the blocked_lock_lock
spinlock, and __getname does a GFP_KERNEL allocation.

The second problem is that the procfiles that this alters (/proc/locks
and /proc/<pid>/fdinfo/*) are considered part of the kernel's ABI, so
we can't just go change the format of them.

If you're interested in getting pathnames, I suggest using the lslocks
program which resolves these pathnames in userland.

> +		if (!pathbuf)
> +			seq_printf(f, "%s ", "UNKNOWN");
> +		else {
> +			ihold(inode);
> +			dentry = d_obtain_alias(inode);
> +			if (!IS_ERR(dentry)) {
> +				path = dentry_path_raw(dentry,
> pathbuf, PATH_MAX);
> +				if (IS_ERR(path)) {
> +					strscpy(pathbuf, "UNKNOWN",
> PATH_MAX);
> +					path = pathbuf;
> +				}
> +				dput(dentry);
> +			} else {
> +				strscpy(pathbuf, "UNKNOWN",
> PATH_MAX);
> +				path = pathbuf;
> +			}
> +			seq_printf(f, "%s ", path);
> +			__putname(pathbuf);
> +		}
>  	} else {
> -		seq_printf(f, "%d <none>:0 ", pid);
> +		seq_printf(f, "%d <none>:0:<none> UNKNOWN ", pid);
>  	}
>  	if (flc->flc_flags & FL_POSIX) {
>  		if (fl->fl_end == OFFSET_MAX)

-- 
Jeff Layton <jlayton@...nel.org>