Le 22/06/2020 à 10:56, Emeric Brun a écrit :
Hi Daniel, Willy,
On 6/19/20 9:22 PM, Willy Tarreau wrote:
Hi Daniel,
On Thu, Jun 18, 2020 at 12:35:29AM -0400, Daniel Corbett wrote:
Hello,
When using a ring log in combination with proto fcgi, it was possible
to cause a crash by sending a request for a non-existent fastcgi file
to php-fpm, causing it to produce the error "Primary script unknown".
When php-fpm produces this error for some reason a sink is not able to be
established and __do_send_log gets passed a null sink.
This commit verifies that the sink exists in __do_send_log before attempting
to use it.
Thanks for the fix and the detailed report, that's very useful! However
the problem is somewhere else, and I suspect is slightly harder to solve.
It's normally not possible to have a null sink on a log server if its
type is a buffer, so we have an inconsistency somewhere that we must
address.
What I'm suspecting (but I could be wrong) is that the fcgi duplicates
the global log settings before they are resolved, then the global ones
are resolved, and the copy in the fcgi part keeps an incompletely
initialized log server. I haven't looked at this area yet since we
support post-check resolution of sink names, but I suspect that it's
applied to frontends only, while as can be seen in your examples there
seem to be other areas requiring log resolution (I wasn't even aware
that we had other log references). So this means that right now we
have to make sure they're all properly addressed, and that in the
future it might be worth keeping a pointer to the global log servers
instead of copying them.
Indeed, we currently resolve ring names doing a loop on proxies to initialize
their
server lists but I ignored that in the case of fastcgi-app, there is a
log-server list
not related to a proxy, I've just talk to Christopher and he told me he already
planned to submit a fix for this bug, initializing correctly those server lists
on
fastcgi-app's configuration post parsing .
I pushed a fix (commit c97406f7). Thanks Daniel !
--
Christopher Faulet
