Project

General

Profile

Actions

Bug #15356

closed

closefrom(INT_MAX) misbehaves and closes all descriptors

Added by Bill Sommerfeld over 1 year ago. Updated over 1 year ago.

Status:
Closed
Priority:
Normal
Category:
lib - userland libraries
Start date:
Due date:
% Done:

100%

Estimated time:
Difficulty:
Bite-size
Tags:
Gerrit CR:
External Bug:

Description

Discovered while running gmake check on a build of xapian-core, which is available at https://xapian.org

xapian includes an implementation of closefrom() for systems that don't have it, and includes a unit test to exercise it, and they helpfully test the system closefrom() if that's what they're using.

The C++ test code that triggers this can be seen at https://github.com/xapian/xapian/blob/master/xapian-core/tests/unittest.cc#L533-L555

The cause is obvious in usr/src/lib/libc/port/gen/closefrom.c:

void
closefrom(int lowfd)
{
        int low = (lowfd < 0)? 0 : lowfd;

        /*
         * Close lowfd right away as a hedge against failing
         * to open the /proc file descriptor directory due
         * all file descriptors being currently used up.
         */
        (void) close(low++);
        (void) fdwalk(void_close, &low);
}

If lowfd is equal to INT_MAX, then low++ results in an integer overflow, and (on typical systems at least) sets low to INT_MIN which is less than any valid fd, so all open descriptors are closed. I believe the fix is to change low++ to low.

Actions #1

Updated by Bill Sommerfeld over 1 year ago

  • Status changed from New to In Progress
Actions #2

Updated by Bill Sommerfeld over 1 year ago

  • Subject changed from closefrom(INT_MAX) misbehaves and closes all descriptors. to closefrom(INT_MAX) misbehaves and closes all descriptors
Actions #3

Updated by Bill Sommerfeld over 1 year ago

  • Assignee set to Bill Sommerfeld
Actions #4

Updated by Electric Monk over 1 year ago

  • Gerrit CR set to 2619
Actions #5

Updated by Bill Sommerfeld over 1 year ago

Testing notes:
- ran both flavors of new closefrom test manually with LD_PRELOAD'ed libc both with and without the fix and got expected fails w/o fix and pass with fix.
- installed a nightly DEBUG build via /opt/onbld/bin/onu in a VM
- rebooted the VM
- installed gcc-7 and system/test/libctest
- ran /opt/libc-tests/bin/libctest; got:
...

Test: /opt/libc-tests/tests/catopen (run as root)                 [00:00] [PASS]
Test: /opt/libc-tests/tests/closefrom.32 (run as root)            [00:00] [PASS]
Test: /opt/libc-tests/tests/closefrom.64 (run as root)            [00:00] [PASS]
Test: /opt/libc-tests/tests/endian.32 (run as root)               [00:00] [PASS]
...
Results Summary
PASS     128

Running Time:   00:08:22
Percent passed: 100.0%
Log directory:  /var/tmp/test_results/20230207T170418

Actions #6

Updated by Bill Sommerfeld over 1 year ago

  • Status changed from In Progress to Pending RTI
Actions #7

Updated by Electric Monk over 1 year ago

  • Status changed from Pending RTI to Closed
  • % Done changed from 0 to 100

git commit a87ab1ea7c1dcd4c11e5daaee1132ac6a23e1296

commit  a87ab1ea7c1dcd4c11e5daaee1132ac6a23e1296
Author: Bill Sommerfeld <sommerfeld@alum.mit.edu>
Date:   2023-02-07T19:36:45.000Z

    15356 closefrom(INT_MAX) misbehaves and closes all descriptors
    Reviewed by: Andy Fiddaman <illumos@fiddaman.net>
    Reviewed by: Marco van Wieringen <marco.van.wieringen@planets.elm.net>
    Approved by: Dan McDonald <danmcd@mnx.io>

Actions

Also available in: Atom PDF