* [PATCH v2] vsprintf: Do not have bprintf dereference pointers
@ 2017-12-29 1:40 Steven Rostedt
2017-12-31 15:18 ` kbuild test robot
0 siblings, 1 reply; 2+ messages in thread
From: Steven Rostedt @ 2017-12-29 1:40 UTC (permalink / raw)
To: LKML; +Cc: Linus Torvalds, Thomas Gleixner, Andrew Morton, Frederic Weisbecker
From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
When trace_printk() was introduced, it was discussed that making it be as
low overhead as possible, that the processing of the format string should be
delayed until it is read. That is, a "trace_printk()" should not convert
the %d into numbers and so on, but instead, save the fmt string and all the
args in the buffer at the time of recording. When the trace_printk() data is
read, it would then parse the format string and do the conversions of the
saved arguments in the tracing buffer.
The code to perform this was added to vsprintf where vbin_printf() would
save the arguments of a specified format string in a buffer, then
bstr_printf() could be used to convert the buffer with the same format
string into the final output, as if vsprintf() was called in one go.
The issue arises when dereferenced pointers are used. The problem is that
something like %*pbl which reads a bitmask, will save the pointer to the
bitmask in the buffer. Then the reading of the buffer via bstr_printf() will
then look at the pointer to process the final output. Obviously the value of
that pointer could have changed since the time it was recorded to the time
the buffer is read. Worse yet, the bitmask could be unmapped, and the
reading of the trace buffer could actually cause a kernel oops.
Another problem is that user space tools such as perf and trace-cmd do not
have access to the contents of these pointers, and they become useless when
the tracing buffer is extracted.
Instead of having vbin_printf() simply save the pointer in the buffer for
later processing, have it perform the formatting at the time bin_printf() is
called. This will fix the issue of dereferencing pointers at a later time,
and has the extra benefit of having user space tools understand these
values.
Since perf and trace-cmd already can handle %p[sSfF] via saving kallsyms,
their pointers are saved and not processed during vbin_printf(). If they
were converted, it would break perf and trace-cmd, as they would not know
how to deal with the conversion.
Reported-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
Changes from v1:
- Have str increment by how much should be copied, not how much that
was.
- Use '\0' and not 0 for nul string terminator.
lib/vsprintf.c | 68 +++++++++++++++++++++++++++++++++++++++++++++++++++-------
1 file changed, 60 insertions(+), 8 deletions(-)
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 01c3957b2de6..d73e0c3fadf4 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -2516,14 +2516,15 @@ int vbin_printf(u32 *bin_buf, size_t size, const char *fmt, va_list args)
{
struct printf_spec spec = {0};
char *str, *end;
+ int width;
str = (char *)bin_buf;
end = (char *)(bin_buf + size);
#define save_arg(type) \
-do { \
+({ \
+ unsigned long long value; \
if (sizeof(type) == 8) { \
- unsigned long long value; \
str = PTR_ALIGN(str, sizeof(u32)); \
value = va_arg(args, unsigned long long); \
if (str + sizeof(type) <= end) { \
@@ -2531,14 +2532,14 @@ do { \
*(u32 *)(str + 4) = *((u32 *)&value + 1); \
} \
} else { \
- unsigned long value; \
str = PTR_ALIGN(str, sizeof(type)); \
value = va_arg(args, int); \
if (str + sizeof(type) <= end) \
*(typeof(type) *)str = (type)value; \
} \
str += sizeof(type); \
-} while (0)
+ value; \
+})
while (*fmt) {
int read = format_decode(fmt, &spec);
@@ -2554,7 +2555,10 @@ do { \
case FORMAT_TYPE_WIDTH:
case FORMAT_TYPE_PRECISION:
- save_arg(int);
+ width = (int)save_arg(int);
+ /* Pointers may require the width */
+ if (*fmt == 'p')
+ set_field_width(&spec, width);
break;
case FORMAT_TYPE_CHAR:
@@ -2576,7 +2580,27 @@ do { \
}
case FORMAT_TYPE_PTR:
- save_arg(void *);
+ /* Dereferenced pointers must be done now */
+ switch (*fmt) {
+ /* Dereference of functions is still OK */
+ case 'S':
+ case 's':
+ case 'F':
+ case 'f':
+ save_arg(void *);
+ break;
+ default:
+ if (!isalnum(*fmt)) {
+ save_arg(void *);
+ break;
+ }
+ str = pointer(fmt, str, end, va_arg(args, void *),
+ spec);
+ if (str + 1 < end)
+ *str++ = '\0';
+ else
+ end[-1] = '\0'; /* Must be nul terminated */
+ }
/* skip all alphanumeric pointer suffixes */
while (isalnum(*fmt))
fmt++;
@@ -2728,11 +2752,39 @@ int bstr_printf(char *buf, size_t size, const char *fmt, const u32 *bin_buf)
break;
}
- case FORMAT_TYPE_PTR:
- str = pointer(fmt, str, end, get_arg(void *), spec);
+ case FORMAT_TYPE_PTR: {
+ bool process = false;
+ int copy, len;
+ /* Non function dereferences were already done */
+ switch (*fmt) {
+ case 'S':
+ case 's':
+ case 'F':
+ case 'f':
+ process = true;
+ break;
+ default:
+ if (!isalnum(*fmt)) {
+ process = true;
+ break;
+ }
+ /* Pointer dereference was already processed */
+ if (str < end) {
+ len = copy = strlen(args);
+ if (copy > end - str)
+ copy = end - str;
+ memcpy(str, args, copy);
+ str += len;
+ args += len;
+ }
+ }
+ if (process)
+ str = pointer(fmt, str, end, get_arg(void *), spec);
+
while (isalnum(*fmt))
fmt++;
break;
+ }
case FORMAT_TYPE_PERCENT_CHAR:
if (str < end)
--
2.13.6
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH v2] vsprintf: Do not have bprintf dereference pointers
2017-12-29 1:40 [PATCH v2] vsprintf: Do not have bprintf dereference pointers Steven Rostedt
@ 2017-12-31 15:18 ` kbuild test robot
0 siblings, 0 replies; 2+ messages in thread
From: kbuild test robot @ 2017-12-31 15:18 UTC (permalink / raw)
To: Steven Rostedt
Cc: kbuild-all, LKML, Linus Torvalds, Thomas Gleixner, Andrew Morton,
Frederic Weisbecker
[-- Attachment #1: Type: text/plain, Size: 13420 bytes --]
Hi Steven,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on linus/master]
[also build test WARNING on v4.15-rc5 next-20171222]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Steven-Rostedt/vsprintf-Do-not-have-bprintf-dereference-pointers/20171231-223129
config: i386-randconfig-x072-201753 (attached as .config)
compiler: gcc-7 (Debian 7.2.0-12) 7.2.1 20171025
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All warnings (new ones prefixed by >>):
lib/vsprintf.c: In function 'vbin_printf':
>> lib/vsprintf.c:2538:27: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
*(typeof(type) *)str = (type)value; \
^
>> lib/vsprintf.c:2590:5: note: in expansion of macro 'save_arg'
save_arg(void *);
^~~~~~~~
>> lib/vsprintf.c:2538:27: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
*(typeof(type) *)str = (type)value; \
^
lib/vsprintf.c:2594:6: note: in expansion of macro 'save_arg'
save_arg(void *);
^~~~~~~~
vim +2538 lib/vsprintf.c
^1da177e4c Linus Torvalds 2005-04-16 2490
4370aa4aa7 Lai Jiangshan 2009-03-06 2491 #ifdef CONFIG_BINARY_PRINTF
4370aa4aa7 Lai Jiangshan 2009-03-06 2492 /*
4370aa4aa7 Lai Jiangshan 2009-03-06 2493 * bprintf service:
4370aa4aa7 Lai Jiangshan 2009-03-06 2494 * vbin_printf() - VA arguments to binary data
4370aa4aa7 Lai Jiangshan 2009-03-06 2495 * bstr_printf() - Binary data to text string
4370aa4aa7 Lai Jiangshan 2009-03-06 2496 */
4370aa4aa7 Lai Jiangshan 2009-03-06 2497
4370aa4aa7 Lai Jiangshan 2009-03-06 2498 /**
4370aa4aa7 Lai Jiangshan 2009-03-06 2499 * vbin_printf - Parse a format string and place args' binary value in a buffer
4370aa4aa7 Lai Jiangshan 2009-03-06 2500 * @bin_buf: The buffer to place args' binary value
4370aa4aa7 Lai Jiangshan 2009-03-06 2501 * @size: The size of the buffer(by words(32bits), not characters)
4370aa4aa7 Lai Jiangshan 2009-03-06 2502 * @fmt: The format string to use
4370aa4aa7 Lai Jiangshan 2009-03-06 2503 * @args: Arguments for the format string
4370aa4aa7 Lai Jiangshan 2009-03-06 2504 *
4370aa4aa7 Lai Jiangshan 2009-03-06 2505 * The format follows C99 vsnprintf, except %n is ignored, and its argument
da3dae54e4 Masanari Iida 2014-09-09 2506 * is skipped.
4370aa4aa7 Lai Jiangshan 2009-03-06 2507 *
4370aa4aa7 Lai Jiangshan 2009-03-06 2508 * The return value is the number of words(32bits) which would be generated for
4370aa4aa7 Lai Jiangshan 2009-03-06 2509 * the given input.
4370aa4aa7 Lai Jiangshan 2009-03-06 2510 *
4370aa4aa7 Lai Jiangshan 2009-03-06 2511 * NOTE:
4370aa4aa7 Lai Jiangshan 2009-03-06 2512 * If the return value is greater than @size, the resulting bin_buf is NOT
4370aa4aa7 Lai Jiangshan 2009-03-06 2513 * valid for bstr_printf().
4370aa4aa7 Lai Jiangshan 2009-03-06 2514 */
4370aa4aa7 Lai Jiangshan 2009-03-06 2515 int vbin_printf(u32 *bin_buf, size_t size, const char *fmt, va_list args)
4370aa4aa7 Lai Jiangshan 2009-03-06 2516 {
fef20d9c13 Frederic Weisbecker 2009-03-06 2517 struct printf_spec spec = {0};
4370aa4aa7 Lai Jiangshan 2009-03-06 2518 char *str, *end;
168dccad05 Steven Rostedt (VMware 2017-12-28 2519) int width;
4370aa4aa7 Lai Jiangshan 2009-03-06 2520
4370aa4aa7 Lai Jiangshan 2009-03-06 2521 str = (char *)bin_buf;
4370aa4aa7 Lai Jiangshan 2009-03-06 2522 end = (char *)(bin_buf + size);
4370aa4aa7 Lai Jiangshan 2009-03-06 2523
4370aa4aa7 Lai Jiangshan 2009-03-06 2524 #define save_arg(type) \
168dccad05 Steven Rostedt (VMware 2017-12-28 2525) ({ \
4370aa4aa7 Lai Jiangshan 2009-03-06 2526 unsigned long long value; \
168dccad05 Steven Rostedt (VMware 2017-12-28 2527) if (sizeof(type) == 8) { \
4370aa4aa7 Lai Jiangshan 2009-03-06 2528 str = PTR_ALIGN(str, sizeof(u32)); \
4370aa4aa7 Lai Jiangshan 2009-03-06 2529 value = va_arg(args, unsigned long long); \
4370aa4aa7 Lai Jiangshan 2009-03-06 2530 if (str + sizeof(type) <= end) { \
4370aa4aa7 Lai Jiangshan 2009-03-06 2531 *(u32 *)str = *(u32 *)&value; \
4370aa4aa7 Lai Jiangshan 2009-03-06 2532 *(u32 *)(str + 4) = *((u32 *)&value + 1); \
4370aa4aa7 Lai Jiangshan 2009-03-06 2533 } \
4370aa4aa7 Lai Jiangshan 2009-03-06 2534 } else { \
4370aa4aa7 Lai Jiangshan 2009-03-06 2535 str = PTR_ALIGN(str, sizeof(type)); \
4370aa4aa7 Lai Jiangshan 2009-03-06 2536 value = va_arg(args, int); \
4370aa4aa7 Lai Jiangshan 2009-03-06 2537 if (str + sizeof(type) <= end) \
4370aa4aa7 Lai Jiangshan 2009-03-06 @2538 *(typeof(type) *)str = (type)value; \
4370aa4aa7 Lai Jiangshan 2009-03-06 2539 } \
4370aa4aa7 Lai Jiangshan 2009-03-06 2540 str += sizeof(type); \
168dccad05 Steven Rostedt (VMware 2017-12-28 2541) value; \
168dccad05 Steven Rostedt (VMware 2017-12-28 2542) })
4370aa4aa7 Lai Jiangshan 2009-03-06 2543
fef20d9c13 Frederic Weisbecker 2009-03-06 2544 while (*fmt) {
d4be151b21 André Goddard Rosa 2009-12-14 2545 int read = format_decode(fmt, &spec);
4370aa4aa7 Lai Jiangshan 2009-03-06 2546
fef20d9c13 Frederic Weisbecker 2009-03-06 2547 fmt += read;
4370aa4aa7 Lai Jiangshan 2009-03-06 2548
fef20d9c13 Frederic Weisbecker 2009-03-06 2549 switch (spec.type) {
fef20d9c13 Frederic Weisbecker 2009-03-06 2550 case FORMAT_TYPE_NONE:
d4be151b21 André Goddard Rosa 2009-12-14 2551 case FORMAT_TYPE_PERCENT_CHAR:
fef20d9c13 Frederic Weisbecker 2009-03-06 2552 break;
b006f19b05 Rasmus Villemoes 2015-11-06 2553 case FORMAT_TYPE_INVALID:
b006f19b05 Rasmus Villemoes 2015-11-06 2554 goto out;
4370aa4aa7 Lai Jiangshan 2009-03-06 2555
ed681a91ab Vegard Nossum 2009-03-14 2556 case FORMAT_TYPE_WIDTH:
fef20d9c13 Frederic Weisbecker 2009-03-06 2557 case FORMAT_TYPE_PRECISION:
168dccad05 Steven Rostedt (VMware 2017-12-28 2558) width = (int)save_arg(int);
168dccad05 Steven Rostedt (VMware 2017-12-28 2559) /* Pointers may require the width */
168dccad05 Steven Rostedt (VMware 2017-12-28 2560) if (*fmt == 'p')
168dccad05 Steven Rostedt (VMware 2017-12-28 2561) set_field_width(&spec, width);
fef20d9c13 Frederic Weisbecker 2009-03-06 2562 break;
4370aa4aa7 Lai Jiangshan 2009-03-06 2563
fef20d9c13 Frederic Weisbecker 2009-03-06 2564 case FORMAT_TYPE_CHAR:
4370aa4aa7 Lai Jiangshan 2009-03-06 2565 save_arg(char);
fef20d9c13 Frederic Weisbecker 2009-03-06 2566 break;
fef20d9c13 Frederic Weisbecker 2009-03-06 2567
fef20d9c13 Frederic Weisbecker 2009-03-06 2568 case FORMAT_TYPE_STR: {
4370aa4aa7 Lai Jiangshan 2009-03-06 2569 const char *save_str = va_arg(args, char *);
4370aa4aa7 Lai Jiangshan 2009-03-06 2570 size_t len;
6c35663411 André Goddard Rosa 2009-12-14 2571
4370aa4aa7 Lai Jiangshan 2009-03-06 2572 if ((unsigned long)save_str > (unsigned long)-PAGE_SIZE
4370aa4aa7 Lai Jiangshan 2009-03-06 2573 || (unsigned long)save_str < PAGE_SIZE)
0f4f81dce9 André Goddard Rosa 2009-12-14 2574 save_str = "(null)";
6c35663411 André Goddard Rosa 2009-12-14 2575 len = strlen(save_str) + 1;
6c35663411 André Goddard Rosa 2009-12-14 2576 if (str + len < end)
6c35663411 André Goddard Rosa 2009-12-14 2577 memcpy(str, save_str, len);
6c35663411 André Goddard Rosa 2009-12-14 2578 str += len;
fef20d9c13 Frederic Weisbecker 2009-03-06 2579 break;
4370aa4aa7 Lai Jiangshan 2009-03-06 2580 }
fef20d9c13 Frederic Weisbecker 2009-03-06 2581
fef20d9c13 Frederic Weisbecker 2009-03-06 2582 case FORMAT_TYPE_PTR:
168dccad05 Steven Rostedt (VMware 2017-12-28 2583) /* Dereferenced pointers must be done now */
168dccad05 Steven Rostedt (VMware 2017-12-28 2584) switch (*fmt) {
168dccad05 Steven Rostedt (VMware 2017-12-28 2585) /* Dereference of functions is still OK */
168dccad05 Steven Rostedt (VMware 2017-12-28 2586) case 'S':
168dccad05 Steven Rostedt (VMware 2017-12-28 2587) case 's':
168dccad05 Steven Rostedt (VMware 2017-12-28 2588) case 'F':
168dccad05 Steven Rostedt (VMware 2017-12-28 2589) case 'f':
4370aa4aa7 Lai Jiangshan 2009-03-06 @2590 save_arg(void *);
168dccad05 Steven Rostedt (VMware 2017-12-28 2591) break;
168dccad05 Steven Rostedt (VMware 2017-12-28 2592) default:
168dccad05 Steven Rostedt (VMware 2017-12-28 2593) if (!isalnum(*fmt)) {
168dccad05 Steven Rostedt (VMware 2017-12-28 2594) save_arg(void *);
168dccad05 Steven Rostedt (VMware 2017-12-28 2595) break;
168dccad05 Steven Rostedt (VMware 2017-12-28 2596) }
168dccad05 Steven Rostedt (VMware 2017-12-28 2597) str = pointer(fmt, str, end, va_arg(args, void *),
168dccad05 Steven Rostedt (VMware 2017-12-28 2598) spec);
168dccad05 Steven Rostedt (VMware 2017-12-28 2599) if (str + 1 < end)
168dccad05 Steven Rostedt (VMware 2017-12-28 2600) *str++ = '\0';
168dccad05 Steven Rostedt (VMware 2017-12-28 2601) else
168dccad05 Steven Rostedt (VMware 2017-12-28 2602) end[-1] = '\0'; /* Must be nul terminated */
168dccad05 Steven Rostedt (VMware 2017-12-28 2603) }
4370aa4aa7 Lai Jiangshan 2009-03-06 2604 /* skip all alphanumeric pointer suffixes */
fef20d9c13 Frederic Weisbecker 2009-03-06 2605 while (isalnum(*fmt))
4370aa4aa7 Lai Jiangshan 2009-03-06 2606 fmt++;
fef20d9c13 Frederic Weisbecker 2009-03-06 2607 break;
fef20d9c13 Frederic Weisbecker 2009-03-06 2608
fef20d9c13 Frederic Weisbecker 2009-03-06 2609 default:
fef20d9c13 Frederic Weisbecker 2009-03-06 2610 switch (spec.type) {
fef20d9c13 Frederic Weisbecker 2009-03-06 2611
fef20d9c13 Frederic Weisbecker 2009-03-06 2612 case FORMAT_TYPE_LONG_LONG:
4370aa4aa7 Lai Jiangshan 2009-03-06 2613 save_arg(long long);
fef20d9c13 Frederic Weisbecker 2009-03-06 2614 break;
fef20d9c13 Frederic Weisbecker 2009-03-06 2615 case FORMAT_TYPE_ULONG:
fef20d9c13 Frederic Weisbecker 2009-03-06 2616 case FORMAT_TYPE_LONG:
4370aa4aa7 Lai Jiangshan 2009-03-06 2617 save_arg(unsigned long);
fef20d9c13 Frederic Weisbecker 2009-03-06 2618 break;
fef20d9c13 Frederic Weisbecker 2009-03-06 2619 case FORMAT_TYPE_SIZE_T:
4370aa4aa7 Lai Jiangshan 2009-03-06 2620 save_arg(size_t);
fef20d9c13 Frederic Weisbecker 2009-03-06 2621 break;
fef20d9c13 Frederic Weisbecker 2009-03-06 2622 case FORMAT_TYPE_PTRDIFF:
4370aa4aa7 Lai Jiangshan 2009-03-06 2623 save_arg(ptrdiff_t);
fef20d9c13 Frederic Weisbecker 2009-03-06 2624 break;
a4e94ef0dd Zhaolei 2009-03-27 2625 case FORMAT_TYPE_UBYTE:
a4e94ef0dd Zhaolei 2009-03-27 2626 case FORMAT_TYPE_BYTE:
a4e94ef0dd Zhaolei 2009-03-27 2627 save_arg(char);
a4e94ef0dd Zhaolei 2009-03-27 2628 break;
fef20d9c13 Frederic Weisbecker 2009-03-06 2629 case FORMAT_TYPE_USHORT:
fef20d9c13 Frederic Weisbecker 2009-03-06 2630 case FORMAT_TYPE_SHORT:
4370aa4aa7 Lai Jiangshan 2009-03-06 2631 save_arg(short);
fef20d9c13 Frederic Weisbecker 2009-03-06 2632 break;
4370aa4aa7 Lai Jiangshan 2009-03-06 2633 default:
fef20d9c13 Frederic Weisbecker 2009-03-06 2634 save_arg(int);
fef20d9c13 Frederic Weisbecker 2009-03-06 2635 }
4370aa4aa7 Lai Jiangshan 2009-03-06 2636 }
4370aa4aa7 Lai Jiangshan 2009-03-06 2637 }
fef20d9c13 Frederic Weisbecker 2009-03-06 2638
b006f19b05 Rasmus Villemoes 2015-11-06 2639 out:
7b9186f5eb André Goddard Rosa 2009-12-14 2640 return (u32 *)(PTR_ALIGN(str, sizeof(u32))) - bin_buf;
fef20d9c13 Frederic Weisbecker 2009-03-06 2641 #undef save_arg
4370aa4aa7 Lai Jiangshan 2009-03-06 2642 }
4370aa4aa7 Lai Jiangshan 2009-03-06 2643 EXPORT_SYMBOL_GPL(vbin_printf);
4370aa4aa7 Lai Jiangshan 2009-03-06 2644
:::::: The code at line 2538 was first introduced by commit
:::::: 4370aa4aa75391a5e2e06bccb0919109f725ed8e vsprintf: add binary printf
:::::: TO: Lai Jiangshan <laijs@cn.fujitsu.com>
:::::: CC: Ingo Molnar <mingo@elte.hu>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 30060 bytes --]
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2017-12-31 15:18 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-12-29 1:40 [PATCH v2] vsprintf: Do not have bprintf dereference pointers Steven Rostedt
2017-12-31 15:18 ` kbuild test robot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®