linux-kernel - [PATCH] tty: serial: samsung: Fix potential buffer overflow in clkname

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250401192420169tLRsDis5R0RrVmdFnFuS9@zte.com.cn>
Date: Tue, 1 Apr 2025 19:24:20 +0800 (CST)
From: <shao.mingyin@....com.cn>
To: <alim.akhtar@...sung.com>
Cc: <yang.yang29@....com.cn>, <xu.xin16@....com.cn>, <ye.xingchen@....com.cn>,
        <krzk@...nel.org>, <gregkh@...uxfoundation.org>,
        <jirislaby@...nel.org>, <linux-arm-kernel@...ts.infradead.org>,
        <linux-samsung-soc@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <linux-serial@...r.kernel.org>, <jiang.peng9@....com.cn>
Subject: [PATCH] tty: serial: samsung: Fix potential buffer overflow in clkname

From: Peng Jiang <jiang.peng9@....com.cn>

Compiling the kernel with gcc12.3 W=1 produces a warning:
/drivers/tty/serial/samsung_tty.c: In function
's3c24xx_serial_set_termios':

/drivers/tty/serial/samsung_tty.c:1392:48:
warning: '%d' directive writing between 1 and 3 bytes
into a region of size 2 [-Wformat-overflow=]
 1392 |  sprintf(clkname, "clk_uart_baud%d", cnt);
      |                    ^~

In function 's3c24xx_serial_getclk',
    inlined from 's3c24xx_serial_set_termios'
at ./drivers/tty/serial/samsung_tty.c:1493:9:

/drivers/tty/serial/samsung_tty.c:1392:34:
note: directive argument in the range [0, 254]
 1392 |  sprintf(clkname, "clk_uart_baud%d", cnt);
      |                    ^~~~~~~~~~~~~~~~~

/drivers/tty/serial/samsung_tty.c:1392:17:
note: 'sprintf' output between 15 and 17 bytes
into a destination of size 15

 1392 |  sprintf(clkname, "clk_uart_baud%d", cnt);
      |                   ^~~~~~~~~~~~~~~~~

The compiler warned about a potential buffer overflow in the
`s3c24xx_serial_set_termios` function due to the use of `sprintf` which
could write more bytes than the allocated size of the `clkname` buffer.
This could lead to undefined behavior and potential security risks.

To reproduce the issue before applying the patch:
CONFIG_SERIAL_SAMSUNG=y
make vmlinux ARCH=arm64 CROSS_COMPILE=aarch64-linux- W=1

To resolve this issue, we have increased the buffer size for `clkname`
to ensure it can accommodate the longest possible string generated by
the formatting operation. Additionally, we have replaced `sprintf` with
`snprintf` to ensure that the function does not write beyond the end of
the buffer, thus preventing any potential overflow.

Signed-off-by: Peng Jiang <jiang.peng9@....com.cn>
Signed-off-by: Shao Mingyin <shao.mingyin@....com.cn>
---
 drivers/tty/serial/samsung_tty.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/serial/samsung_tty.c b/drivers/tty/serial/samsung_tty.c
index 210fff7164c1..5a0005033afa 100644
--- a/drivers/tty/serial/samsung_tty.c
+++ b/drivers/tty/serial/samsung_tty.c
@@ -1339,7 +1339,7 @@ static void s3c24xx_serial_pm(struct uart_port *port, unsigned int level,
  *
  */

-#define MAX_CLK_NAME_LENGTH 15
+#define MAX_CLK_NAME_LENGTH 18

 static inline u8 s3c24xx_serial_getsource(struct uart_port *port)
 {
@@ -1389,7 +1389,7 @@ static unsigned int s3c24xx_serial_getclk(struct s3c24xx_uart_port *ourport,
 			!(ourport->cfg->clk_sel & (1 << cnt)))
 			continue;

-		sprintf(clkname, "clk_uart_baud%d", cnt);
+		snprintf(clkname, sizeof(clkname), "clk_uart_baud%d", cnt);
 		clk = clk_get(ourport->port.dev, clkname);
 		if (IS_ERR(clk))
 			continue;
@@ -1787,7 +1787,7 @@ static int s3c24xx_serial_enable_baudclk(struct s3c24xx_uart_port *ourport)
 		if (!(clk_sel & (1 << clk_num)))
 			continue;

-		sprintf(clk_name, "clk_uart_baud%d", clk_num);
+		snprintf(clk_name, sizeof(clk_name), "clk_uart_baud%d", clk_num);
 		clk = clk_get(dev, clk_name);
 		if (IS_ERR(clk))
 			continue;
@@ -2335,7 +2335,7 @@ s3c24xx_serial_get_options(struct uart_port *port, int *baud,
 		/* now calculate the baud rate */

 		clk_sel = s3c24xx_serial_getsource(port);
-		sprintf(clk_name, "clk_uart_baud%d", clk_sel);
+		snprintf(clk_name, sizeof(clk_name), "clk_uart_baud%d", clk_sel);

 		clk = clk_get(port->dev, clk_name);
 		if (!IS_ERR(clk))
-- 
2.25.1