Re: Hang in bgp_delete

[Graham Northup]

On 02/17/2017 01:49 PM, Graham Northup wrote:
> On 02/16/2017 11:25 AM, Chet Ramey wrote:
>> On 2/11/17 5:04 PM, Graham Northup wrote:
>>
>>> Bash Version: 4.4
>>> Patch Level: 11
>>> Release Status: release
>>>
>>> Description:
>>>
>>> I'm getting a mysterious hang on one of our Arch Linux machines for a
>>> particular, rather simple script; getting a debugger attached to the
>>> process after building some debugging symbols, I tracked the hang down
>>> to this loop in bgp_delete (with some minor formatting):
>>
>> It seems obvious in retrospect that the cause is in bgp_add, where there's
>> no check for the hashed pid colliding with the index into the pidstat list.
>> Here's a patch that avoids that issue and catches the symptom you found in
>> case the cause is something else.
>>
>> Chet
>>
> 
> Just built bash with the patch applied; I'll be checking in every couple
> of days to see if that happens again. (As I mentioned previously, it's a
> bit hard to instigate; consider no news to be good news :) .
> 
> Thanks for the fix!
> - Graham
> 

I have some bad news for you: the fix works for one-cycles, but I just
found a three-cycle during this entertaining conversation with GDB:

(gdb) gcore
warning: target file /proc/12891/cmdline contained unexpected null
characters
Saved corefile core.12891
(gdb) disp psi
1: psi = 2996
(gdb) n
882           if (psi == bgpids.storage[psi].bucket_next)       /* catch 
reported
bug */
1: psi = 2996
(gdb)
878       for (psi = *(pshash_getbucket (pid)); psi != NO_PIDSTAT; psi =
bgpids.storage[psi].bucket_next)
1: psi = 9967
(gdb)
880           if (bgpids.storage[psi].pid == pid)
1: psi = 9967
(gdb)
882           if (psi == bgpids.storage[psi].bucket_next)       /* catch 
reported
bug */
1: psi = 9967
(gdb)
878       for (psi = *(pshash_getbucket (pid)); psi != NO_PIDSTAT; psi =
bgpids.storage[psi].bucket_next)
1: psi = 3697
(gdb)
880           if (bgpids.storage[psi].pid == pid)
1: psi = 3697
(gdb)
882           if (psi == bgpids.storage[psi].bucket_next)       /* catch 
reported
bug */
1: psi = 3697
(gdb)
878       for (psi = *(pshash_getbucket (pid)); psi != NO_PIDSTAT; psi =
bgpids.storage[psi].bucket_next)
1: psi = 2996

...all the rest of the conditions are the same, modulo the patched bash,
so I'm eliding the details; as before, I left the process running, so
let me know if you need me to poke it--I can also give you that core
dump if you'd like :)

Thanks,
Graham

signature.asc
Description: OpenPGP digital signature