Date: Thu, 22 Apr 2010 18:01:18 -0700 From: Garrett Cooper <yanefbsd@gmail.com> To: d@delphij.net Cc: FreeBSD-Hackers <freebsd-hackers@freebsd.org>, Matthew Fleming <matthew.fleming@isilon.com> Subject: Re: Error checking in ioctl(2)? Message-ID: <k2x7d6fde3d1004221801xdbb2042fi7d38ae84f2bbeece@mail.gmail.com> In-Reply-To: <4BD0EEFA.90907@delphij.net> References: <w2v7d6fde3d1004221627jff97746bkcb8cd3ca5e5a7492@mail.gmail.com> <06D5F9F6F655AD4C92E28B662F7F853E039E33A7@seaxch09.desktop.isilon.com> <u2o7d6fde3d1004221745o32173f04hdf77c080979026c8@mail.gmail.com> <4BD0EEFA.90907@delphij.net>
next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, Apr 22, 2010 at 5:51 PM, Xin LI <delphij@delphij.net> 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
>> <matthew.fleming@isilon.com> 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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?k2x7d6fde3d1004221801xdbb2042fi7d38ae84f2bbeece>