Hi Matthew

-----Original Message-----
From: Matthew Wilcox <willy@infradead.org>
Sent: Friday, May 28, 2021 10:53 PM
To: Justin He <redacted>
Cc: Linus Torvalds <torvalds@linux-foundation.org>; Petr Mladek
[off-list ref]; Steven Rostedt [off-list ref]; Sergey
Senozhatsky [off-list ref]; Andy Shevchenko
[off-list ref]; Rasmus Villemoes
[off-list ref]; Jonathan Corbet [off-list ref]; Alexander
Viro [off-list ref]; Luca Coelho [off-list ref];
Kalle Valo [off-list ref]; David S. Miller [off-list ref];
Jakub Kicinski [off-list ref]; Heiko Carstens [off-list ref];
Vasily Gorbik [off-list ref]; Christian Borntraeger
[off-list ref]; Johannes Berg [off-list ref]; linux-
doc@vger.kernel.org; linux-kernel@vger.kernel.org; linux-
wireless@vger.kernel.org; netdev@vger.kernel.org; linux-
s390@vger.kernel.org
Subject: Re: [PATCH RFCv2 2/3] lib/vsprintf.c: make %pD print full path
for file

On Fri, May 28, 2021 at 02:22:01PM +0000, Justin He wrote:

quoted

quoted

On Fri, May 28, 2021 at 07:39:50PM +0800, Jia He wrote:

quoted

We have '%pD' for printing a filename. It may not be perfect (by
default it only prints one component.)

As suggested by Linus at [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 "file_dentry_name()" simply shouldn't use "dentry_name()" at

all.

quoted

quoted

quoted

Despite that shared code origin, and despite that similar letter
choice (lower-vs-upper case), a dentry and a file really are very
different from a name standpoint.

Here stack space is preferred for file_d_path_name() because it is
much safer. The stack size 256 is a compromise between stack

overflow

quoted

quoted

quoted

and too short full path.

How is it "safer"?  You already have a buffer passed from the caller.
Are you saying that d_path_fast() might overrun a really small buffer
but won't overrun a 256 byte buffer?

No, it won't overrun a 256 byte buf. When the full path size is larger

than 256, the p->len is < 0 in prepend_name, and this overrun will be

quoted

dectected in extract_string() with "-ENAMETOOLONG".

Each printk contains 2 vsnprintf. vsnprintf() returns the required size

after formatting the string.

quoted

1. vprintk_store() will invoke 1st vsnprintf() will 8 bytes space to get

the reserve_size. In this case, the _buf_ could be less than _end_ by
design.

quoted

2. Then it invokes 2nd printk_sprint()->vscnprintf()->vsnprintf() to

really fill the space.

I think you need to explain _that_ in the commit log, not make some
nebulous claim of "safer".

Okay

quoted

If we choose the stack space, it can meet above 2 cases.

If we pass the parameter like:
p = d_path_fast(path, buf, end - buf);
We need to handle the complicated logic in prepend_name()
I have tried this way in local test, the code logic is very complicated
and not so graceful.
e.g. I need to firstly go through the loop and get the full path size of
that file. And then return reserved_size for that 1st vsnprintf

I'm not sure why it's so complicated.  p->len records how many bytes
are needed for the entire path; can't you just return -p->len ?

prepend_name() will return at the beginning if p->len is <0 in this case,
we can't even get the correct full path size if keep __prepend_path unchanged.
We need another new helper __prepend_path_size() to get the full path size
regardless of the negative value p->len.

More than that, even the 1st vsnprintf could have _end_ > _buf_ in some case:
What if printk("%pD", filp) ? The 1st vsnprintf has positive (end-buf).

This make things more complicated.

Hope I have described it clearly as above.

--
Cheers,
Justin (Jia He)


IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.