All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jia He <justin.he@arm.com>
To: Petr Mladek <pmladek@suse.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	Sergey Senozhatsky <senozhatsky@chromium.org>,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	Rasmus Villemoes <linux@rasmusvillemoes.dk>,
	Jonathan Corbet <corbet@lwn.net>,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	Linus Torvalds <torvalds@linux-foundation.org>
Cc: "Peter Zijlstra (Intel)" <peterz@infradead.org>,
	Eric Biggers <ebiggers@google.com>,
	"Ahmed S. Darwish" <a.darwish@linutronix.de>,
	linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-fsdevel@vger.kernel.org,
	Matthew Wilcox <willy@infradead.org>,
	Christoph Hellwig <hch@infradead.org>,
	nd@arm.com, Jia He <justin.he@arm.com>
Subject: [PATCH v2 2/4] lib/vsprintf.c: make '%pD' print the full path of file
Date: Wed, 23 Jun 2021 13:50:09 +0800	[thread overview]
Message-ID: <20210623055011.22916-3-justin.he@arm.com> (raw)
In-Reply-To: <20210623055011.22916-1-justin.he@arm.com>

Previously, the specifier '%pD' is for printing dentry name of struct
file. It may not be perfect (by default it only prints one component.)

As suggested by Linus [1]:
> A dentry has a parent, but at the same time, a dentry really does
> inherently have "one name" (and given just the dentry pointers, you
> can't show mount-related parenthood, so in many ways the "show just
> one name" makes sense for "%pd" in ways it doesn't necessarily for
> "%pD"). But while a dentry arguably has that "one primary component",
> a _file_ is certainly not exclusively about that last component.

Hence change the behavior of '%pD' to print the full path of that file.

If someone invokes snprintf() with small but positive space,
prepend_name_with_len() moves or truncates the string partially. More
than that, kasprintf() will pass NULL @buf and @end as the parameters,
and @end - @buf can be negative in some case. Hence make it return at
the very beginning with false in these cases.

Precision is never going to be used with %p (or any of its kernel
extensions) if -Wformat is turned on.

Link: https://lore.kernel.org/lkml/CAHk-=wimsMqGdzik187YWLb-ru+iktb4MYbMQG1rnZ81dXYFVg@mail.gmail.com/ [1]
Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Jia He <justin.he@arm.com>
---
 Documentation/core-api/printk-formats.rst |  5 +--
 lib/vsprintf.c                            | 40 ++++++++++++++++++++---
 2 files changed, 39 insertions(+), 6 deletions(-)

diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst
index f063a384c7c8..3c0d0f90b171 100644
--- a/Documentation/core-api/printk-formats.rst
+++ b/Documentation/core-api/printk-formats.rst
@@ -408,12 +408,13 @@ dentry names
 ::
 
 	%pd{,2,3,4}
-	%pD{,2,3,4}
+	%pD
 
 For printing dentry name; if we race with :c:func:`d_move`, the name might
 be a mix of old and new ones, but it won't oops.  %pd dentry is a safer
 equivalent of %s dentry->d_name.name we used to use, %pd<n> prints ``n``
-last components.  %pD does the same thing for struct file.
+last components. %pD prints full file path together with mount-related
+parenthood.
 
 Passed by reference.
 
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index f0c35d9b65bf..f4494129081f 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -26,6 +26,7 @@
 #include <linux/types.h>
 #include <linux/string.h>
 #include <linux/ctype.h>
+#include <linux/dcache.h>
 #include <linux/kernel.h>
 #include <linux/kallsyms.h>
 #include <linux/math64.h>
@@ -920,13 +921,44 @@ char *dentry_name(char *buf, char *end, const struct dentry *d, struct printf_sp
 }
 
 static noinline_for_stack
