Hi Chenjie,
2015-08-31 13:48 GMT+02:00 chenjie <[email protected]>:
> Hello Xabier Oneca:
>
> The test code just explain the code by a simple code.
> It is not a busybox test case.
Yes, I know. I should have specified what I meant.
In your test case you have
if (l > sizeof(msg) - 1)
l = sizeof(msg) - 1;
While in the code removed by the patch the same line says
if (l > sizeof(msg) - 2)
l = sizeof(msg) - 2;
So your test case doesn't test *actual* Busybox code.
> Original code will lead to msg[128],indeed.
I've been halve an hour trying to understand why original code was
wrong until I saw differences between your test case and Busybox code!
Also, note that vsnprintf needs an extra char for final \0, so
"sizeof(msg) - 2" is ok.
> On 2015/8/31 19:30, Xabier Oneca -- xOneca wrote:
>> Hello Chenjie,
>>
>> Your test case does not match the previous code of message of the
>> patch, and I think your patch is not necessary.
>>
>> Cheers,
>>
>> Xabier Oneca_,,_
>>
>> 2015-08-31 19:55 GMT+02:00 <[email protected]>:
>>> From: chenjie <[email protected]>
>>>
>>> The message function will lead to a buffer overflow.
>>> The test case like this:
>>> #include <stdio.h>
>>> #include <string.h>
>>> #include <stdarg.h>
>>> #include <stdlib.h>
>>> void message(int where, const char *fmt, ...){
>>> va_list arguments;
>>> unsigned l;
>>> char msg[128];
>>>
>>> msg[0] = '\r';
>>> va_start(arguments, fmt);
>>> l = 1 + vsnprintf(msg + 1, sizeof(msg) - 2, fmt, arguments);
>>> if (l > sizeof(msg) - 1)
>>> l = sizeof(msg) - 1;
>>> va_end(arguments);
>>>
>>> msg[l] = '\0';
>>> msg[l++] = '\n';
>>> printf("l is lenth %d\n",l);
>>> msg[l] = '\0';
>>> }
>>>
>>>
>>> int main(){
>>> char *arguments = "/usr/sbin/syslog-ng -f
>>> /etc/syslog-ng/syslog-ng.conf -p /var/run/syslogd.pid -F";
>>> message(1, "process '%s' (pid 1234) exited. "
>>> "Scheduling for restart.",
>>> arguments);
>>> }
>>>
>>> we can see msg[128]='\0' but this is wrong.The arguments
>>> which we can find in the /etc/inittab.
>>>
>>> Signed-off-by: Chen Jie <[email protected]>
>>> ---
>>> init/init.c | 6 +++---
>>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/init/init.c b/init/init.c
>>> index b2fe856..b8f2e73 100644
>>> --- a/init/init.c
>>> +++ b/init/init.c
>>> @@ -221,9 +221,9 @@ static void message(int where, const char *fmt, ...)
>>>
>>> msg[0] = '\r';
>>> va_start(arguments, fmt);
>>> - l = 1 + vsnprintf(msg + 1, sizeof(msg) - 2, fmt, arguments);
>>> - if (l > sizeof(msg) - 2)
>>> - l = sizeof(msg) - 2;
>>> + l = 1 + vsnprintf(msg + 1, sizeof(msg) - 3, fmt, arguments);
>>> + if (l > sizeof(msg) - 3)
>>> + l = sizeof(msg) - 3;
>>> va_end(arguments);
>>>
>>> #if ENABLE_FEATURE_INIT_SYSLOG
>>> --
>>> 1.8.0
Cheers,
Xabier Oneca_,,_
_______________________________________________
busybox mailing list
[email protected]
http://lists.busybox.net/mailman/listinfo/busybox
