change log for rtems (2011-03-15)

[Ralf Corsepius]

On 03/15/2011 04:34 PM, Joel Sherrill wrote:
> On 03/15/2011 10:27 AM, Ralf Corsepius wrote:
>> On 03/15/2011 04:10 PM, rtems-vc at rtems.org wrote:
>>
>>> 2011-03-15 Joel Sherrill<joel.sherrill at oarcorp.com>
>>>
>>> * libmisc/shell/main_setenv.c: Address Coverity issue 134 which
>>> recommends using strnXXX methods.
>>>
>>> M 1.2778 cpukit/ChangeLog
>>> M 1.4 cpukit/libmisc/shell/main_setenv.c
>>>
>>> diff -u rtems/cpukit/ChangeLog:1.2777 rtems/cpukit/ChangeLog:1.2778
>>> --- rtems/cpukit/ChangeLog:1.2777 Tue Mar 15 09:52:31 2011
>>> +++ rtems/cpukit/ChangeLog Tue Mar 15 10:02:41 2011
>>> @@ -1,3 +1,8 @@
>>> +2011-03-15 Joel Sherrill<joel.sherrill at oarcorp.com>
>>> +
>>> + * libmisc/shell/main_setenv.c: Address Coverity issue 134 which
>>> + recommends using strnXXX methods.
>>> +
>>> 2011-03-15 Sebastian Huber<sebastian.huber at embedded-brains.de>
>>>
>>> * libmisc/cpuuse/cpuusagereport.c: Avoid assumptions on execution
>>>
>>> diff -u rtems/cpukit/libmisc/shell/main_setenv.c:1.3
>>> rtems/cpukit/libmisc/shell/main_setenv.c:1.4
>>> --- rtems/cpukit/libmisc/shell/main_setenv.c:1.3 Sat Aug 28 15:10:00
>>> 2010
>>> +++ rtems/cpukit/libmisc/shell/main_setenv.c Tue Mar 15 10:02:41 2011
>>> @@ -16,6 +16,13 @@
>>> #include<rtems/shell.h>
>>> #include "internal.h"
>>>
>>> +/*
>>> + * Limit examining or copying more than 256 characters at a time.
>>> + * Yes, this is very arbitrary. If there are POSIX constants, then
>>> + * they should be used.
>>> + */
>>> +#define MAX 156
>>> +
>>> int rtems_shell_main_setenv(int argc, char *argv[])
>>> {
>>> char* env = NULL;
>>> @@ -32,7 +39,7 @@
>>> env = argv[1];
>>>
>>> for (arg = 2; arg< argc; arg++)
>>> - len += strlen(argv[arg]);
>>> + len += strnlen(argv[arg], MAX);
>>>
>>> len += argc - 2 - 1;
>>>
>>> @@ -44,8 +51,8 @@
>>> }
>>>
>>> for (arg = 2, p = string; arg< argc; arg++) {
>>> - strcpy(p, argv[arg]);
>>> - p += strlen(argv[arg]);
>>> + strncpy(p, argv[arg], MAX);
>>> + p += strnlen(argv[arg], MAX);
>>> if (arg< (argc - 1)) {
>>> *p = ' ';
>>> p++;
>>>
>> argv is defined as being 0 terminated
>> => using strnlen is wrong.
>> => Coverity is wrong
>>
>> Also, by limiting the string you are
>> * causing bugs
>> * introducing memory bloat (a static buffer).
>>
> There are no static buffers and none were introduced.
OK.

>> IMO this patch is simply plain wrong and should be reverted.

Think about "calling rtems_shell_main_setenv with a very long argument.

With your changes applied the code will silently misbehave.