-char *file_dentry_name(char *buf, char *end, const struct file *f,
+char *file_d_path_name(char *buf, char *end, const struct file *f,
 			struct printf_spec spec, const char *fmt)
 {
+	char *p;
+	const struct path *path;
+	int prepend_len, widen_len, dpath_len;
+
 	if (check_pointer(&buf, end, f, spec))
 		return buf;
 
-	return dentry_name(buf, end, f->f_path.dentry, spec, fmt);
+	path = &f->f_path;
+	if (check_pointer(&buf, end, path, spec))
+		return buf;
+
+	p = d_path_unsafe(path, buf, end - buf, &prepend_len);
+
+	/* Calculate the full d_path length, ignoring the tail '\0' */
+	dpath_len = end - buf - prepend_len - 1;
+
+	widen_len = max_t(int, dpath_len, spec.field_width);
+
+	/* Case 1: Already started past the buffer. Just forward @buf. */
+	if (buf >= end)
+		return buf + widen_len;
+
+	/*
+	 * Case 2: The entire remaining space of the buffer filled by
+	 * the truncated path. Still need to get moved right when
+	 * the filled width is greather than the full path length.
+	 */
+	if (prepend_len < 0)
+		return widen_string(buf + dpath_len, dpath_len, end, spec);
+
+	/*
+	 * Case 3: The full path is printed at the end of the buffer.
+	 * Print it at the right location in the same buffer.
+	 */
+	return string_nocheck(buf, end, p, spec);
 }
 #ifdef CONFIG_BLOCK
 static noinline_for_stack
@@ -2296,7 +2328,7 @@ early_param("no_hash_pointers", no_hash_pointers_enable);
  * - 'a[pd]' For address types [p] phys_addr_t, [d] dma_addr_t and derivatives
  *           (default assumed to be phys_addr_t, passed by reference)
  * - 'd[234]' For a dentry name (optionally 2-4 last components)
- * - 'D[234]' Same as 'd' but for a struct file
+ * - 'D' For full path name of a struct file
  * - 'g' For block_device name (gendisk + partition number)
  * - 't[RT][dt][r]' For time and date as represented by:
  *      R    struct rtc_time
@@ -2395,7 +2427,7 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr,
 	case 'C':
 		return clock(buf, end, ptr, spec, fmt);
 	case 'D':
-		return file_dentry_name(buf, end, ptr, spec, fmt);
+		return file_d_path_name(buf, end, ptr, spec, fmt);
 #ifdef CONFIG_BLOCK
 	case 'g':
 		return bdev_name(buf, end, ptr, spec, fmt);
-- 
2.17.1


  parent reply	other threads:[~2021-06-23  5:50 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-23  5:50 [PATCH v2 0/4] make '%pD' print the full path of file Jia He
2021-06-23  5:50 ` [PATCH v2 1/4] fs: introduce helper d_path_unsafe() Jia He
2021-06-23  9:10   ` Andy Shevchenko
2021-06-24  5:48     ` Justin He
2021-07-14  8:33       ` Justin He
2021-07-14  9:17         ` Andy Shevchenko
2021-06-28  5:13   ` Justin He
2021-06-28  9:06     ` Petr Mladek
2021-07-02  6:36       ` Justin He
2021-06-23  5:50 ` Jia He [this message]
2021-06-23  9:14   ` [PATCH v2 2/4] lib/vsprintf.c: make '%pD' print the full path of file Andy Shevchenko
2021-06-24  9:01   ` Petr Mladek
2021-06-24 10:49     ` Andy Shevchenko
2021-06-25  2:29     ` Justin He
2021-06-25  2:32       ` Justin He
2021-06-23  5:50 ` [PATCH v2 3/4] lib/test_printf.c: split write-beyond-buffer check in two Jia He
2021-06-23  9:16   ` Andy Shevchenko
2021-06-23  5:50 ` [PATCH v2 4/4] lib/test_printf.c: add test cases for '%pD' Jia He
2021-06-23  9:17 ` [PATCH v2 0/4] make '%pD' print the full path of file Andy Shevchenko
     [not found]   ` <AM6PR08MB43763816EBF217940F5B3E15F7139@AM6PR08MB4376.eurprd08.prod.outlook.com>
2021-07-14 14:17     ` Andy Shevchenko

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=20210623055011.22916-3-justin.he@arm.com \
    --to=justin.he@arm.com \
    --cc=a.darwish@linutronix.de \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=corbet@lwn.net \
    --cc=ebiggers@google.com \
    --cc=hch@infradead.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@rasmusvillemoes.dk \
    --cc=nd@arm.com \
    --cc=peterz@infradead.org \
    --cc=pmladek@suse.com \
    --cc=rostedt@goodmis.org \
    --cc=senozhatsky@chromium.org \
    --cc=torvalds@linux-foundation.org \
    --cc=viro@zeniv.linux.org.uk \
    --cc=willy@infradead.org \
    /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.