<html><head><meta http-equiv="Content-Type" content="text/html charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; ">Hello,<div><br></div><div>I have hit upon a case where some ntlm_auth processes would return (and write the NT_KEY to the connecting pipe) but FR still complains that it failed and denies authentication (this is on 2.2.5).</div><div><br></div><div>This manifests in the logs like the following:</div><div><br></div><div><font face="Courier">Tue Oct 28 11:10:15 2014 : Auth: Login incorrect (mschap: External script says NT_KEY: 4BCE6CA72058BA7EE500D1A68A8771C0): [tstRad9] (from client 155.98.204.47 port 0 cli 02-00-00-00-00-01 via TLS tunnel)</font></div><div><br></div><div><br></div><div>Since this is actually the output for a valid and successful authentication, it appears that the exit code is the real issue.</div><div>That exit code is either that of the process itself or 2 if rad_waitpid times out while waiting for the child.</div><div><br></div><div>After adding some debugging statements and recompiling I found that there were cases where reap_children would reap a child process but the pid would not be found in thread_pool.waiters. This only happened when there were a significant number of auths per seconds and still not consistently. Some head scratching ensued and a colleague then suggested there may be a race condition between rad_fork (where it calls fr_hash_table_insert) and reap_children (where it calls fr_hash_table_finddata).</div><div><br></div><div>So I wrote a quick patch which I submit to your consideration. It adds the pid to the hash table if not found.</div><div>In our (admittedly limited) tests this has fixed the issue. No more ntlm_auth waiting ten seconds before timing out. </div><div>I don't claim for this patch to be perfect. For one thing it causes an error to be logged by rad_fork (" Failed to store PID, creating what will be a zombie process") when the pid is added to the hash table in reap_children. </div><div><br></div><div>There may be better ways to do this, perhaps simply with better locking in rad_fork. This is only meant as a fix while the issue is discussed and improved upon.</div><div><br></div><div><br></div><div></div></body></html>