From owner-freebsd-hackers@FreeBSD.ORG  Thu Apr 22 23:36:14 2010
Return-Path: <owner-freebsd-hackers@FreeBSD.ORG>
Delivered-To: freebsd-hackers@freebsd.org
Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34])
	by hub.freebsd.org (Postfix) with ESMTP id CCE721065672
	for <freebsd-hackers@freebsd.org>; Thu, 22 Apr 2010 23:36:14 +0000 (UTC)
	(envelope-from matthew.fleming@isilon.com)
Received: from seaxch09.isilon.com (seaxch09.isilon.com [74.85.160.25])
	by mx1.freebsd.org (Postfix) with ESMTP id B0C2D8FC15
	for <freebsd-hackers@freebsd.org>; Thu, 22 Apr 2010 23:36:14 +0000 (UTC)
X-MimeOLE: Produced By Microsoft Exchange V6.5
Content-class: urn:content-classes:message
MIME-Version: 1.0
Content-Type: text/plain;
	charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
Date: Thu, 22 Apr 2010 16:36:28 -0700
Message-ID: <06D5F9F6F655AD4C92E28B662F7F853E039E33A7@seaxch09.desktop.isilon.com>
In-Reply-To: <w2v7d6fde3d1004221627jff97746bkcb8cd3ca5e5a7492@mail.gmail.com>
X-MS-Has-Attach: 
X-MS-TNEF-Correlator: 
Thread-Topic: Error checking in ioctl(2)?
Thread-Index: Acric2xRXegHoEqhQuW+5GLVcjM2ZwAAMjZw
References: <w2v7d6fde3d1004221627jff97746bkcb8cd3ca5e5a7492@mail.gmail.com>
From: "Matthew Fleming" <matthew.fleming@isilon.com>
To: "Garrett Cooper" <yanefbsd@gmail.com>
Cc: FreeBSD-Hackers <freebsd-hackers@freebsd.org>
Subject: RE: Error checking in ioctl(2)?
X-BeenThere: freebsd-hackers@freebsd.org
X-Mailman-Version: 2.1.5
Precedence: list
List-Id: Technical Discussions relating to FreeBSD
	<freebsd-hackers.freebsd.org>
List-Unsubscribe: <http://lists.freebsd.org/mailman/listinfo/freebsd-hackers>, 
	<mailto:freebsd-hackers-request@freebsd.org?subject=unsubscribe>
List-Archive: <http://lists.freebsd.org/pipermail/freebsd-hackers>
List-Post: <mailto:freebsd-hackers@freebsd.org>
List-Help: <mailto:freebsd-hackers-request@freebsd.org?subject=help>
List-Subscribe: <http://lists.freebsd.org/mailman/listinfo/freebsd-hackers>,
	<mailto:freebsd-hackers-request@freebsd.org?subject=subscribe>
X-List-Received-Date: Thu, 22 Apr 2010 23:36:14 -0000

> 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:
>=20
>         if (size > 0) {
>                 if (com & IOC_VOID) {
>                         /* Integer argument. */
>                         arg =3D (intptr_t)uap->data;
>                         data =3D (void *)&arg;
>                         size =3D 0;
>                 } else
>                         data =3D malloc((u_long)size, M_IOCTLOPS,
> M_WAITOK); /* XXX: can fail -- do we care? */

malloc(9) with M_WAITOK cannot return NULL.  So the rest of your XXX
comments are not at issue.

Also, free(9) is documented to do the right thing when asked to
free(NULL).

copyin/copyout are really just bcopy but unlike most kernel code they
are allowed to take a page fault.  They deal with this by setting a
function pointer in PCB_ONFAULT, which is used in trap() to set a return
instruction pointer.

Cheers,
matthew