Coverity report dereference after null check in /src/http/ngx_http_upstream.c

[Maxim Dounin]

Hello!

On Fri, Dec 01, 2017 at 09:42:37PM -0200, Ranier Vf wrote:

> Coverity report dereference after null check in
> /src/http/ngx_http_upstream.c
> 
> CID 400852 (#1 of 1): Dereference after null check (FORWARD_NULL)22.
> var_deref_op: Dereferencing null pointer u->pipe.
> 
> line 4356: ngx_http_file_cache_free(r->cache, u->pipe->temp_file);
> 
> Required add check u->pipe?

It's complicated.

Simply adding a check in a particular place is certainly wrong, as 
r->cache is only expected to exist only if u->pipe exists as well.

On the other hand, in theory it is possible for r->cache to be not 
in sync with "u" in a very specific case of filter finalization 
(combined with caching, and also combined with error_page 
redirection to a different location with upstream), when "u" comes 
from an old upstream connection, and r->cache was already updated 
to work with a new one.  Additional u->pipe checks won't help here 
though, as almost every thing done with r->cache in such a case is 
likely wrong.

One of the possible fixes I've suggested several years ago is to 
move r->cache to upstream structure, so the old "u" will be 
self-consistent, though Igor disagree with such approach.  Not 
sure there are other good solutions though.

The problem is mostly theoretical though, as all practical cases 
work fine.

-- 
Maxim Dounin
http://mdounin.ru/