From owner-freebsd-hackers@FreeBSD.ORG Thu Apr 22 23:27:27 2010 Return-Path: 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 4EB45106566B for ; Thu, 22 Apr 2010 23:27:27 +0000 (UTC) (envelope-from yanefbsd@gmail.com) Received: from mail-qy0-f181.google.com (mail-qy0-f181.google.com [209.85.221.181]) by mx1.freebsd.org (Postfix) with ESMTP id 0C4EA8FC13 for ; Thu, 22 Apr 2010 23:27:26 +0000 (UTC) Received: by qyk11 with SMTP id 11so10562394qyk.13 for ; Thu, 22 Apr 2010 16:27:26 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:received:date:received:message-id :subject:from:to:content-type; bh=NNCZY8VoHPKFXC0WN8rZBrfVp3fn5Hf2lhMvJBYFol0=; b=L1hUqMZgOZJuHXXfYIGC2sOR0480b5DYzEB0aC+UDGYJpNbfWyzD0mwePsLcbSXr77 XxHqeFl3vGD8BBSlLAkKXZWANKFv6ebSbL50rWodwX8ZIEyAKYLDuTdl7NXHD93YP3sY dD1whLBquSytU5S4R5lP5GsDBOn7tW5uUSd7U= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:date:message-id:subject:from:to:content-type; b=xnnwhfR6BFbBZhk9qh4VqdpWPosGXp/LeqfFr+IhE9kilge10f/GDJxph1bUXuDomQ H9ku8Yp5tSC9AC+7jFIJ81YUeV1m71TiMl4E09T0VkJDyegQdzxeAInnmFQIjZ4xl8Pn 02dnSP8nfoagUGV6nvuEeM/aeJB8nn9iTrQLc= MIME-Version: 1.0 Received: by 10.229.233.11 with HTTP; Thu, 22 Apr 2010 16:27:26 -0700 (PDT) Date: Thu, 22 Apr 2010 16:27:26 -0700 Received: by 10.229.221.14 with SMTP id ia14mr927162qcb.8.1271978846188; Thu, 22 Apr 2010 16:27:26 -0700 (PDT) Message-ID: From: Garrett Cooper To: FreeBSD-Hackers Content-Type: text/plain; charset=ISO-8859-1 Subject: 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 22 Apr 2010 23:27:27 -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: 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