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-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20240705085834.17224-1-tuhaowen@uniontech.com>
Date: Fri,  5 Jul 2024 16:58:34 +0800
From: tuhaowen <tuhaowen@...ontech.com>
To: gregkh@...uxfoundation.org
Cc: alexander.deucher@....com,
	huangbibo@...ontech.com,
	linux-kernel@...r.kernel.org,
	sudipm.mukherjee@...il.com,
	tuhaowen@...ontech.com,
	wangyuli@...ontech.com
Subject: Re: Re: [PATCH] dev/parport: fix the array out-of-bounds risk

On Fri, Jul 05, 2024 at 02:45:45PM +0800, Greg Kroah-Hartman wrote:

> I'm not disputing that this change looks correct, I'm asking that you
> redo it and properly check the array size when writing to it so as to
> ensure that it really is correct in case our math is incorrect
> somewhere.

Thank you for your feedback. I have redone the change and 
ensured that the array size is properly checked when writing to it. 
Please find my updated patch below.

Thanks,

Haowen.Tu


>From cfdf8445987147f3a7e4f1232a8c6e8a271e4a6d Mon Sep 17 00:00:00 2001
From: tuhaowen <tuhaowen@...ontech.com>
Date: Mon, 27 May 2024 14:34:32 +0800
Subject: [PATCH] dev/parport: fix the array out-of-bounds risk

Fixed array out-of-bounds issues caused by sprintf
by replacing it with snprintf for safer data copying,
ensuring the destination buffer is not overflowed.

Below is the stack trace I encountered during the actual issue:

[ 66.575408s] [pid:5118,cpu4,QThread,4]Kernel panic - not syncing: stack-protector:
Kernel stack is corrupted in: do_hardware_base_addr+0xcc/0xd0 [parport]
[ 66.575408s] [pid:5118,cpu4,QThread,5]CPU: 4 PID: 5118 Comm:
QThread Tainted: G S W O 5.10.97-arm64-desktop #7100.57021.2
[ 66.575439s] [pid:5118,cpu4,QThread,6]TGID: 5087 Comm: EFileApp
[ 66.575439s] [pid:5118,cpu4,QThread,7]Hardware name: HUAWEI HUAWEI QingYun
PGUX-W515x-B081/SP1PANGUXM, BIOS 1.00.07 04/29/2024
[ 66.575439s] [pid:5118,cpu4,QThread,8]Call trace:
[ 66.575469s] [pid:5118,cpu4,QThread,9] dump_backtrace+0x0/0x1c0
[ 66.575469s] [pid:5118,cpu4,QThread,0] show_stack+0x14/0x20
[ 66.575469s] [pid:5118,cpu4,QThread,1] dump_stack+0xd4/0x10c
[ 66.575500s] [pid:5118,cpu4,QThread,2] panic+0x1d8/0x3bc
[ 66.575500s] [pid:5118,cpu4,QThread,3] __stack_chk_fail+0x2c/0x38
[ 66.575500s] [pid:5118,cpu4,QThread,4] do_hardware_base_addr+0xcc/0xd0 [parport]

Signed-off-by: tuhaowen <tuhaowen@...ontech.com>
---
 drivers/parport/procfs.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/parport/procfs.c b/drivers/parport/procfs.c
index bd388560ed59..27674a34ca3c 100644
--- a/drivers/parport/procfs.c
+++ b/drivers/parport/procfs.c
@@ -51,12 +51,12 @@ static int do_active_device(struct ctl_table *table, int write,
 
        for (dev = port->devices; dev ; dev = dev->next) {
                if(dev == port->cad) {
-                       len += sprintf(buffer, "%s\n", dev->name);
+                       len += snprintf(buffer, 64, "%s\n", dev->name);
                }
        }
 
        if(!len) {
-               len += sprintf(buffer, "%s\n", "none");
+               len += snprintf(buffer, 64, "%s\n", "none");
        }
 
        if (len > *lenp)
@@ -87,19 +87,19 @@ static int do_autoprobe(struct ctl_table *table, int write,
        }
 
        if ((str = info->class_name) != NULL)
-               len += sprintf (buffer + len, "CLASS:%s;\n", str);
+               len += snprintf (buffer + len, 256 - len, "CLASS:%s;\n", str);
 
        if ((str = info->model) != NULL)
-               len += sprintf (buffer + len, "MODEL:%s;\n", str);
+               len += snprintf (buffer + len, 256 - len, "MODEL:%s;\n", str);
 
        if ((str = info->mfr) != NULL)
-               len += sprintf (buffer + len, "MANUFACTURER:%s;\n", str);
+               len += snprintf (buffer + len, 256 - len, "MANUFACTURER:%s;\n", str);
 
        if ((str = info->description) != NULL)
-               len += sprintf (buffer + len, "DESCRIPTION:%s;\n", str);
+               len += snprintf (buffer + len, 256 - len, "DESCRIPTION:%s;\n", str);
 
        if ((str = info->cmdset) != NULL)
-               len += sprintf (buffer + len, "COMMAND SET:%s;\n", str);
+               len += snprintf (buffer + len, 256 - len, "COMMAND SET:%s;\n", str);
 
        if (len > *lenp)
                len = *lenp;
@@ -117,7 +117,7 @@ static int do_hardware_base_addr(struct ctl_table *table, int write,
                                 void *result, size_t *lenp, loff_t *ppos)
 {
        struct parport *port = (struct parport *)table->extra1;
-       char buffer[20];
+       char buffer[64];
        int len = 0;
 
        if (*ppos) {
@@ -128,7 +128,7 @@ static int do_hardware_base_addr(struct ctl_table *table, int write,
        if (write) /* permissions prevent this anyway */
                return -EACCES;
 
-       len += sprintf (buffer, "%lu\t%lu\n", port->base, port->base_hi);
+       len += snprintf (buffer, 64, "%lu\t%lu\n", port->base, port->base_hi);
 
        if (len > *lenp)
                len = *lenp;
@@ -155,7 +155,7 @@ static int do_hardware_irq(struct ctl_table *table, int write,
        if (write) /* permissions prevent this anyway */
                return -EACCES;
 
-       len += sprintf (buffer, "%d\n", port->irq);
+       len += snprintf (buffer, 20, "%d\n", port->irq);
 
        if (len > *lenp)
                len = *lenp;
@@ -182,7 +182,7 @@ static int do_hardware_dma(struct ctl_table *table, int write,
        if (write) /* permissions prevent this anyway */
                return -EACCES;
 
-       len += sprintf (buffer, "%d\n", port->dma);
+       len += snprintf (buffer, 20, "%d\n", port->dma);
 
        if (len > *lenp)
                len = *lenp;
@@ -213,7 +213,7 @@ static int do_hardware_modes(struct ctl_table *table, int write,
 #define printmode(x)                                                   \
 do {                                                                   \
        if (port->modes & PARPORT_MODE_##x)                             \
-               len += sprintf(buffer + len, "%s%s", f++ ? "," : "", #x); \
+               len += snprintf(buffer + len, 40 - len, "%s%s", f++ ? "," : "", #x); \
 } while (0)
                int f = 0;
                printmode(PCSPP);
-- 
2.20.1


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