The panic() function uses a static buffer to format its arguments into, simply
to emit the result via printk("%s", buf). This buffer is not large enough for
some existing users in Xen. e.g.:
(XEN) ****************************************
(XEN) Panic on CPU 0:
(XEN) Invalid device tree blob at physical address 0x46a00000.
(XEN) The DTB must be 8-byte aligned and must not exceed 2 MB in size.
(XEN)
(XEN) Plea****************************************
The remainder of this particular message is 'e check your bootloader.', but
has been inherited by RISC-V from ARM.
It is also pointless double buffering. Implement vprintk() beside printk(),
and use it directly rather than rendering into a local buffer, removing it as
one source of message limitation.
This marginally simplifies panic(), and drops a global used-once buffer.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Julien Grall <julien@xen.org>
CC: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
CC: Bertrand Marquis <bertrand.marquis@arm.com>
CC: Michal Orzel <michal.orzel@amd.com>
CC: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
CC: Oleksii Kurochko <oleksii.kurochko@gmail.com>
This taken from a security series (hence partially reviewed already), and
thought it only applied to a forthcoming selftest, but then Ayan posted a
truncated example to Matrix.
Therefore this needs backporting, and might be wanted for 4.20 at this point.
It's low risk and easy to test.
---
xen/drivers/char/console.c | 21 +++++++++++++--------
xen/include/xen/lib.h | 2 ++
2 files changed, 15 insertions(+), 8 deletions(-)
diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index 7da8c5296f3b..2926c97df9a4 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -962,11 +962,17 @@ static void vprintk_common(const char *prefix, const char *fmt, va_list args)
local_irq_restore(flags);
}
+void vprintk(const char *fmt, va_list args)
+{
+ vprintk_common("(XEN) ", fmt, args);
+}
+
void printk(const char *fmt, ...)
{
va_list args;
+
va_start(args, fmt);
- vprintk_common("(XEN) ", fmt, args);
+ vprintk(fmt, args);
va_end(args);
}
@@ -1268,23 +1274,22 @@ void panic(const char *fmt, ...)
va_list args;
unsigned long flags;
static DEFINE_SPINLOCK(lock);
- static char buf[128];
spin_debug_disable();
spinlock_profile_printall('\0');
debugtrace_dump();
- /* Protects buf[] and ensure multi-line message prints atomically. */
+ /* Ensure multi-line message prints atomically. */
spin_lock_irqsave(&lock, flags);
- va_start(args, fmt);
- (void)vsnprintf(buf, sizeof(buf), fmt, args);
- va_end(args);
-
console_start_sync();
printk("\n****************************************\n");
printk("Panic on CPU %d:\n", smp_processor_id());
- printk("%s", buf);
+
+ va_start(args, fmt);
+ vprintk(fmt, args);
+ va_end(args);
+
printk("****************************************\n\n");
if ( opt_noreboot )
printk("Manual reset required ('noreboot' specified)\n");
diff --git a/xen/include/xen/lib.h b/xen/include/xen/lib.h
index 81b722ea3e80..130f96791f75 100644
--- a/xen/include/xen/lib.h
+++ b/xen/include/xen/lib.h
@@ -60,6 +60,8 @@ debugtrace_printk(const char *fmt, ...) {}
extern void printk(const char *fmt, ...)
__attribute__ ((format (printf, 1, 2), cold));
+void vprintk(const char *fmt, va_list args)
+ __attribute__ ((format (printf, 1, 0), cold));
#define printk_once(fmt, args...) \
({ \
--
2.39.5
On 2/17/25 8:44 PM, Andrew Cooper wrote:
> The panic() function uses a static buffer to format its arguments into, simply
> to emit the result via printk("%s", buf). This buffer is not large enough for
> some existing users in Xen. e.g.:
>
> (XEN) ****************************************
> (XEN) Panic on CPU 0:
> (XEN) Invalid device tree blob at physical address 0x46a00000.
> (XEN) The DTB must be 8-byte aligned and must not exceed 2 MB in size.
> (XEN)
> (XEN) Plea****************************************
>
> The remainder of this particular message is 'e check your bootloader.', but
> has been inherited by RISC-V from ARM.
>
> It is also pointless double buffering. Implement vprintk() beside printk(),
> and use it directly rather than rendering into a local buffer, removing it as
> one source of message limitation.
>
> This marginally simplifies panic(), and drops a global used-once buffer.
>
> Signed-off-by: Andrew Cooper<andrew.cooper3@citrix.com>
> Reviewed-by: Jan Beulich<jbeulich@suse.com>
Release-Acked-By: Oleksii Kurochko<oleksii.kurochko@gmail.com>
Thanks.
~ Oleksii
> ---
> CC: Jan Beulich<JBeulich@suse.com>
> CC: Roger Pau Monné<roger.pau@citrix.com>
> CC: Stefano Stabellini<sstabellini@kernel.org>
> CC: Julien Grall<julien@xen.org>
> CC: Volodymyr Babchuk<Volodymyr_Babchuk@epam.com>
> CC: Bertrand Marquis<bertrand.marquis@arm.com>
> CC: Michal Orzel<michal.orzel@amd.com>
> CC: Ayan Kumar Halder<ayan.kumar.halder@amd.com>
> CC: Oleksii Kurochko<oleksii.kurochko@gmail.com>
>
> This taken from a security series (hence partially reviewed already), and
> thought it only applied to a forthcoming selftest, but then Ayan posted a
> truncated example to Matrix.
>
> Therefore this needs backporting, and might be wanted for 4.20 at this point.
> It's low risk and easy to test.
> ---
> xen/drivers/char/console.c | 21 +++++++++++++--------
> xen/include/xen/lib.h | 2 ++
> 2 files changed, 15 insertions(+), 8 deletions(-)
>
> diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
> index 7da8c5296f3b..2926c97df9a4 100644
> --- a/xen/drivers/char/console.c
> +++ b/xen/drivers/char/console.c
> @@ -962,11 +962,17 @@ static void vprintk_common(const char *prefix, const char *fmt, va_list args)
> local_irq_restore(flags);
> }
>
> +void vprintk(const char *fmt, va_list args)
> +{
> + vprintk_common("(XEN) ", fmt, args);
> +}
> +
> void printk(const char *fmt, ...)
> {
> va_list args;
> +
> va_start(args, fmt);
> - vprintk_common("(XEN)", fmt, args); + vprintk(fmt, args); va_end(args); } @@ -1268,23
> +1274,22 @@ void panic(const char *fmt, ...) va_list args; unsigned
> long flags; static DEFINE_SPINLOCK(lock); - static char buf[128];
> spin_debug_disable(); spinlock_profile_printall('\0');
> debugtrace_dump(); - /* Protects buf[] and ensure multi-line message
> prints atomically. */ + /* Ensure multi-line message prints
> atomically. */ spin_lock_irqsave(&lock, flags); - va_start(args, fmt);
> - (void)vsnprintf(buf, sizeof(buf), fmt, args); - va_end(args); -
> console_start_sync(); printk("\n****************************************\n");
> printk("Panic on CPU %d:\n", smp_processor_id());
> - printk("%s", buf);
> +
> + va_start(args, fmt);
> + vprintk(fmt, args);
> + va_end(args);
> +
> printk("****************************************\n\n");
> if ( opt_noreboot )
> printk("Manual reset required ('noreboot' specified)\n");
> diff --git a/xen/include/xen/lib.h b/xen/include/xen/lib.h
> index 81b722ea3e80..130f96791f75 100644
> --- a/xen/include/xen/lib.h
> +++ b/xen/include/xen/lib.h
> @@ -60,6 +60,8 @@ debugtrace_printk(const char *fmt, ...) {}
>
> extern void printk(const char *fmt, ...)
> __attribute__ ((format (printf, 1, 2), cold));
> +void vprintk(const char *fmt, va_list args)
> + __attribute__ ((format (printf, 1, 0), cold));
>
> #define printk_once(fmt, args...) \
> ({ \
© 2016 - 2025 Red Hat, Inc.