* [PATCH] net: s2io: fix buffer overflow
@ 2010-07-23 16:36 ` Kulikov Vasiliy
0 siblings, 0 replies; 4+ messages in thread
From: Kulikov Vasiliy @ 2010-07-23 16:36 UTC (permalink / raw)
To: kernel-janitors
Cc: Ramkrishna Vepa, Sivakumar Subramani, Sreenivasa Honnur,
Jon Mason, David S. Miller, Joe Perches, Jiri Pirko, netdev
vpd_data[] is allocated as kmalloc(256, GFP_KERNEL), so if cnt = 255
then (cnt + 3) overflows 256. memset() is executed without checking.
vpd_data[cnt+2] must be less than 256-cnt-2 as the latter is number of
vpd_data[] elements to copy.
Do not fill with zero the beginning of nic->serial_num as it will
be filled with vpd_data[].
String in product_name[] should be terminated by '\0'.
Signed-off-by: Kulikov Vasiliy <segooon@gmail.com>
---
drivers/net/s2io.c | 28 ++++++++++++++++++----------
1 files changed, 18 insertions(+), 10 deletions(-)
diff --git a/drivers/net/s2io.c b/drivers/net/s2io.c
index b8b8584..18bc5b7 100644
--- a/drivers/net/s2io.c
+++ b/drivers/net/s2io.c
@@ -5796,7 +5796,7 @@ static void s2io_vpd_read(struct s2io_nic *nic)
{
u8 *vpd_data;
u8 data;
- int i = 0, cnt, fail = 0;
+ int i = 0, cnt, len, fail = 0;
int vpd_addr = 0x80;
struct swStat *swstats = &nic->mac_control.stats_info->sw_stat;
@@ -5837,20 +5837,28 @@ static void s2io_vpd_read(struct s2io_nic *nic)
if (!fail) {
/* read serial number of adapter */
- for (cnt = 0; cnt < 256; cnt++) {
+ for (cnt = 0; cnt < 252; cnt++) {
if ((vpd_data[cnt] == 'S') &&
- (vpd_data[cnt+1] == 'N') &&
- (vpd_data[cnt+2] < VPD_STRING_LEN)) {
- memset(nic->serial_num, 0, VPD_STRING_LEN);
- memcpy(nic->serial_num, &vpd_data[cnt + 3],
- vpd_data[cnt+2]);
- break;
+ (vpd_data[cnt+1] == 'N')) {
+ len = vpd_data[cnt+2];
+ if (len < min(VPD_STRING_LEN, 256-cnt-2)) {
+ memcpy(nic->serial_num,
+ &vpd_data[cnt + 3],
+ len);
+ memset(nic->serial_num+len,
+ 0,
+ VPD_STRING_LEN-len);
+ break;
+ }
}
}
}
- if ((!fail) && (vpd_data[1] < VPD_STRING_LEN))
- memcpy(nic->product_name, &vpd_data[3], vpd_data[1]);
+ if ((!fail) && (vpd_data[1] < VPD_STRING_LEN)) {
+ len = vpd_data[1];
+ memcpy(nic->product_name, &vpd_data[3], len);
+ nic->product_name[len] = 0;
+ }
kfree(vpd_data);
swstats->mem_freed += 256;
}
--
1.7.0.4
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH] net: s2io: fix buffer overflow
@ 2010-07-23 16:36 ` Kulikov Vasiliy
0 siblings, 0 replies; 4+ messages in thread
From: Kulikov Vasiliy @ 2010-07-23 16:36 UTC (permalink / raw)
To: kernel-janitors
Cc: Ramkrishna Vepa, Sivakumar Subramani, Sreenivasa Honnur,
Jon Mason, David S. Miller, Joe Perches, Jiri Pirko, netdev
vpd_data[] is allocated as kmalloc(256, GFP_KERNEL), so if cnt = 255
then (cnt + 3) overflows 256. memset() is executed without checking.
vpd_data[cnt+2] must be less than 256-cnt-2 as the latter is number of
vpd_data[] elements to copy.
Do not fill with zero the beginning of nic->serial_num as it will
be filled with vpd_data[].
String in product_name[] should be terminated by '\0'.
Signed-off-by: Kulikov Vasiliy <segooon@gmail.com>
---
drivers/net/s2io.c | 28 ++++++++++++++++++----------
1 files changed, 18 insertions(+), 10 deletions(-)
diff --git a/drivers/net/s2io.c b/drivers/net/s2io.c
index b8b8584..18bc5b7 100644
--- a/drivers/net/s2io.c
+++ b/drivers/net/s2io.c
@@ -5796,7 +5796,7 @@ static void s2io_vpd_read(struct s2io_nic *nic)
{
u8 *vpd_data;
u8 data;
- int i = 0, cnt, fail = 0;
+ int i = 0, cnt, len, fail = 0;
int vpd_addr = 0x80;
struct swStat *swstats = &nic->mac_control.stats_info->sw_stat;
@@ -5837,20 +5837,28 @@ static void s2io_vpd_read(struct s2io_nic *nic)
if (!fail) {
/* read serial number of adapter */
- for (cnt = 0; cnt < 256; cnt++) {
+ for (cnt = 0; cnt < 252; cnt++) {
if ((vpd_data[cnt] = 'S') &&
- (vpd_data[cnt+1] = 'N') &&
- (vpd_data[cnt+2] < VPD_STRING_LEN)) {
- memset(nic->serial_num, 0, VPD_STRING_LEN);
- memcpy(nic->serial_num, &vpd_data[cnt + 3],
- vpd_data[cnt+2]);
- break;
+ (vpd_data[cnt+1] = 'N')) {
+ len = vpd_data[cnt+2];
+ if (len < min(VPD_STRING_LEN, 256-cnt-2)) {
+ memcpy(nic->serial_num,
+ &vpd_data[cnt + 3],
+ len);
+ memset(nic->serial_num+len,
+ 0,
+ VPD_STRING_LEN-len);
+ break;
+ }
}
}
}
- if ((!fail) && (vpd_data[1] < VPD_STRING_LEN))
- memcpy(nic->product_name, &vpd_data[3], vpd_data[1]);
+ if ((!fail) && (vpd_data[1] < VPD_STRING_LEN)) {
+ len = vpd_data[1];
+ memcpy(nic->product_name, &vpd_data[3], len);
+ nic->product_name[len] = 0;
+ }
kfree(vpd_data);
swstats->mem_freed += 256;
}
--
1.7.0.4
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] net: s2io: fix buffer overflow
2010-07-23 16:36 ` Kulikov Vasiliy
@ 2010-07-23 20:06 ` David Miller
-1 siblings, 0 replies; 4+ messages in thread
From: David Miller @ 2010-07-23 20:06 UTC (permalink / raw)
To: segooon
Cc: kernel-janitors, ramkrishna.vepa, sivakumar.subramani,
sreenivasa.honnur, jon.mason, joe, jpirko, netdev
From: Kulikov Vasiliy <segooon@gmail.com>
Date: Fri, 23 Jul 2010 20:36:15 +0400
> vpd_data[] is allocated as kmalloc(256, GFP_KERNEL), so if cnt = 255
> then (cnt + 3) overflows 256. memset() is executed without checking.
> vpd_data[cnt+2] must be less than 256-cnt-2 as the latter is number of
> vpd_data[] elements to copy.
>
> Do not fill with zero the beginning of nic->serial_num as it will
> be filled with vpd_data[].
>
> String in product_name[] should be terminated by '\0'.
>
> Signed-off-by: Kulikov Vasiliy <segooon@gmail.com>
Applied.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] net: s2io: fix buffer overflow
@ 2010-07-23 20:06 ` David Miller
0 siblings, 0 replies; 4+ messages in thread
From: David Miller @ 2010-07-23 20:06 UTC (permalink / raw)
To: segooon
Cc: kernel-janitors, ramkrishna.vepa, sivakumar.subramani,
sreenivasa.honnur, jon.mason, joe, jpirko, netdev
From: Kulikov Vasiliy <segooon@gmail.com>
Date: Fri, 23 Jul 2010 20:36:15 +0400
> vpd_data[] is allocated as kmalloc(256, GFP_KERNEL), so if cnt = 255
> then (cnt + 3) overflows 256. memset() is executed without checking.
> vpd_data[cnt+2] must be less than 256-cnt-2 as the latter is number of
> vpd_data[] elements to copy.
>
> Do not fill with zero the beginning of nic->serial_num as it will
> be filled with vpd_data[].
>
> String in product_name[] should be terminated by '\0'.
>
> Signed-off-by: Kulikov Vasiliy <segooon@gmail.com>
Applied.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2010-07-23 20:06 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-07-23 16:36 [PATCH] net: s2io: fix buffer overflow Kulikov Vasiliy
2010-07-23 16:36 ` Kulikov Vasiliy
2010-07-23 20:06 ` David Miller
2010-07-23 20:06 ` David Miller
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.