On 31/10/2020 00:18, Scott Q. wrote:> I have implemented this change and the core dumps went away. > > It appears so far that it's indeed the right fix. > > Has this change been introduced in 2.3.11.3 ?? > > Thanks!I've also been running it for 4 days without any further panics / segfaults and no noticed adverse effects. I'd recommend the dovecot team to consider including the patch in the next update. John diff -ur dovecot-2.3.11.3-orig/src/lib-http/http-client-request.c dovecot-2.3.11.3/src/lib-http/http-client-request.c --- dovecot-2.3.11.3-orig/src/lib-http/http-client-request.c??? 2020-08-12 14:20:41.000000000 +0200 +++ dovecot-2.3.11.3/src/lib-http/http-client-request.c 2020-10-27 13:06:09.352973130 +0100 @@ -1229,12 +1229,12 @@ ??????? const char *error; ??????? uoff_t offset; -?????? i_assert(req->payload_input != NULL); -?????? i_assert(req->payload_output != NULL); - ??????? if (req->payload_finished) ??????????????? return http_client_request_finish_payload_out(req); +?????? i_assert(req->payload_input != NULL); +?????? i_assert(req->payload_output != NULL); + ??????? io_remove(&conn->io_req_payload); ??????? /* chunked ostream needs to write to the parent stream's buffer */ -------------- next part -------------- An HTML attachment was scrubbed... URL: <https://dovecot.org/pipermail/dovecot/attachments/20201031/5b418979/attachment.html>
But do you know how this bug was introduced ? I checked the history of that file and don't see anything introduced recently that might cause this:?https://github.com/dovecot/core/commits/master/src/lib-http/http-client-request.c The only one that might be somewhat related is this one from Apr 27 by Stephan Bosch https://github.com/dovecot/core/commit/799b52accf71e86756dde738d22c1c6a500a7e29#diff-1c02b3481573ffd33c9abccd3f5a6752a5cd81ca83389f4380657f7309c06366 On Saturday, 31/10/2020 at 08:40 John Fawcett wrote: On 31/10/2020 00:18, Scott Q. wrote: I have implemented this change and the core dumps went away. It appears so far that it's indeed the right fix. Has this change been introduced in 2.3.11.3 ?? Thanks! I've also been running it for 4 days without any further panics / segfaults and no noticed adverse effects. I'd recommend the dovecot team to consider including the patch in the next update. John diff -ur dovecot-2.3.11.3-orig/src/lib-http/http-client-request.c dovecot-2.3.11.3/src/lib-http/http-client-request.c --- dovecot-2.3.11.3-orig/src/lib-http/http-client-request.c??? 2020-08-12 14:20:41.000000000 +0200 +++ dovecot-2.3.11.3/src/lib-http/http-client-request.c 2020-10-27 13:06:09.352973130 +0100 @@ -1229,12 +1229,12 @@ ??????? const char *error; ??????? uoff_t offset; -?????? i_assert(req->payload_input != NULL); -?????? i_assert(req->payload_output != NULL); - ??????? if (req->payload_finished) ??????????????? return http_client_request_finish_payload_out(req); +?????? i_assert(req->payload_input != NULL); +?????? i_assert(req->payload_output != NULL); + ??????? io_remove(&conn->io_req_payload); ??????? /* chunked ostream needs to write to the parent stream's buffer */ -------------- next part -------------- An HTML attachment was scrubbed... URL: <https://dovecot.org/pipermail/dovecot/attachments/20201031/0f6df6f9/attachment.html>
On 31/10/2020 15:15, Scott Q. wrote:> But do you know how this bug was introduced ? > > I checked the history of that file and don't see anything introduced > recently that might cause > this:?https://github.com/dovecot/core/commits/master/src/lib-http/http-client-request.c > <https://github.com/dovecot/core/commits/master/src/lib-http/http-client-request.c> > > The only one that might be somewhat related is this one from Apr 27 by > Stephan Bosch > > https://github.com/dovecot/core/commit/799b52accf71e86756dde738d22c1c6a500a7e29#diff-1c02b3481573ffd33c9abccd3f5a6752a5cd81ca83389f4380657f7309c06366 > <https://github.com/dovecot/core/commit/799b52accf71e86756dde738d22c1c6a500a7e29#diff-1c02b3481573ffd33c9abccd3f5a6752a5cd81ca83389f4380657f7309c06366> >I don't know exactly. I'm guess it's not the above commit, since the problem is happening only when payload_finished is TRUE on arriving at the start of the http_client_request_send_more function and those lines set it to FALSE. Though I can't exclude it that it might be an indirect effect of this. There are two places where http_client_request_send_more is called but the problematic call is the one that occurs in http-client-connection.c within the http_client_connection_continue_request function. I wasn't able to identify a specific commit that introduced this problem. The http-client-connection.c file and the whole library has undergone numerous non trivial commits between 2.3.10 and 2.3.11.3 so the answer is not easy to identify by code inspectiion. With the various changes, the code now gets to http_client_request_send_more with payload_finished true but with payload_input NULL whereas in 2.3.10 payload_input was not NULL. (It's a logical deduction from the fact we didn't see the assert failures before and that part of the code has not changed). John -------------- next part -------------- An HTML attachment was scrubbed... URL: <https://dovecot.org/pipermail/dovecot/attachments/20201031/f9d6740c/attachment.html>