pkgsrc-Bugs archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

pkg/47126: i386 sudo fails on NetBSD amd64 kernel



>Number:         47126
>Category:       pkg
>Synopsis:       i386 sudo fails on NetBSD amd64 kernel
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    pkg-manager
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Fri Oct 26 06:40:01 +0000 2012
>Originator:     Alan Barrett
>Release:        NetBSD 6.99.14
>Organization:
Not much
>Environment:
pkgsrc/security/sudo-1.7.9p1, compiled under NetBSD/i386,
running under NetBSD-6.99.14/amd64
>Description:
        The get_process_ttyname function in sudo's ttyname.c source file
        uses sysctl([CTL_KERN, KERN_PROC, KERN_PROC_PID, ...], ...)
        to get a struct kinfo_proc, and then analyses it to try to find the
        name of the process's controlling terminal. terminal.

        struct kinfo_proc contains data types whose size changes on
        NetBSD between i386 and amd64.  When the data is supplied by an
        amd64 kernel, and analysed by code that ws expecting data from an
        i386 kernel, it fails.

>How-To-Repeat:
        Install NetBSD/i386.  Build and install sudo from pkgrc.
        Build and boot a NetBSD/amd64 kernel, while keeping the
        old i386 sudo program.  Try to run sudo.  See it fail with
        this message:

        sudo: internal error, tried to erealloc(0)

>Fix:
        What happens is that sudo calls sysctl with a buffer
        large enough for an i386 struct kinfo_proc.  sysctl fails
        with ENOMEM because the amd64 structure is bigger, and
        (correctly) sets the size variable to zero.  sudo then
        loops with the intent to trying again with a larger buffer
        size, but the size is now zero, so it hits an error case
        in the erealloc function.

        1) Mishandling the size in the sysctl retry loop is clearly a
        bug in sudo.  Change it to keep track of the size better:

        do {
+           size_t oldsize;
            size += size / 10;
            ki_proc = erealloc(ki_proc, size);
+           oldsize = size;
            rc = sysctl(mib, sudo_kp_namelen, ki_proc, &size, NULL, 0);
+           if (size < oldsize)
+               size = oldsize;
        } while (rc == -1 && errno == ENOMEM);

        2) Returning an amd64 struct kinfo_proc to an i386 process
        is a broblem, but not really a kernel bug.  The NetBSD kernel already
        has struct kinfo_proc2, which is intended to have a
        machine-independent size.  sudo could use a different sysctl
        mib to retrieve struct kinfo_proc2 instead of struct kinfo_proc.

        3) It might be reasonable to replace all the kernel memory
        grovelling with a simple call to ttyname(3) on a fd that
        has been opened to /dev/tty, like this:

+/*
+ * Return a string from ttyname() containing the tty to which the process is
+ * attached or NULL if there is no tty associated with the process.
+ */
+char *
+get_process_ttyname()
+{
+    int ttyfd;
+    char *tty;
+
+    ttyfd = open("/dev/tty", O_RDONLY);
+    if (ttyfd < 0)
+       return NULL;
+    tty = ttyname(ttyfd);
+    close(ttyfd);
+    return (tty ? estrdup(tty) : NULL);
+}

        It would also be possible to use something similar to the
        above code to augment, rather than replace, the
        other versions of get_process_ttyname that are already
        present in sudo's ttyname.c.

        Unfortunately, devname(3) can return "/dev/tty" instead
        of the actual name of the underlying device.  On NetBSD,
        it returns the name of the underlying device if it's a
        pty, but not if it's a hardware tty.



Home | Main Index | Thread Index | Old Index