From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from metis.ext.pengutronix.de ([2001:67c:670:201:290:27ff:fe1d:cc33]) by bombadil.infradead.org with esmtps (Exim 4.92 #3 (Red Hat Linux)) id 1hsju7-0004jp-Py for barebox@lists.infradead.org; Wed, 31 Jul 2019 08:24:13 +0000 References: <20190730154809.27903-1-a.fatoum@pengutronix.de> <0d2bfef4-bc62-9f33-346b-490c5283ea59@pengutronix.de> From: Ahmad Fatoum Message-ID: Date: Wed, 31 Jul 2019 10:24:09 +0200 MIME-Version: 1.0 In-Reply-To: <0d2bfef4-bc62-9f33-346b-490c5283ea59@pengutronix.de> Content-Language: en-US List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "barebox" Errors-To: barebox-bounces+u.kleine-koenig=pengutronix.de@lists.infradead.org Subject: Re: [PATCH] console: fix out-of-bounds read in dputc(/dev/*, ...) To: Bastian Krause , barebox@lists.infradead.org On 31/7/19 09:05, Bastian Krause wrote: > On 7/30/19 5:48 PM, Ahmad Fatoum wrote: >> Trying to output a single character via >> echo -a /dev/serial0-1 >> currently results in garbage output after the newline, because console.c's >> fops_write discards the buffer length and passes the buffer to >> (struct cdev)::puts which only handles NUL-terminated strings. >> >> Fix this by amending (struct cdev)::puts with a new nbytes parameter, >> which is correctly propagated. >> >> Fixes: 1233570798 ("console: expose consoles in devfs") > > The is the wrong commit id, I guess you mean b4f55fcf35. Oh, off by one (commit). Will resend. > >> Cc: Bastian Krause >> Signed-off-by: Ahmad Fatoum >> --- >> common/console.c | 12 ++++++------ >> common/ratp/ratp.c | 12 +++++------- >> drivers/serial/efi-stdio.c | 5 +++-- >> drivers/serial/serial_efi.c | 5 +++-- >> fs/pstore/platform.c | 7 ++++--- >> include/console.h | 2 +- >> 6 files changed, 22 insertions(+), 21 deletions(-) >> >> diff --git a/common/console.c b/common/console.c >> index 47ccf2e54de0..59218ea4e22f 100644 >> --- a/common/console.c >> +++ b/common/console.c >> @@ -253,17 +253,17 @@ static void console_set_stdoutpath(struct console_device *cdev) >> free(str); >> } >> >> -static int __console_puts(struct console_device *cdev, const char *s) >> +static int __console_puts(struct console_device *cdev, const char *s, >> + size_t nbytes) >> { >> - int n = 0; >> + int n; >> >> - while (*s) { >> + for (n = 0; n < nbytes; n++) { >> if (*s == '\n') { >> cdev->putc(cdev, '\r'); >> n++; >> } >> cdev->putc(cdev, *s); >> - n++; >> s++; >> } >> return n; >> @@ -298,7 +298,7 @@ static ssize_t fops_write(struct cdev* dev, const void* buf, size_t count, >> { >> struct console_device *priv = dev->priv; >> >> - priv->puts(priv, buf); >> + priv->puts(priv, buf, count); >> >> return 0; >> } >> @@ -545,7 +545,7 @@ int console_puts(unsigned int ch, const char *str) >> if (initialized == CONSOLE_INIT_FULL) { >> for_each_console(cdev) { >> if (cdev->f_active & ch) { >> - n = cdev->puts(cdev, str); >> + n = cdev->puts(cdev, str, strlen(str)); >> } >> } >> return n; >> diff --git a/common/ratp/ratp.c b/common/ratp/ratp.c >> index 9aea1786d684..e84ad221672a 100644 >> --- a/common/ratp/ratp.c >> +++ b/common/ratp/ratp.c >> @@ -259,19 +259,17 @@ static int ratp_console_tstc(struct console_device *cdev) >> return kfifo_len(ctx->console_recv_fifo) ? 1 : 0; >> } >> >> -static int ratp_console_puts(struct console_device *cdev, const char *s) >> +static int ratp_console_puts(struct console_device *cdev, const char *s, >> + size_t nbytes) >> { >> struct ratp_ctx *ctx = container_of(cdev, struct ratp_ctx, ratp_console); >> - int len = 0; >> - >> - len = strlen(s); >> >> if (ratp_busy(&ctx->ratp)) >> - return len; >> + return nbytes; >> >> - kfifo_put(ctx->console_transmit_fifo, s, len); >> + kfifo_put(ctx->console_transmit_fifo, s, nbytes); >> >> - return len; >> + return nbytes; >> } >> >> static void ratp_console_putc(struct console_device *cdev, char c) >> diff --git a/drivers/serial/efi-stdio.c b/drivers/serial/efi-stdio.c >> index 0703f727e766..2ca89fa4f861 100644 >> --- a/drivers/serial/efi-stdio.c >> +++ b/drivers/serial/efi-stdio.c >> @@ -243,12 +243,13 @@ static int efi_process_key(struct efi_console_priv *priv, const char *inp) >> return 1; >> } >> >> -static int efi_console_puts(struct console_device *cdev, const char *s) >> +static int efi_console_puts(struct console_device *cdev, const char *s, >> + size_t nbytes) >> { >> struct efi_console_priv *priv = to_efi(cdev); >> int n = 0; >> >> - while (*s) { >> + while (nbytes--) { >> if (*s == 27) { >> priv->efi_console_buffer[n] = 0; >> priv->out->output_string(priv->out, >> diff --git a/drivers/serial/serial_efi.c b/drivers/serial/serial_efi.c >> index f0a2b22c2bc2..667d51f622ec 100644 >> --- a/drivers/serial/serial_efi.c >> +++ b/drivers/serial/serial_efi.c >> @@ -130,13 +130,14 @@ static void efi_serial_putc(struct console_device *cdev, char c) >> serial->write(serial, &buffersize, &c); >> } >> >> -static int efi_serial_puts(struct console_device *cdev, const char *s) >> +static int efi_serial_puts(struct console_device *cdev, const char *s, >> + size_t nbytes) >> { >> struct efi_serial_port *uart = to_efi_serial_port(cdev); >> struct efi_serial_io_protocol *serial = uart->serial; >> uint32_t control; >> efi_status_t efiret; >> - unsigned long buffersize = strlen(s) * sizeof(char); >> + unsigned long buffersize = nbytes; >> >> do { >> efiret = serial->getcontrol(serial, &control); >> diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c >> index 755363c30999..601bfca6b025 100644 >> --- a/fs/pstore/platform.c >> +++ b/fs/pstore/platform.c >> @@ -67,10 +67,11 @@ static void pstore_console_write(const char *s, unsigned c) >> } >> } >> >> -static int pstore_console_puts(struct console_device *cdev, const char *s) >> +static int pstore_console_puts(struct console_device *cdev, const char *s, >> + size_t nbytes) >> { >> - pstore_console_write(s, strlen(s)); >> - return strlen(s); >> + pstore_console_write(s, nbytes); >> + return nbytes; >> } >> >> static void pstore_console_putc(struct console_device *cdev, char c) >> diff --git a/include/console.h b/include/console.h >> index 673921331db7..1d86a584a7f2 100644 >> --- a/include/console.h >> +++ b/include/console.h >> @@ -42,7 +42,7 @@ struct console_device { >> >> int (*tstc)(struct console_device *cdev); >> void (*putc)(struct console_device *cdev, char c); >> - int (*puts)(struct console_device *cdev, const char *s); >> + int (*puts)(struct console_device *cdev, const char *s, size_t nbytes); >> int (*getc)(struct console_device *cdev); >> int (*setbrg)(struct console_device *cdev, int baudrate); >> void (*flush)(struct console_device *cdev); >> > > Looks correct. > > Bastian > -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox