diff mbox

validation: tm: use strncmp() to avoid potential string overrun

Message ID 01000156523a42d8-6b5237b5-82af-4bd3-9463-69c14865c7d8-000000@email.amazonses.com
State Accepted
Commit 3eb700768b6369e674e5d5a9758eb39f8fd34c3d
Headers show

Commit Message

Bill Fischofer Aug. 3, 2016, 9:06 p.m. UTC 
Resolve Bug https://bugs.linaro.org/show_bug.cgi?id=2426 by switching from
strcmp() to strncmp()

Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org>

---
 test/common_plat/validation/api/traffic_mngr/traffic_mngr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
2.7.4

Comments

Mike Holmes Aug. 4, 2016, 3:23 p.m. UTC | #1 
On 3 August 2016 at 17:06, Bill Fischofer <bill.fischofer@linaro.org> wrote:

> Resolve Bug https://bugs.linaro.org/show_bug.cgi?id=2426 by switching from

> strcmp() to strncmp()

>

> Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org>

>


Reviewd-by: Mike Holmes <mike.holmes@linaro.org>

This raises a question on how many name lengths we need, maybe all the
public API ones can be merged  ?

test/common_plat/validation/api/traffic_mngr/traffic_mngr.c:#define
TM_NAME_LEN              32
helper/include/odp/helper/table.h:#define ODPH_TABLE_NAME_LEN      32
include/odp/api/spec/pool.h:#define ODP_POOL_NAME_LEN  32
include/odp/api/spec/shared_memory.h:#define ODP_SHM_NAME_LEN 32
include/odp/api/spec/timer.h:#define ODP_TIMER_POOL_NAME_LEN  32
platform/linux-generic/include/odp/api/plat/classification_types.h:#define
ODP_COS_NAME_LEN 32
platform/linux-generic/include/odp/api/plat/queue_types.h:#define
ODP_QUEUE_NAME_LEN 32
platform/linux-generic/include/odp/api/plat/schedule_types.h:#define
ODP_SCHED_GROUP_NAME_LEN 32
platform/linux-generic/include/odp_name_table_internal.h:#define
_ODP_INT_NAME_LEN 32
platform/linux-generic/include/odp_packet_io_internal.h:#define
PKTIO_NAME_LEN 256



---
>  test/common_plat/validation/api/traffic_mngr/traffic_mngr.c | 2 +-

>  1 file changed, 1 insertion(+), 1 deletion(-)

>

> diff --git a/test/common_plat/validation/api/traffic_mngr/traffic_mngr.c

> b/test/common_plat/validation/api/traffic_mngr/traffic_mngr.c

> index b857800..c7bde40 100644

> --- a/test/common_plat/validation/api/traffic_mngr/traffic_mngr.c

> +++ b/test/common_plat/validation/api/traffic_mngr/traffic_mngr.c

