From: Peng Jiang <jiang.peng9@xxxxxxxxxx>
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@xxxxxxxxxx>
Signed-off-by: Shao Mingyin <shao.mingyin@xxxxxxxxxx>
---
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
