Menu

#2675 Double file descriptor close while reading interface info

backport-needed
closed
None
5
2015-11-24
2015-10-20
Vit Zikmund
No

Hi.
After a few months chasing a particular class of instabilities caused by a "wild" file descriptor close in our high-concurrency networking product, we've finally found one of the culprits.

Please see the "Address_Scan_Init(void)" function in /agent/mibgroups/mibII/ipAddr.c on any of the supported releases (5.7.3 (LTS), 5.6.2.1, 5.5.2.1, 5.4.4 (LTS)) for this construct:

int fd;
// ...
do
{
    if ((fd = socket(AF_INET, SOCK_DGRAM, 0)) < 0)
    {
        // handle error
        return;
    }
    // get some info for the fd
    close(fd);
}
while(/*whatever retry condition*/);
// ...
close(fd); // <<--- hello there, mr. killer

Notice that inside the do-while loop, the fd is both opened (socket()) and closed, so its lifetime is perfectly set to the scope of the loop. Nevertheless, there's another call to close() outside the loop at the end of the function. This call not only is superfluous, but is downright devastating for a multi-threaded program.

While in a single-threaded program this only causes the close to fail with EBADF, on multi-threaded program it has the potential to close an already reopened FD that belongs to a different thread. As the pool of open file descriptors is global for the whole process, this can cause a lot of weird and hard to diagnose problems.

The fix is trivial - remove the last call to close() and all sorts of cryptic problems go away.

In fact, it has been already fixed on the master branch (as far as I noticed) as a side-effect of Commit [[2d788f]](http://sourceforge.net/p/net-snmp/code/ci/2d788f25ee9422f2220f1218b7a50c03852154cd/)

Not knowing anything about your maintenance process I'll leave this information for someone of you to handle the fix the way you find most suitable. Thank you.

Vit

Discussion

  • Niels Baggesen

    Niels Baggesen - 2015-11-24
    • status: open --> closed
    • assigned_to: Niels Baggesen
     
  • Niels Baggesen

    Niels Baggesen - 2015-11-24

    Thank for the patch! It has been applied toall active branches.

     

Log in to post a comment.