* [PATCH] dev/parport: fix the array out-of-bounds risk @ 2024-06-26 2:11 tuhaowen 2024-07-04 10:07 ` Greg KH 0 siblings, 1 reply; 9+ messages in thread From: tuhaowen @ 2024-06-26 2:11 UTC (permalink / raw) To: sudipm.mukherjee, alexander.deucher, gregkh Cc: linux-kernel, huangbibo, wangyuli, tuhaowen The array buffer size is 20 bytes. When executing code in a 64-bit CPU environment, up to 42 bytes of data will be written into this array (the size of "%lu\t%lu\n" is 20 + 1 + 20 + 1). In fact, this line of code for 32-bit CPUs also has the risk of crossing the boundary, but it can exceed 2 bytes at most. With good luck, it is local variables that are damaged, and there are no serious consequences. Signed-off-by: tuhaowen <tuhaowen@uniontech.com> --- drivers/parport/procfs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/parport/procfs.c b/drivers/parport/procfs.c index bd388560ed59..9f501d5fa1ec 100644 --- a/drivers/parport/procfs.c +++ b/drivers/parport/procfs.c @@ -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) { -- 2.20.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] dev/parport: fix the array out-of-bounds risk 2024-06-26 2:11 [PATCH] dev/parport: fix the array out-of-bounds risk tuhaowen @ 2024-07-04 10:07 ` Greg KH 2024-07-05 6:36 ` tuhaowen 0 siblings, 1 reply; 9+ messages in thread From: Greg KH @ 2024-07-04 10:07 UTC (permalink / raw) To: tuhaowen Cc: sudipm.mukherjee, alexander.deucher, linux-kernel, huangbibo, wangyuli On Wed, Jun 26, 2024 at 10:11:36AM +0800, tuhaowen wrote: > The array buffer size is 20 bytes. > When executing code in a 64-bit CPU environment, up to 42 bytes of > data will be written into this array > (the size of "%lu\t%lu\n" is 20 + 1 + 20 + 1). > > In fact, this line of code for 32-bit CPUs also has the risk of > crossing the boundary, but it can exceed 2 bytes at most. With good > luck, it is local variables that are damaged, and there are no serious > consequences. Usually because no one actually has this hardware anymore :) Can you also properly test the buffer size when writing into it so that even if the math is incorrect, it will not overflow? thanks, greg k-h ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Re: [PATCH] dev/parport: fix the array out-of-bounds risk 2024-07-04 10:07 ` Greg KH @ 2024-07-05 6:36 ` tuhaowen 2024-07-05 6:45 ` Greg KH 0 siblings, 1 reply; 9+ messages in thread From: tuhaowen @ 2024-07-05 6:36 UTC (permalink / raw) To: gregkh Cc: alexander.deucher, huangbibo, linux-kernel, sudipm.mukherjee, tuhaowen, wangyuli On Thu, Jul 04, 2024 at 06:07:47PM +0800, Greg Kroah-Hartman wrote: > Usually because no one actually has this hardware anymore :) > > Can you also properly test the buffer size when writing into it so that > even if the math is incorrect, it will not overflow? As of now, I have encountered these three devices: BUNET BU1L02, EB-LINK EB-2C1B01, and SYBA FG-EMT03A-N. When these PCIe to parallel port cards are installed, the system experiences abnormal reboots. I am not sure if there are other devices with these issues. 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] The array buffer size is 20 bytes. When executing code in a 64-bit CPU environment, up to 44 bytes of data will be written into this array (the size of "%lu\t%lu\n" is 21 + 1 + 21 + 1). This modification will resolve the current issue without introducing new problems. Thanks, Haowen.Tu ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Re: [PATCH] dev/parport: fix the array out-of-bounds risk 2024-07-05 6:36 ` tuhaowen @ 2024-07-05 6:45 ` Greg KH 2024-07-05 8:58 ` tuhaowen 0 siblings, 1 reply; 9+ messages in thread From: Greg KH @ 2024-07-05 6:45 UTC (permalink / raw) To: tuhaowen Cc: alexander.deucher, huangbibo, linux-kernel, sudipm.mukherjee, wangyuli On Fri, Jul 05, 2024 at 02:36:58PM +0800, tuhaowen wrote: > On Thu, Jul 04, 2024 at 06:07:47PM +0800, Greg Kroah-Hartman wrote: > > > Usually because no one actually has this hardware anymore :) > > > > Can you also properly test the buffer size when writing into it so that > > even if the math is incorrect, it will not overflow? > > > As of now, I have encountered these three devices: BUNET BU1L02, > EB-LINK EB-2C1B01, and SYBA FG-EMT03A-N. When these PCIe to parallel > port cards are installed, the system experiences abnormal reboots. > I am not sure if there are other devices with these issues. > > 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] > > > The array buffer size is 20 bytes. > When executing code in a 64-bit CPU environment, > up to 44 bytes of data will be written into this array > (the size of "%lu\t%lu\n" is 21 + 1 + 21 + 1). > > This modification will resolve the current issue without introducing new problems. 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. thanks, greg k-h ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Re: [PATCH] dev/parport: fix the array out-of-bounds risk 2024-07-05 6:45 ` Greg KH @ 2024-07-05 8:58 ` tuhaowen 2024-07-05 9:42 ` Greg KH 0 siblings, 1 reply; 9+ messages in thread From: tuhaowen @ 2024-07-05 8:58 UTC (permalink / raw) To: gregkh Cc: alexander.deucher, huangbibo, linux-kernel, sudipm.mukherjee, tuhaowen, wangyuli 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@uniontech.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@uniontech.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 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: Re: [PATCH] dev/parport: fix the array out-of-bounds risk 2024-07-05 8:58 ` tuhaowen @ 2024-07-05 9:42 ` Greg KH 2024-07-08 2:33 ` tuhaowen 0 siblings, 1 reply; 9+ messages in thread From: Greg KH @ 2024-07-05 9:42 UTC (permalink / raw) To: tuhaowen Cc: alexander.deucher, huangbibo, linux-kernel, sudipm.mukherjee, wangyuli On Fri, Jul 05, 2024 at 04:58:34PM +0800, tuhaowen wrote: > 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@uniontech.com> > Date: Mon, 27 May 2024 14:34:32 +0800 > Subject: [PATCH] dev/parport: fix the array out-of-bounds risk Sorry, we can't take inline patches like this, please read the documentation for how to submit a new version of a patch. But, some comments below: > 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); Don't use magic numbers here and everywhere else, use sizeof() to compute these properly everywhere. thanks, greg k-h ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] dev/parport: fix the array out-of-bounds risk 2024-07-05 9:42 ` Greg KH @ 2024-07-08 2:33 ` tuhaowen 2024-07-08 7:18 ` Greg KH 0 siblings, 1 reply; 9+ messages in thread From: tuhaowen @ 2024-07-08 2:33 UTC (permalink / raw) To: gregkh Cc: alexander.deucher, huangbibo, linux-kernel, sudipm.mukherjee, tuhaowen, wangyuli 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@uniontech.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..c2e371c50dcf 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, sizeof(buffer), "%s\n", dev->name); } } if(!len) { - len += sprintf(buffer, "%s\n", "none"); + len += snprintf(buffer, sizeof(buffer), "%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, sizeof(buffer) - len, "CLASS:%s;\n", str); if ((str = info->model) != NULL) - len += sprintf (buffer + len, "MODEL:%s;\n", str); + len += snprintf (buffer + len, sizeof(buffer) - len, "MODEL:%s;\n", str); if ((str = info->mfr) != NULL) - len += sprintf (buffer + len, "MANUFACTURER:%s;\n", str); + len += snprintf (buffer + len, sizeof(buffer) - len, "MANUFACTURER:%s;\n", str); if ((str = info->description) != NULL) - len += sprintf (buffer + len, "DESCRIPTION:%s;\n", str); + len += snprintf (buffer + len, sizeof(buffer) - len, "DESCRIPTION:%s;\n", str); if ((str = info->cmdset) != NULL) - len += sprintf (buffer + len, "COMMAND SET:%s;\n", str); + len += snprintf (buffer + len, sizeof(buffer) - 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, sizeof(buffer), "%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, sizeof(buffer), "%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, sizeof(buffer), "%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, sizeof(buffer) - len, "%s%s", f++ ? "," : "", #x); \ } while (0) int f = 0; printmode(PCSPP); -- 2.20.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] dev/parport: fix the array out-of-bounds risk 2024-07-08 2:33 ` tuhaowen @ 2024-07-08 7:18 ` Greg KH 2024-07-08 8:04 ` [PATCH v2] " tuhaowen 0 siblings, 1 reply; 9+ messages in thread From: Greg KH @ 2024-07-08 7:18 UTC (permalink / raw) To: tuhaowen Cc: alexander.deucher, huangbibo, linux-kernel, sudipm.mukherjee, wangyuli On Mon, Jul 08, 2024 at 10:33:30AM +0800, tuhaowen wrote: > 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@uniontech.com> > --- > drivers/parport/procfs.c | 24 ++++++++++++------------ > 1 file changed, 12 insertions(+), 12 deletions(-) > Hi, This is the friendly patch-bot of Greg Kroah-Hartman. You have sent him a patch that has triggered this response. He used to manually respond to these common problems, but in order to save his sanity (he kept writing the same thing over and over, yet to different people), I was created. Hopefully you will not take offence and will fix the problem in your patch and resubmit it so that it can be accepted into the Linux kernel tree. You are receiving this message because of the following common error(s) as indicated below: - It looks like you did not use your normal name for the patch on either the Signed-off-by: line, or the From: line (both of which have to match). Please read the kernel file, Documentation/process/submitting-patches.rst for how to do this correctly. - This looks like a new version of a previously submitted patch, but you did not list below the --- line any changes from the previous version. Please read the section entitled "The canonical patch format" in the kernel file, Documentation/process/submitting-patches.rst for what needs to be done here to properly describe this. If you wish to discuss this problem further, or you have questions about how to resolve this issue, please feel free to respond to this email and Greg will reply once he has dug out from the pending patches received from other developers. thanks, greg k-h's patch email bot ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2] dev/parport: fix the array out-of-bounds risk 2024-07-08 7:18 ` Greg KH @ 2024-07-08 8:04 ` tuhaowen 0 siblings, 0 replies; 9+ messages in thread From: tuhaowen @ 2024-07-08 8:04 UTC (permalink / raw) To: gregkh Cc: alexander.deucher, huangbibo, linux-kernel, sudipm.mukherjee, tuhaowen, wangyuli 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@uniontech.com> --- Changes in v2: - Replaced `sprintf` with `snprintf` to prevent array out-of-bounds issues. - Link to v1: https://lore.kernel.org/all/20240626021136.12282-1-tuhaowen@uniontech.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..c2e371c50dcf 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, sizeof(buffer), "%s\n", dev->name); } } if(!len) { - len += sprintf(buffer, "%s\n", "none"); + len += snprintf(buffer, sizeof(buffer), "%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, sizeof(buffer) - len, "CLASS:%s;\n", str); if ((str = info->model) != NULL) - len += sprintf (buffer + len, "MODEL:%s;\n", str); + len += snprintf (buffer + len, sizeof(buffer) - len, "MODEL:%s;\n", str); if ((str = info->mfr) != NULL) - len += sprintf (buffer + len, "MANUFACTURER:%s;\n", str); + len += snprintf (buffer + len, sizeof(buffer) - len, "MANUFACTURER:%s;\n", str); if ((str = info->description) != NULL) - len += sprintf (buffer + len, "DESCRIPTION:%s;\n", str); + len += snprintf (buffer + len, sizeof(buffer) - len, "DESCRIPTION:%s;\n", str); if ((str = info->cmdset) != NULL) - len += sprintf (buffer + len, "COMMAND SET:%s;\n", str); + len += snprintf (buffer + len, sizeof(buffer) - 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, sizeof(buffer), "%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, sizeof(buffer), "%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, sizeof(buffer), "%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, sizeof(buffer) - len, "%s%s", f++ ? "," : "", #x); \ } while (0) int f = 0; printmode(PCSPP); -- 2.20.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
end of thread, other threads:[~2024-07-08 8:05 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-06-26 2:11 [PATCH] dev/parport: fix the array out-of-bounds risk tuhaowen 2024-07-04 10:07 ` Greg KH 2024-07-05 6:36 ` tuhaowen 2024-07-05 6:45 ` Greg KH 2024-07-05 8:58 ` tuhaowen 2024-07-05 9:42 ` Greg KH 2024-07-08 2:33 ` tuhaowen 2024-07-08 7:18 ` Greg KH 2024-07-08 8:04 ` [PATCH v2] " tuhaowen
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox