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 1hsiff-0000HD-KC for barebox@lists.infradead.org; Wed, 31 Jul 2019 07:05:28 +0000 References: <20190730154809.27903-1-a.fatoum@pengutronix.de> From: Bastian Krause Message-ID: <0d2bfef4-bc62-9f33-346b-490c5283ea59@pengutronix.de> Date: Wed, 31 Jul 2019 09:05:07 +0200 MIME-Version: 1.0 In-Reply-To: <20190730154809.27903-1-a.fatoum@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: Ahmad Fatoum , barebox@lists.infradead.org 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. > 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 Amtsgericht Hildesheim, HRA 2686 _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox