[2/2] app/testpmd: fix stack overflow for EEPROM display
Checks
Commit Message
When the size of EEPROM exceeds the default thread stack size(8MB),
e.g.: 10Mb size, it will be cashed with stack overflow.
Allocate the data of EPPROM information on the heap.
Fixes: 6b67721dee2a ("app/testpmd: add EEPROM command")
Signed-off-by: Steve Yang <stevex.yang@intel.com>
---
app/test-pmd/config.c | 22 ++++++++++++++++++----
1 file changed, 18 insertions(+), 4 deletions(-)
Comments
Hi Steve
The patch looks ok to me.
On 1/20/2022 8:29 AM, Steve Yang wrote:
> When the size of EEPROM exceeds the default thread stack size(8MB),
> e.g.: 10Mb size, it will be cashed with stack overflow.
Just spelling: 10Mb/10MB, cashed/crashed
Can even rephrase, like: it will crash due to stack overflow
> Allocate the data of EPPROM information on the heap.
>
> Fixes: 6b67721dee2a ("app/testpmd: add EEPROM command")
>
> Signed-off-by: Steve Yang<stevex.yang@intel.com>
Acked-by: Aman Singh <aman.deep.singh@intel.com>
> ---
> app/test-pmd/config.c | 22 ++++++++++++++++++----
> 1 file changed, 18 insertions(+), 4 deletions(-)
>
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> index 1722d6c8f8..e812f57151 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -912,10 +912,15 @@ port_eeprom_display(portid_t port_id)
> return;
> }
>
> - char buf[len_eeprom];
> einfo.offset = 0;
> einfo.length = len_eeprom;
> - einfo.data = buf;
> + einfo.data = calloc(1, len_eeprom);
> + if (!einfo.data) {
> + fprintf(stderr,
> + "Allocation of port %u eeprom data failed\n",
> + port_id);
> + return;
> + }
>
> ret = rte_eth_dev_get_eeprom(port_id, &einfo);
> if (ret != 0) {
> @@ -933,10 +938,12 @@ port_eeprom_display(portid_t port_id)
> fprintf(stderr, "Unable to get EEPROM: %d\n", ret);
> break;
> }
> + free(einfo.data);
> return;
> }
> rte_hexdump(stdout, "hexdump", einfo.data, einfo.length);
> printf("Finish -- Port: %d EEPROM length: %d bytes\n", port_id, len_eeprom); + free(einfo.data); } void @@ -972,10 +979,15
> @@ port_module_eeprom_display(portid_t port_id) return; } - char
> buf[minfo.eeprom_len]; einfo.offset = 0; einfo.length =
> minfo.eeprom_len; - einfo.data = buf; + einfo.data = calloc(1,
> minfo.eeprom_len); + if (!einfo.data) { + fprintf(stderr, + "Allocation of port %u eeprom data failed\n",
> + port_id);
> + return;
> + }
>
> ret = rte_eth_dev_get_module_eeprom(port_id, &einfo);
> if (ret != 0) {
> @@ -994,11 +1006,13 @@ port_module_eeprom_display(portid_t port_id)
> ret);
> break;
> }
> + free(einfo.data);
> return;
> }
>
> rte_hexdump(stdout, "hexdump", einfo.data, einfo.length);
> printf("Finish -- Port: %d MODULE EEPROM length: %d bytes\n", port_id, einfo.length);
> + free(einfo.data);
> }
>
> int
On 2/2/2022 10:00 AM, Singh, Aman Deep wrote:
> Hi Steve
> The patch looks ok to me.
>
> On 1/20/2022 8:29 AM, Steve Yang wrote:
>> When the size of EEPROM exceeds the default thread stack size(8MB),
>> e.g.: 10Mb size, it will be cashed with stack overflow.
> Just spelling: 10Mb/10MB, cashed/crashed
> Can even rephrase, like: it will crash due to stack overflow
fixed while merging
>> Allocate the data of EPPROM information on the heap.
>>
>> Fixes: 6b67721dee2a ("app/testpmd: add EEPROM command")
>>
>> Signed-off-by: Steve Yang<stevex.yang@intel.com>
>
> Acked-by: Aman Singh <aman.deep.singh@intel.com>
>
Tested-by: Ferruh Yigit <ferruh.yigit@intel.com>
Applied to dpdk-next-net/main, thanks.
@@ -912,10 +912,15 @@ port_eeprom_display(portid_t port_id)
return;
}
- char buf[len_eeprom];
einfo.offset = 0;
einfo.length = len_eeprom;
- einfo.data = buf;
+ einfo.data = calloc(1, len_eeprom);
+ if (!einfo.data) {
+ fprintf(stderr,
+ "Allocation of port %u eeprom data failed\n",
+ port_id);
+ return;
+ }
ret = rte_eth_dev_get_eeprom(port_id, &einfo);
if (ret != 0) {
@@ -933,10 +938,12 @@ port_eeprom_display(portid_t port_id)
fprintf(stderr, "Unable to get EEPROM: %d\n", ret);
break;
}
+ free(einfo.data);
return;
}
rte_hexdump(stdout, "hexdump", einfo.data, einfo.length);
printf("Finish -- Port: %d EEPROM length: %d bytes\n", port_id, len_eeprom);
+ free(einfo.data);
}
void
@@ -972,10 +979,15 @@ port_module_eeprom_display(portid_t port_id)
return;
}
- char buf[minfo.eeprom_len];
einfo.offset = 0;
einfo.length = minfo.eeprom_len;
- einfo.data = buf;
+ einfo.data = calloc(1, minfo.eeprom_len);
+ if (!einfo.data) {
+ fprintf(stderr,
+ "Allocation of port %u eeprom data failed\n",
+ port_id);
+ return;
+ }
ret = rte_eth_dev_get_module_eeprom(port_id, &einfo);
if (ret != 0) {
@@ -994,11 +1006,13 @@ port_module_eeprom_display(portid_t port_id)
ret);
break;
}
+ free(einfo.data);
return;
}
rte_hexdump(stdout, "hexdump", einfo.data, einfo.length);
printf("Finish -- Port: %d MODULE EEPROM length: %d bytes\n", port_id, einfo.length);
+ free(einfo.data);
}
int