Skip site navigation (1)Skip section navigation (2) 
Date:      Thu, 22 Apr 2010 16:27:26 -0700
From:      Garrett Cooper <yanefbsd@gmail.com>
To:        FreeBSD-Hackers <freebsd-hackers@freebsd.org>
Subject:   Error checking in ioctl(2)?
Message-ID:  <w2v7d6fde3d1004221627jff97746bkcb8cd3ca5e5a7492@mail.gmail.com>
next in thread | raw e-mail | index | archive | help 
Hi hackers,
    I realize that this isn't 100% userland code, so the checks should
be minimalized, but when looking at the ioctl(2) syscall code (at
least I think it is... there's another dupe hanging around in
sys/dev/hptmv/ioctl.c), I had some questions related to the error
handling not being done in the code:

        if (size > 0) {
                if (com & IOC_VOID) {
                        /* Integer argument. */
                        arg = (intptr_t)uap->data;
                        data = (void *)&arg;
                        size = 0;
                } else
                        data = malloc((u_long)size, M_IOCTLOPS,
M_WAITOK); /* XXX: can fail -- do we care? */
        } else
                data = (void *)&uap->data;
        if (com & IOC_IN) {
                error = copyin(uap->data, data, (u_int)size); /* XXX:
Is this ok if it's NULL? */
                if (error) {
                        if (size > 0)
                                free(data, M_IOCTLOPS); /* XXX: What
if size > 0 and data was NULL? */
                        return (error);
                }
        } else if (com & IOC_OUT) {
                /*
                 * Zero the buffer so the user always
                 * gets back something deterministic.
                 */
                bzero(data, size); /* XXX: This will barf with a segfault. */
        }

        error = kern_ioctl(td, uap->fd, com, data);

        if (error == 0 && (com & IOC_OUT))
                error = copyout(data, uap->data, (u_int)size); /* XXX:
Is this ok if it's NULL? This will be triggered with NULL only if com
& IOC_IN was true and com & IOC_OUT was true -- not sure if that is
possible or not... */

    I tried following some of the logic in amd64/amd64/support.S to
figure out what's going on with copyin(2) and copyout(2), but my ASM64
foo isn't good enough to grok what's going on there yet.
Thanks,
-Garrett

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?w2v7d6fde3d1004221627jff97746bkcb8cd3ca5e5a7492>