Re: [PATCH RFCv2 2/3] lib/vsprintf.c: make %pD print full path for file
From: Matthew Wilcox <willy@infradead.org>
Date: 2021-05-28 13:01:29
Also in:
linux-s390, linux-wireless, lkml, netdev
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?
quoted hunk ↗ jump to hunk
@@ -920,13 +921,25 @@ 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) { + const struct path *path; + char *p; + char full_path[256]; + 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_fast(path, full_path, sizeof(full_path)); + if (IS_ERR(p)) + return err_ptr(buf, end, p, spec); + + return string_nocheck(buf, end, p, spec); } #ifdef CONFIG_BLOCK static noinline_for_stack