> @@ -1507,7 +1507,7 @@ static tm_node_desc_t *find_node_desc(uint8_t

>  tm_system_idx,

>                 name_ptr++;

>

>         while (node_desc != NULL) {

> -               if (strcmp(node_desc->node_name, node_name) == 0)

> +               if (strncmp(node_desc->node_name, node_name, TM_NAME_LEN)

> == 0)

>                         return node_desc;

>

>                 if (name_ptr == NULL)

> --

> 2.7.4

>

>



-- 
Mike Holmes
Technical Manager - Linaro Networking Group
Linaro.org <http://www.linaro.org/> *│ *Open source software for ARM SoCs
"Work should be fun and collaborative, the rest follows"
Bill Fischofer Aug. 4, 2016, 5:53 p.m. UTC | #2 
On Thu, Aug 4, 2016 at 10:23 AM, Mike Holmes <mike.holmes@linaro.org> wrote:

>

>

> On 3 August 2016 at 17:06, Bill Fischofer <bill.fischofer@linaro.org>

> wrote:

>

>> Resolve Bug https://bugs.linaro.org/show_bug.cgi?id=2426 by switching

>> from

>> strcmp() to strncmp()

>>

>> Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org>

>>

>

> Reviewd-by: Mike Holmes <mike.holmes@linaro.org>

>

> This raises a question on how many name lengths we need, maybe all the

> public API ones can be merged  ?

>

> test/common_plat/validation/api/traffic_mngr/traffic_mngr.c:#define

> TM_NAME_LEN              32

> helper/include/odp/helper/table.h:#define ODPH_TABLE_NAME_LEN      32

> include/odp/api/spec/pool.h:#define ODP_POOL_NAME_LEN  32

> include/odp/api/spec/shared_memory.h:#define ODP_SHM_NAME_LEN 32

> include/odp/api/spec/timer.h:#define ODP_TIMER_POOL_NAME_LEN  32

> platform/linux-generic/include/odp/api/plat/classification_types.h:#define

> ODP_COS_NAME_LEN 32

> platform/linux-generic/include/odp/api/plat/queue_types.h:#define

> ODP_QUEUE_NAME_LEN 32

> platform/linux-generic/include/odp/api/plat/schedule_types.h:#define

> ODP_SCHED_GROUP_NAME_LEN 32

> platform/linux-generic/include/odp_name_table_internal.h:#define

> _ODP_INT_NAME_LEN 32

> platform/linux-generic/include/odp_packet_io_internal.h:#define

> PKTIO_NAME_LEN 256

>


These should probably all be made internal and moved to the various
odp_xxx_capability() APIs for consistency. A possible cleanup item for
Tiger Moth? We should get Petri's opinion when he returns.


>

>

>

> ---

>>  test/common_plat/validation/api/traffic_mngr/traffic_mngr.c | 2 +-

>>  1 file changed, 1 insertion(+), 1 deletion(-)

>>

>> diff --git a/test/common_plat/validation/api/traffic_mngr/traffic_mngr.c

>> b/test/common_plat/validation/api/traffic_mngr/traffic_mngr.c

>> index b857800..c7bde40 100644

>> --- a/test/common_plat/validation/api/traffic_mngr/traffic_mngr.c

>> +++ b/test/common_plat/validation/api/traffic_mngr/traffic_mngr.c

>> @@ -1507,7 +1507,7 @@ static tm_node_desc_t *find_node_desc(uint8_t

>>  tm_system_idx,

>>                 name_ptr++;

>>

>>         while (node_desc != NULL) {

>> -               if (strcmp(node_desc->node_name, node_name) == 0)

>> +               if (strncmp(node_desc->node_name, node_name, TM_NAME_LEN)

>> == 0)

>>                         return node_desc;

>>

>>                 if (name_ptr == NULL)

>> --

>> 2.7.4

>>

>>

>

>

> --

> Mike Holmes

> Technical Manager - Linaro Networking Group

> Linaro.org <http://www.linaro.org/> *│ *Open source software for ARM SoCs

> "Work should be fun and collaborative, the rest follows"

>

>

>
Maxim Uvarov Aug. 5, 2016, 3:47 p.m. UTC | #3 
Merged,
Maxim.

On 08/04/16 20:53, Bill Fischofer wrote:
> On Thu, Aug 4, 2016 at 10:23 AM, Mike Holmes <mike.holmes@linaro.org> wrote:

>

>>

>> On 3 August 2016 at 17:06, Bill Fischofer <bill.fischofer@linaro.org>

>> wrote:

>>

>>> Resolve Bug https://bugs.linaro.org/show_bug.cgi?id=2426 by switching

>>> from

>>> strcmp() to strncmp()

>>>

>>> Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org>

>>>

>> Reviewd-by: Mike Holmes <mike.holmes@linaro.org>

>>

>> This raises a question on how many name lengths we need, maybe all the

>> public API ones can be merged  ?

>>

>> test/common_plat/validation/api/traffic_mngr/traffic_mngr.c:#define

>> TM_NAME_LEN              32

>> helper/include/odp/helper/table.h:#define ODPH_TABLE_NAME_LEN      32

>> include/odp/api/spec/pool.h:#define ODP_POOL_NAME_LEN  32

>> include/odp/api/spec/shared_memory.h:#define ODP_SHM_NAME_LEN 32

>> include/odp/api/spec/timer.h:#define ODP_TIMER_POOL_NAME_LEN  32

>> platform/linux-generic/include/odp/api/plat/classification_types.h:#define

>> ODP_COS_NAME_LEN 32

>> platform/linux-generic/include/odp/api/plat/queue_types.h:#define

>> ODP_QUEUE_NAME_LEN 32

>> platform/linux-generic/include/odp/api/plat/schedule_types.h:#define

>> ODP_SCHED_GROUP_NAME_LEN 32

>> platform/linux-generic/include/odp_name_table_internal.h:#define

>> _ODP_INT_NAME_LEN 32

>> platform/linux-generic/include/odp_packet_io_internal.h:#define

>> PKTIO_NAME_LEN 256

>>

> These should probably all be made internal and moved to the various

> odp_xxx_capability() APIs for consistency. A possible cleanup item for

> Tiger Moth? We should get Petri's opinion when he returns.

>

>

>>

>>

>> ---

>>>   test/common_plat/validation/api/traffic_mngr/traffic_mngr.c | 2 +-

>>>   1 file changed, 1 insertion(+), 1 deletion(-)

>>>

>>> diff --git a/test/common_plat/validation/api/traffic_mngr/traffic_mngr.c

>>> b/test/common_plat/validation/api/traffic_mngr/traffic_mngr.c

>>> index b857800..c7bde40 100644

>>> --- a/test/common_plat/validation/api/traffic_mngr/traffic_mngr.c

>>> +++ b/test/common_plat/validation/api/traffic_mngr/traffic_mngr.c

>>> @@ -1507,7 +1507,7 @@ static tm_node_desc_t *find_node_desc(uint8_t

>>>   tm_system_idx,

>>>                  name_ptr++;

>>>

>>>          while (node_desc != NULL) {

>>> -               if (strcmp(node_desc->node_name, node_name) == 0)

>>> +               if (strncmp(node_desc->node_name, node_name, TM_NAME_LEN)

>>> == 0)

>>>                          return node_desc;

>>>

>>>                  if (name_ptr == NULL)

>>> --

>>> 2.7.4

>>>

>>>

>>

>> --

>> Mike Holmes

>> Technical Manager - Linaro Networking Group

>> Linaro.org <http://www.linaro.org/> *│ *Open source software for ARM SoCs

>> "Work should be fun and collaborative, the rest follows"

>>

>>

>>
diff mbox

Patch

diff --git a/test/common_plat/validation/api/traffic_mngr/traffic_mngr.c b/test/common_plat/validation/api/traffic_mngr/traffic_mngr.c
index b857800..c7bde40 100644
--- a/test/common_plat/validation/api/traffic_mngr/traffic_mngr.c
+++ b/test/common_plat/validation/api/traffic_mngr/traffic_mngr.c
@@ -1507,7 +1507,7 @@  static tm_node_desc_t *find_node_desc(uint8_t     tm_system_idx,
 		name_ptr++;
 
 	while (node_desc != NULL) {
-		if (strcmp(node_desc->node_name, node_name) == 0)
+		if (strncmp(node_desc->node_name, node_name, TM_NAME_LEN) == 0)
 			return node_desc;
 
 		if (name_ptr == NULL)