squid-1744
Version:
Squid 2.6.Stable3
Bug link:
http://bugs.squid-cache.org/show_bug.cgi?id=1744
How it is diagnosed (reproduced or source analysis)?
We did not reproduce this failure. Just source code analysis..
Symptom:
Crash due to assertion assertion failed: helper.c:324: "!srv->request".
An interesting thing is when developers asked for debug level log, the failure disappeared. With -d debugging option, it can’t be reproduce since the bug is under the statement “if (XX && debug_enabled) initialization”. So, without debugging msg, the bug can be reproduced but it’s difficult to diagnose.
Root cause:
Uninitialized pointer “srv->request”.
helperStatefulReleaseServer(helper_stateful_server * srv) {
debug(84, 5) ("helperStatefulReleaseServer: %p\n", srv);
/* Where the assertion fails! */
assert(!srv->request);
/* This helperStatefulReset will dereference srv->request*/
helperStatefulReset(srv);
}
Developers asked for stack trace, configuration file and logs to diagnose the failure. With the stack trace, eventually developers traced that the reason ‘srv->request’ was not initialized was because a variable value ‘decoded’ was not set correctly. Below is the propagation of this bug to the failure.
main () {
… ...
while (fgets(buf, BUFFER_SIZE, stdin) != NULL) {
- if ((strlen(buf) > 3) && NTLM_packet_debug_enabled) {
+ if (strlen(buf) > 3)
decoded = base64_decode(buf + 3);
+ if ((strlen(buf) > 3) && NTLM_packet_debug_enabled) {
initialization for debugging
}
…
}
authenticateNTLMReleaseServer(ntlm_request_t * ntlm_request) {
helper_stateful_server *server = ntlm_request->authserver;
// ntlm_request->authserver->request == NULL
debug(29, 9) ("authenticateNTLMReleaseServer: releasing server '%p'\n", server);
helperStatefulReleaseServer(server); // server->request
}
authenticateNTLMHandleReply(void *data, void *srv, char *reply){
// r->data, mtlm_request->authserver, NULL
if (!reply) { /* this debugging msg was printed */
debug(29, 1) ("AuthenticateNTLMHandleReply: Helper '%p' crashed!.\n", srv);
reply = (char *) "BH Internal error";
}
..
ntlm_request->authserver = srv;
// srv->request == NULL
if (strncasecmp(reply, "BH ", 3) == 0) {
authenticateNTLMReleaseServer(ntlm_request); <-- crash!!!!
// ntlm_request->authserver->request == NULL
debug(29, 1) ("authenticateNTLMHandleReply: Error validating user via NTLM. Error returned '%s'\n", reply); /* before printing this debug msg, assert happened */
}
}
static void
helperStatefulDispatch(helper_stateful_server * srv, helper_stateful_request * r)
{
if (!r->buf) {
if (cbdataValid(r->data)) {
r->callback(r->data, srv, NULL); //authenticateNTLMHandleReply(r->data, ntlm_request->authserver, NULL)
}
return;
}
srv->request = r; /* this path should be taken */
comm_write(srv->wfd, r->buf, wtrlen(r->buf)...)
debug(84, 5) ("helperStatefulDispatch: Request sent to %s #%d, %d bytes\n",
hlp->id_name, srv->index + 1, (int) strlen(r->buf));
/* this request seems to be sent to fakeauth_auth
}
helperStatefulSubmit(statefulhelper * hlp, const char *buf, HLPSCB * callback, void *data, helper_stateful_server * srv)
{
r->callback = callback;
r->data = data;
if (buf)
r->buf = xstrdup(buf);
if (!srv)
srv = helperStatefulGetServer(hlp);
if (srv) {
assert(!srv->request);
assert(!srv->flags.busy);
helperStatefulDispatch(srv, r);
}
/* this bug is exposed when r->buf != NULL, and helperStatefulDispatch successfully set srv->request */
}
/* send the initial data to a stateful ntlm authenticator module */
authenticateNTLMStart(auth_user_request_t * auth_user_request, RH * handler, void *data)
{
if (ntlm_request->auth_state == AUTHENTICATE_STATE_INITIAL) {
snprintf(buf, 8192, "YR %s\n", sent_string);
} else {
snprintf(buf, 8192, "KK %s\n", sent_string);
}
helperStatefulSubmit(ntlmauthenticators, buf, authenticateNTLMHandleReply, r, ntlm_request->authserver);
}
Is there Error Message?
Yes.
Can Errlog anticipate this error message?
Yes. This pattern falls into memory safety check, that checks for NULL before access. So even without the assertion, Errlog is able to automatically insert it.
Also, it would be very useful to print the stack trace with this abort (as requested by the developers).