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

Sergey Brester serg.brester at sebres.de
Mon Nov 18 16:48:46 UTC 2019 

 

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 at 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.nginx.org/pipermail/nginx-devel/attachments/20191118/36369cfc/attachment-0001.htm>

More information about the nginx-devel mailing list