bin/41332: paxctl(8) leaks file descriptors

[jmiller%myipplus.net@localhost]

>Number:         41332
>Category:       bin
>Synopsis:       paxctl(8) leaks file descriptors
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    bin-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sat May 02 05:10:00 +0000 2009
>Originator:     Jason V. Miller
>Release:        custom build netbsd-5
>Organization:
>Environment:
NetBSD lust 5.0 NetBSD 5.0 (LUST) #0: Thu Apr 30 14:47:02 MDT 2009  
src@lust:/home/src/cvs/netbsd/netbsd-5/obj/sys/arch/i386/compile/LUST i386

>Description:
After open() succeeds in process_one(), a subsequent error processing the file 
will cause the function to return an error immediately without closing the file 
descriptor. This causes a leak and can prevent successful processing of a large 
set of files during a single execution.
>How-To-Repeat:
Run something like this on a large directory, which should contain several 
non-ELF files:

bash-4.0$ paxctl -m /usr/pkg/bin/*

Note that you'll be required to have a ulimit (-n) less than the number of 
files that will error to reproduce the issue.
>Fix:
The following patch fixes the problem by closing the file descriptor of the 
file currently processing if an error is encountered after open() succeeds.

--- paxctl.c    2009-05-01 22:53:14.000000000 -0600
+++ paxctl.c_orig       2009-05-01 22:51:10.000000000 -0600
@@ -203,12 +203,12 @@
 
        if (read(fd, &e, sizeof(e)) != sizeof(e)) {
                warn("Can't read ELF header from `%s'", name);
-               goto error;
+               return 1;
        }
 
        if (memcmp(e.h32.e_ident, ELFMAG, SELFMAG) != 0) {
                warnx("Bad ELF magic from `%s' (maybe it's not an ELF?)", name);
-               goto error;
+               return 1;
        }
 
        if (e.h32.e_ehsize == sizeof(e.h32)) {
@@ -226,14 +226,14 @@
        } else {
                warnx("Bad ELF size %d from `%s' (maybe it's not an ELF?)",
                    (int)e.h32.e_ehsize, name);
-               goto error;
+               return 1;
        }
 
        for (i = 0; i < EH(e_phnum); i++) {
                if (pread(fd, &p, PHSIZE, (off_t)EH(e_phoff) + i * PHSIZE) !=
                    PHSIZE) {
                        warn("Can't read program header data from `%s'", name);
-                       goto error;
+                       return 1;
                }
 
                if (PH(p_type) != PT_NOTE)
@@ -241,7 +241,7 @@
 
                if (pread(fd, &n, NHSIZE, (off_t)PH(p_offset)) != NHSIZE) {
                        warn("Can't read note header from `%s'", name);
-                       goto error;
+                       return 1;
                }
                if (NH(n_type) != ELF_NOTE_TYPE_PAX_TAG ||
                    NH(n_descsz) != ELF_NOTE_PAX_DESCSZ ||
@@ -250,14 +250,14 @@
                if (pread(fd, &pax_tag, sizeof(pax_tag), PH(p_offset) + NHSIZE)
                    != sizeof(pax_tag)) {
                        warn("Can't read pax_tag from `%s'", name);
-                       goto error;
+                       return 1;
                }
                if (memcmp(pax_tag.name, ELF_NOTE_PAX_NAME,
                    sizeof(pax_tag.name)) != 0) {
                        warn("Unknown pax_tag name `%*.*s' from `%s'",
                            ELF_NOTE_PAX_NAMESZ, ELF_NOTE_PAX_NAMESZ,
                            pax_tag.name, name);
-                       goto error;
+                       return 1;
                }
                ok = 1;
 
@@ -286,7 +286,7 @@
                if (!pax_flags_sane(SWAP(pax_tag.flags))) {
                        warnx("New flags 0x%x don't make sense",
                            (uint32_t)SWAP(pax_tag.flags));
-                       goto error;
+                       return 1;
                }
 
                if (pwrite(fd, &pax_tag, sizeof(pax_tag),
@@ -309,10 +309,6 @@
                (void)printf("No PaX flags.\n");
        }
        return 0;
-
-error:
-       (void)close(fd);
-       return 1;
 }
 
 int