From owner-freebsd-hackers@FreeBSD.ORG Fri Apr 23 01:01:20 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 0B10E106564A for ; Fri, 23 Apr 2010 01:01:20 +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 B601A8FC14 for ; Fri, 23 Apr 2010 01:01:19 +0000 (UTC) Received: by qyk11 with SMTP id 11so10675423qyk.13 for ; Thu, 22 Apr 2010 18:01:19 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:received:in-reply-to:references :date:received:message-id:subject:from:to:cc:content-type :content-transfer-encoding; bh=wl6HfG3MTG5FY0ktwREMByf5Zake4STjVBzZryNFsA4=; b=W05/bB3vCFof8g0YOjZxVuNKd054AYsnrD2HfYAWsQO676okl1ynA467hHpJgC1E4o dEIz3/PPmNCcG8obfA3uMr2xtM7faMVU8dpmRrp63v1ZL+93zwMmn6pEDDPnj6fWpvZj s+rXgBt4pFGNqwjsFPUVFw03MUxSHcyn60JIo= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type:content-transfer-encoding; b=MQOzQzx+u7Sq/PuDru9n6UkIg7HedoXq3Rs+JxelzipKmlJumaMviX+d1aO2fQylli AinqWu6AuUj7+9g3uMrFO4PlZfgNAytexpEV6HKLAqU/6Ox4TuKwtu6nNnRcI/RFJtpv gDTqWUsPq/4ddFDlErm3urgMsLk2zR5yqpMys= MIME-Version: 1.0 Received: by 10.229.233.11 with HTTP; Thu, 22 Apr 2010 18:01:18 -0700 (PDT) In-Reply-To: <4BD0EEFA.90907@delphij.net> References: <06D5F9F6F655AD4C92E28B662F7F853E039E33A7@seaxch09.desktop.isilon.com> <4BD0EEFA.90907@delphij.net> Date: Thu, 22 Apr 2010 18:01:18 -0700 Received: by 10.229.96.82 with SMTP id g18mr745117qcn.82.1271984478959; Thu, 22 Apr 2010 18:01:18 -0700 (PDT) Message-ID: From: Garrett Cooper To: d@delphij.net Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Cc: FreeBSD-Hackers , Matthew Fleming 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 23 Apr 2010 01:01:20 -0000 On Thu, Apr 22, 2010 at 5:51 PM, Xin LI wrote: > -----BEGIN PGP SIGNED MESSAGE----- > Hash: SHA1 > > On 2010/04/22 17:45, Garrett Cooper wrote: >> On Thu, Apr 22, 2010 at 4:36 PM, Matthew Fleming >> wrote: >>>> Hi hackers, >>>> =A0 =A0 I realize that this isn't 100% userland code, so the checks sh= ould >>>> 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: >>>> >>>> =A0 =A0 =A0 =A0 if (size > 0) { >>>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (com & IOC_VOID) { >>>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* Integer argument. *= / >>>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 arg =3D (intptr_t)uap-= >data; >>>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 data =3D (void *)&arg; >>>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 size =3D 0; >>>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 } else >>>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 data =3D malloc((u_lon= g)size, M_IOCTLOPS, >>>> M_WAITOK); /* XXX: can fail -- do we care? */ >>> >>> malloc(9) with M_WAITOK cannot return NULL. =A0So 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. =A0They deal with this by setting a >>> function pointer in PCB_ONFAULT, which is used in trap() to set a retur= n >>> instruction pointer. >> >> Matt, >> =A0 =A0 Awesome. I can see I need to do a bit more reading in malloc(3) = :)... >> Thanks for the info! > > It's actually malloc(9)... =A0I personally feels it pretty confusing at > the beginning when I learned about it. Yeah... that makes more sense. It'd be kind of stupid to go through the libc shim to get to kernel memory management. Thanks for the correction :}, -Garrett