Messages in this thread |  | | Date | Mon, 9 Apr 2018 15:50:28 +0200 | From | Petr Mladek <> | Subject | Re: [PATCH v4 4/9] vsprintf: Consolidate handling of unknown pointer specifiers |
| |
On Sat 2018-04-07 17:26:40, Andy Shevchenko wrote: > On Wed, 2018-04-04 at 10:58 +0200, Petr Mladek wrote: > > There are few printk formats that make sense only with two or more > > specifiers. Also some specifiers make sense only when a kernel feature > > is enabled. > > > > The handling of unknown specifiers is strange, inconsistent, and > > even leaking the address. For example, netdev_bits() prints the > > non-hashed pointer value or clock() prints "(null)". > > > > The best solution seems to be in flags_string(). It does not print any > > misleading value. Instead it calls WARN_ONCE() describing the unknown > > specifier. Therefore it clearly shows the problem and helps to find > > it. > > > > Note that WARN_ONCE() used to cause recursive printk(). But it is safe > > now because vscnprintf() is called in printk_safe context from > > vprintk_emit(). > > > > > - if (!IS_ENABLED(CONFIG_HAVE_CLK) || !clk) > > + if (!IS_ENABLED(CONFIG_HAVE_CLK)) { > > + WARN_ONCE(1, "Unsupported pointer format specifier: > > %%pC\n"); > > + return buf; > > + } > > + > > + if (!clk) > > return string(buf, end, NULL, spec); > > This change collides with my patch series. Can you elaborate what your > thoughts are about my patches? Are you going incorporate them to your > series? Should I send them independently?
Good question. I think that the best solution will be that I go over your patchset and just add all valid ones into printk.git for-4.18. Then I will base v5 of this patchset on top of it.
I should have done this earlier. But I did not expect that long way for the access-check stuff. We originally planned to do the access check first, see https://lkml.kernel.org/r/1520000254.10722.389.camel@linux.intel.com But the access check patchset still need some love, so it makes sense to switch the order.
Best Regards, Petr
|  |