linux-kernel - RE: [PATCH RFCv2 2/3] lib/vsprintf.c: make %pD print full path for file

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <AM6PR08MB437691E7314C6B774EFED4BDF7229@AM6PR08MB4376.eurprd08.prod.outlook.com>
Date:   Fri, 28 May 2021 14:22:01 +0000
From:   Justin He <Justin.He@....com>
To:     Matthew Wilcox <willy@...radead.org>
CC:     Linus Torvalds <torvalds@...ux-foundation.org>,
        Petr Mladek <pmladek@...e.com>,
        Steven Rostedt <rostedt@...dmis.org>,
        Sergey Senozhatsky <senozhatsky@...omium.org>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Rasmus Villemoes <linux@...musvillemoes.dk>,
        Jonathan Corbet <corbet@....net>,
        Alexander Viro <viro@...iv.linux.org.uk>,
        Luca Coelho <luciano.coelho@...el.com>,
        Kalle Valo <kvalo@...eaurora.org>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Heiko Carstens <hca@...ux.ibm.com>,
        Vasily Gorbik <gor@...ux.ibm.com>,
        Christian Borntraeger <borntraeger@...ibm.com>,
        Johannes Berg <johannes.berg@...el.com>,
        "linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-wireless@...r.kernel.org" <linux-wireless@...r.kernel.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-s390@...r.kernel.org" <linux-s390@...r.kernel.org>
Subject: RE: [PATCH RFCv2 2/3] lib/vsprintf.c: make %pD print full path for
 file


Hi Matthew
> -----Original Message-----
> From: Matthew Wilcox <willy@...radead.org>
> Sent: Friday, May 28, 2021 9:00 PM
> To: Justin He <Justin.He@....com>
> Cc: Linus Torvalds <torvalds@...ux-foundation.org>; Petr Mladek
> <pmladek@...e.com>; Steven Rostedt <rostedt@...dmis.org>; Sergey
> Senozhatsky <senozhatsky@...omium.org>; Andy Shevchenko
> <andriy.shevchenko@...ux.intel.com>; Rasmus Villemoes
> <linux@...musvillemoes.dk>; Jonathan Corbet <corbet@....net>; Alexander
> Viro <viro@...iv.linux.org.uk>; Luca Coelho <luciano.coelho@...el.com>;
> Kalle Valo <kvalo@...eaurora.org>; David S. Miller <davem@...emloft.net>;
> Jakub Kicinski <kuba@...nel.org>; Heiko Carstens <hca@...ux.ibm.com>;
> Vasily Gorbik <gor@...ux.ibm.com>; Christian Borntraeger
> <borntraeger@...ibm.com>; Johannes Berg <johannes.berg@...el.com>; linux-
> doc@...r.kernel.org; linux-kernel@...r.kernel.org; linux-
> wireless@...r.kernel.org; netdev@...r.kernel.org; linux-
> s390@...r.kernel.org
> Subject: Re: [PATCH RFCv2 2/3] lib/vsprintf.c: make %pD print full path
> for file
>
> On Fri, May 28, 2021 at 07:39:50PM +0800, Jia He wrote:
> > 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.
> > 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
> > 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
dectected in extract_string() with "-ENAMETOOLONG".

Each printk contains 2 vsnprintf. vsnprintf() returns the required size after formatting the string.
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.
2. Then it invokes 2nd printk_sprint()->vscnprintf()->vsnprintf() to really fill the space.

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

Thanks for any suggestion

--
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.