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.

Maybe it should be changed to this one:

                if (rev->available == 0 && !rev->pending_eof) {

                if (rev->available <= 0 && !rev->pending_eof) { 

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). 
And there are some code pices where nginx would expect positive
ev->available.

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).

Regards,
Sergey.

18.11.2019 15:03, Dave Brennan wrote: 

> For the last few years we have been using the "nginx_upload" module to 
> streamline result posting within our environment. 
> 
> With the introduction of nginx 1.17.5 we saw a large number of segment 
> faults, causing us to revert to 1.17.4 on our development system. 
> 
> While isolating the fault we added an increase in debug messages to monitor 
> the request and context variables being passed to event handlers. 
> 
> So a good response in 1.17.4 looks like this:- 
> 
> 2019/11/14 10:24:21 [debug] 12398#12398: *9770 Upload handle pre alloc 
> Request address = 0000563E9FE451F0 Context = 0000000000000000 
> 
> 2019/11/14 10:24:21 [debug] 12398#12398: *9770 Upload Handler post alloc 
> Request address = 0000563E9FE451F0 Context = 0000563E9FE81CD8 
> 
> 2019/11/14 10:24:21 [debug] 12398#12398: *9770 Upload_eval_path Request 
> address = 0000563E9FE451F0 Context = 0000563E9FE81CD8 
> 
> 2019/11/14 10:24:21 [debug] 12398#12398: *9770 Upload eval state path Request 
> address = 0000563E9FE451F0 Context = 0000563E9FE81CD8 
> 
> 2019/11/14 10:24:21 [debug] 12398#12398: *9770 Upload client read Request 
> address = 0000563E9FE451F0 Context = 0000563E9FE81CD8 
> 
> 2019/11/14 10:24:21 [debug] 12398#12398: *9770 do read upload client Request 
> address = 0000563E9FE451F0 Context = 0000563E9FE81CD8 
> 
> 2019/11/14 10:24:21 [debug] 12398#12398: *9770 process request body Request 
> address = 0000563E9FE451F0 Context = 0000563E9FE81CD8 
> 
> 2019/11/14 10:24:21 [debug] 12398#12398: *9770 Upload variable Request 
> address = 0000563E9FE451F0 Context = 0000563E9FE81CD8 
> 
> 2019/11/14 10:24:21 [debug] 12398#12398: *9770 Upload variable Request 
> address = 0000563E9FE451F0 Context = 0000563E9FE81CD8 
> 
> 2019/11/14 10:24:21 [debug] 12398#12398: *9770 Upload variable Request 
> address = 0000563E9FE451F0 Context = 0000563E9FE81CD8 
> 
> 2019/11/14 10:24:21 [debug] 12398#12398: *9770 Upload variable Request 
> address = 0000563E9FE451F0 Context = 0000563E9FE81CD8 
> 
> 2019/11/14 10:24:21 [debug] 12398#12398: *9770 Upload variable Request 
> address = 0000563E9FE451F0 Context = 0000563E9FE81CD8 
> 
> 2019/11/14 10:24:21 [debug] 12398#12398: *9770 Upload variable Request 
> address = 0000563E9FE451F0 Context = 0000563E9FE81CD8 
> 
> 2019/11/14 10:24:21 [debug] 12398#12398: *9770 Upload variable Request 
> address = 0000563E9FE451F0 Context = 0000563E9FE81CD8 
> 
> 2019/11/14 10:24:21 [debug] 12398#12398: *9770 upload md5 variable Request 
> address = 0000563E9FE451F0 Context = 0000563E9FE81CD8 
> 
> 2019/11/14 10:24:21 [debug] 12398#12398: *9770 Upload variable Request 
> address = 0000563E9FE451F0 Context = 0000563E9FE81CD8 
> 
> 2019/11/14 10:24:21 [debug] 12398#12398: *9770 Upload File size variable 
> Request address = 0000563E9FE451F0 Context = 0000563E9FE81CD8 
> 
> 2019/11/14 10:24:21 [debug] 12398#12398: *9770 Upload Body Handler Request 
> address = 0000563E9FE451F0 Context = 0000563E9FE81CD8 
> 
> In 1.17.5 the event stream looks like this:- 
> 
> 2019/11/13 14:21:52 [debug] 28086#28086: *3703 Upload handle pre alloc 
> Request address = 0000558ADDD4F780 Context = 0000000000000000 
> 
> 2019/11/13 14:21:52 [debug] 28086#28086: *3703 Upload Handler post alloc 
> Request address = 0000558ADDD4F780 Context = 0000558ADDD49FF8 
> 
> 2019/11/13 14:21:52 [debug] 28086#28086: *3703 Upload_eval_path Request 
> address = 0000558ADDD4F780 Context = 0000558ADDD49FF8 
> 
> 2019/11/13 14:21:52 [debug] 28086#28086: *3703 Upload eval state path Request 
> address = 0000558ADDD4F780 Context = 0000558ADDD49FF8 
> 
> 2019/11/13 14:21:52 [debug] 28086#28086: *3703 Upload client read Request 
> address = 0000558ADDD4F780 Context = 0000558ADDD49FF8 
> 
> 2019/11/13 14:21:52 [debug] 28086#28086: *3703 do read upload client Request 
> address = 0000558ADDD4F780 Context = 0000558ADDD49FF8 
> 
> 2019/11/13 14:21:52 [debug] 28086#28086: *3703 process request body Request 
> address = 0000558ADDD4F780 Context = 0000558ADDD49FF8 
> 
> 2019/11/13 14:21:52 [debug] 28086#28086: *3703 Upload variable Request 
> address = 0000558ADDD4F780 Context = 0000558ADDD49FF8 
> 
> 2019/11/13 14:21:52 [debug] 28086#28086: *3703 Upload variable Request 
> address = 0000558ADDD4F780 Context = 0000558ADDD49FF8 
> 
> 2019/11/13 14:21:52 [debug] 28086#28086: *3703 Upload variable Request 
> address = 0000558ADDD4F780 Context = 0000558ADDD49FF8 
> 
> 2019/11/13 14:21:52 [debug] 28086#28086: *3703 Upload variable Request 
> address = 0000558ADDD4F780 Context = 0000558ADDD49FF8 
> 
> 2019/11/13 14:21:52 [debug] 28086#28086: *3703 Upload variable Request 
> address = 0000558ADDD4F780 Context = 0000558ADDD49FF8 
> 
> 2019/11/13 14:21:52 [debug] 28086#28086: *3703 Upload variable Request 
> address = 0000558ADDD4F780 Context = 0000558ADDD49FF8 
> 
> 2019/11/13 14:21:52 [debug] 28086#28086: *3703 process request body Request 
> address = 0000558ADDD4F780 Context = 0000558ADDD49FF8 
> 
> 2019/11/13 14:21:52 [debug] 28086#28086: *3703 Upload variable Request 
> address = 0000558ADDD4F780 Context = 0000558ADDD49FF8 
> 
> 2019/11/13 14:21:52 [debug] 28086#28086: *3703 upload md5 variable Request 
> address = 0000558ADDD4F780 Context = 0000558ADDD49FF8 
> 
> 2019/11/13 14:21:52 [debug] 28086#28086: *3703 Upload variable Request 
> address = 0000558ADDD4F780 Context = 0000558ADDD49FF8 
> 
> 2019/11/13 14:21:52 [debug] 28086#28086: *3703 Upload File size variable 
> Request address = 0000558ADDD4F780 Context = 0000558ADDD49FF8 
> 
> 2019/11/13 14:21:52 [debug] 28086#28086: *3703 Upload Body Handler Request 
> address = 0000558ADDD4F780 Context = 0000558ADDD49FF8 
> 
> 2019/11/13 14:21:52 [DEBUG] 28086#28086: *3703 READ UPLOAD CLENT REQUEST BODY 
> REQUEST ADDRESS = 0000558ADDD4F780 CONTEXT = 0000000000000000 
> 
> 2019/11/13 14:21:52 [DEBUG] 28086#28086: *3703 DO READ UPLOAD CLIENT REQUEST 
> ADDRESS = 0000558ADDD4F780 CONTEXT = 0000000000000000 
> 
> There appears to be an extra call to the request "read event" and although 
> the request address has not changed the context address returned by:- 
> 
> ngx_http_upload_ctx_t *u = ngx_http_get_module_ctx(r, 
> ngx_http_upload_module); 
> 
> Returns NULL, which causes any reference to the context table to cause a 
> segment fault. 
> 
> While it is possible to work round this by checking for a NULL context, the 
> read event appears to be rouge when compared to the previous version of 
> nginx, and I can only assume has been generated by code changes in 1.17.5. 
> 
> Dave Brennan
> Cyber Protection Senior Technologist 
> 
> CORVID Protect Holdings Limited, trading as CORVID Protect, is registered in 
> Guernsey, company number FC034204, whose registered office is at Royal Bank 
> Place, 1 Glategny Esplanade, St Peter Port, Guernsey GY1 4ND. CORVID Protect 
> Holdings Limited is a subsidiary company of Ultra Electronics Holdings plc 
> registered in England and Wales, company number 02830397, whose registered 
> office is at 35 Portman Square, London W1H 6LR. 
> 
> Ultra Electronics is committed to safeguarding the privacy of all personal 
> data: data privacy notice. Email communications may be monitored by us, as 
> permitted by applicable law and regulations. This email is confidential and 
> may also be privileged. If you have received this message in error you should 
> notify the sender immediately by reply e-mail and delete the message from 
> your system.
> 
> _______________________________________________
> nginx-devel mailing list
> nginx-devel@nginx.org
> http://mailman.nginx.org/mailman/listinfo/nginx-devel [1]
 

Links:
------
[1] http://mailman.nginx.org/mailman/listinfo/nginx-devel
[2] https://hg.nginx.org/nginx/rev/efd71d49bde0
[3] https://hg.nginx.org/nginx/rev/efd71d49bde0#l10.8
[4] https://hg.nginx.org/nginx/rev/efd71d49bde0#l11.8
[5] https://hg.nginx.org/nginx/rev/efd71d49bde0#l10.38
[6] https://hg.nginx.org/nginx/rev/efd71d49bde0#l10.28
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel

Reply via email to