Re: [PATCH 02/13] hvc: Fix use of uninitialized array in udbg_hvc_putc
From: Michael Ellerman <mpe@ellerman.id.au>
Date: 2023-12-21 12:09:52
Also in:
linux-mm, lkml
Subsystem:
hypervisor virtual console driver, linux for powerpc (32-bit and 64-bit), the rest, tty layer and serial drivers · Maintainers:
Madhavan Srinivasan, Michael Ellerman, Linus Torvalds, Greg Kroah-Hartman, Jiri Slaby
Christophe Leroy [off-list ref] writes:
Le 14/12/2023 à 06:55, Nicholas Miehlbradt a écrit :quoted
All elements of bounce_buffer are eventually read and passed to the hypervisor so it should probably be fully initialized.should or shall ?quoted
Signed-off-by: Nicholas Miehlbradt <redacted>Should be a Fixed: tag ?quoted
--- drivers/tty/hvc/hvc_vio.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)diff --git a/drivers/tty/hvc/hvc_vio.c b/drivers/tty/hvc/hvc_vio.c index 736b230f5ec0..1e88bfcdde20 100644 --- a/drivers/tty/hvc/hvc_vio.c +++ b/drivers/tty/hvc/hvc_vio.c@@ -227,7 +227,7 @@ static const struct hv_ops hvterm_hvsi_ops = { static void udbg_hvc_putc(char c) { int count = -1; - unsigned char bounce_buffer[16]; + unsigned char bounce_buffer[16] = { 0 };Why 16 while we have a count of 1 in the call to hvterm_raw_put_chars() ?
Because hvterm_raw_put_chars() calls hvc_put_chars() which requires a 16 byte buffer, because it passes the buffer directly to firmware which expects a 16 byte buffer. It's a pretty horrible calling convention, but I guess it's to avoid needing another bounce buffer inside hvc_put_chars(). We should probably do the change below, to at least document the interface better. cheers
diff --git a/arch/powerpc/include/asm/hvconsole.h b/arch/powerpc/include/asm/hvconsole.h
index ccb2034506f0..0ee7ed019e23 100644
--- a/arch/powerpc/include/asm/hvconsole.h
+++ b/arch/powerpc/include/asm/hvconsole.h@@ -22,7 +22,7 @@ * parm is included to conform to put_chars() function pointer template */ extern int hvc_get_chars(uint32_t vtermno, char *buf, int count); -extern int hvc_put_chars(uint32_t vtermno, const char *buf, int count); +extern int hvc_put_chars(uint32_t vtermno, const char buf[16], int count); /* Provided by HVC VIO */ void hvc_vio_init_early(void);
diff --git a/arch/powerpc/platforms/pseries/hvconsole.c b/arch/powerpc/platforms/pseries/hvconsole.c
index 1ac52963e08b..c40a82e49d59 100644
--- a/arch/powerpc/platforms/pseries/hvconsole.c
+++ b/arch/powerpc/platforms/pseries/hvconsole.c@@ -52,7 +52,7 @@ EXPORT_SYMBOL(hvc_get_chars); * firmware. Must be at least 16 bytes, even if count is less than 16. * @count: Send this number of characters. */ -int hvc_put_chars(uint32_t vtermno, const char *buf, int count) +int hvc_put_chars(uint32_t vtermno, const char buf[16], int count) { unsigned long *lbuf = (unsigned long *) buf; long ret;
diff --git a/drivers/tty/hvc/hvc_vio.c b/drivers/tty/hvc/hvc_vio.c
index 736b230f5ec0..011b239a7e52 100644
--- a/drivers/tty/hvc/hvc_vio.c
+++ b/drivers/tty/hvc/hvc_vio.c@@ -115,7 +115,7 @@ static int hvterm_raw_get_chars(uint32_t vtermno, char *buf, int count) * you are sending fewer chars. * @count: number of chars to send. */ -static int hvterm_raw_put_chars(uint32_t vtermno, const char *buf, int count) +static int hvterm_raw_put_chars(uint32_t vtermno, const char buf[16], int count) { struct hvterm_priv *pv = hvterm_privs[vtermno];