Seg fault in http read event handler caused by rouge event call without context

[Maxim Dounin]

Hello!

On Mon, Nov 18, 2019 at 05:48:46PM +0100, Sergey Brester wrote:

> Looks like [efd71d49bde0 [2]] could be indeed responsible for that:
> 
> I see at least one state where rev->ready could remain 1 (after
> rev->available gets 0) e. g. deviation between blocks
> [efd71d49bde0#l10.8 [3]] and [efd71d49bde0#l11.8 [4]] where first did
> not reset rev->ready and for example if ngx_socket_nread in
> [efd71d49bde0#l10.38 [5]] would write 0 into rev->available, so
> rev->ready remains 1 yet.

There is no deviation here.  The rev->available field holds the 
number of bytes available for reading (or -1 if it's not known), 
while rev->ready indicates if reading is possible.  The reading 
can be possible even when rev->available is zero - notably, when 
there is a potential pending EOF.

> Maybe it should be changed to this one:
> 
>  		if (rev->available == 0 && !rev->pending_eof) {
> 
>  		if (rev->available <= 0 && !rev->pending_eof) { 

No.  Negative rev->available means we don't know how many bytes 
are available, but some are.  It is wrong to stop reading in 
rev->available is negative.

> Also rev->available could remain negative if n != size and
> ngx_readv_chain or ngx_unix_recv wouldn't enter this blocks or if
> ngx_socket_nread failed (returns -1). 

Sure, this is expected.

> And there are some code pices where nginx would expect positive
> ev->available.

If you think you know places which expect only positive 
ev->available, please point them out.

> So I guess either one of this blocks are not fully correct, or perhaps
> the block [efd71d49bde0#l10.28 [6]] could be moved to end of the #if
> (NGX_HAVE_FIONREAD) block (before #endif at least in case
> !rev->pending_eof).

The code in the NGX_HAVE_FIONREAD block does not require any 
particular event method and/or kernel version, so it cannot rely 
on rev->pending_eof being correctly set.

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