Hello:
I am so sorry. my code is not the newest code.
Thank you very much for taking time with me.
My code is
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);
#if ENABLE_FEATURE_INIT_SYSLOG
msg[l] = '\0';
if (where & L_LOG) {
/* Log the message to syslogd */
openlog(applet_name, 0, LOG_DAEMON);
/* don't print "\r" */
syslog(LOG_INFO, "%s", msg + 1);
closelog();
}
msg[l++] = '\n';
msg[l] = '\0';
Cheers,
chen jie
On 2015/8/31 20:08, Xabier Oneca -- xOneca wrote:
> 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
