bug report for nginx version: nginx/1.3.6

[Vladimir Shebordaev]

On 05.10.2012 12:05, Wang Tiefeng wrote:
> Hi!
>       When buf > last - 50， buf (= last - 50) is an invalid
> memory address. And the follow lines write on this invalid memoy.
>   AIthough, bufs for log in nginx are all bigger than 50, the
> function does not depend on this. At least , I think this
> funciton is not robust。
>

Usually, nginx'es "last" pointers indicate the memory location 
right after the end of a buffer, so you'd better look at the 
entire context to take the code semantics into account instead of 
being that overcautious about every code snippet like valgrind do.


Hope it helps.


Regards,
Vladimir


> 2012/10/4 Maxim Dounin <mdounin at mdounin.ru
> <mailto:mdounin at mdounin.ru>>
>
>     Hello!
>
>     On Thu, Oct 04, 2012 at 06:42:42PM +0800, Wang Tiefeng wrote:
>
>      > Recently，I start to read nginx source code.
>      > I chose nginx/1.3.6 a relatively new version。
>      >
>      > When I read file ngx_log.c, the function ngx_log_errno()
>     confused me .
>      >
>      > There may be some bugs in the following codes :
>      >  238     if (buf > last - 50) {
>      > 239
>      > 240         /* leave a space for an error code */
>      > 241
>      > 242         buf = last - 50;
>      > 243         *buf++ = '.';
>      > 244         *buf++ = '.';
>      > 245         *buf++ = '.';
>      > 246     }
>      >
>      > Althoug，I am not sure about my judgment，valgrind reports
>     invalid write on
>      > line 243.
>
>     See no problem here.  The code depends on the fact that the
>     buffer
>     used for printing errors is at least 50 bytes long, and the
>     "last"
>     pointer marks it's end, but it looks perfectly safe as long as
>     ngx_log_errno() is used correctly.
>
>     --
>     Maxim Dounin
>     http://nginx.com/support.html
>