From owner-freebsd-hackers@FreeBSD.ORG Mon Jan 21 02:50:22 2013 Return-Path: Delivered-To: hackers@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by hub.freebsd.org (Postfix) with ESMTP id 1DB02AE9; Mon, 21 Jan 2013 02:50:22 +0000 (UTC) (envelope-from yuri@rawbw.com) Received: from shell0.rawbw.com (shell0.rawbw.com [198.144.192.45]) by mx1.freebsd.org (Postfix) with ESMTP id F2FED29F; Mon, 21 Jan 2013 02:50:21 +0000 (UTC) Received: from eagle.yuri.org (stunnel@localhost [127.0.0.1]) (authenticated bits=0) by shell0.rawbw.com (8.14.4/8.14.4) with ESMTP id r0L2oKJm068072; Sun, 20 Jan 2013 18:50:21 -0800 (PST) (envelope-from yuri@rawbw.com) Message-ID: <50FCACEC.8000100@rawbw.com> Date: Sun, 20 Jan 2013 18:50:20 -0800 From: Yuri User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:17.0) Gecko/20130112 Thunderbird/17.0.2 MIME-Version: 1.0 To: mdf@freebsd.org Subject: Re: How to validate the variable size memory block in ioctl handler? References: <50FC7767.4050207@rawbw.com> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-Content-Filtered-By: Mailman/MimeDel 2.1.14 Cc: hackers@freebsd.org X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 21 Jan 2013 02:50:22 -0000 On 01/20/2013 16:59, mdf@freebsd.org wrote: > To do what you want it sounds like you want your handler to take something like: > > struct var_ioctl { > int len; > void *data; > }; > > Then then handler itself would have to use copyin/copyout to access > the data. There's no simpler way. I think I found the simpler way, see the draft patch below. Generic macro _IOWRE will handle the case when the first integer in ioctl parameter holds the actual size of the structure. This way of passing the variable array sizes is quite common in various APIs. Other potential uses would also benefit from this. Yuri Index: sys/kern/sys_generic.c =================================================================== --- sys/kern/sys_generic.c (revision 245654) +++ sys/kern/sys_generic.c (working copy) @@ -640,6 +640,7 @@ int arg, error; u_int size; caddr_t data; + int vsize; if (uap->com > 0xffffffff) { printf( @@ -654,6 +655,14 @@ * copied to/from the user's address space. */ size = IOCPARM_LEN(com); + if (size == IOC_VARIABLE) { + /* first integer has the length of the memory */ + error = copyin(uap->data, (caddr_t)&vsize, sizeof(vsize)); + if (error) + return (error); + size = (u_int)vsize; + } if ((size > IOCPARM_MAX) || ((com & (IOC_VOID | IOC_IN | IOC_OUT)) == 0) || #if defined(COMPAT_FREEBSD5) || defined(COMPAT_FREEBSD4) || defined(COMPAT_43) Index: sys/sys/ioccom.h =================================================================== --- sys/sys/ioccom.h (revision 245654) +++ sys/sys/ioccom.h (working copy) @@ -50,6 +50,7 @@ #define IOC_IN 0x80000000 /* copy in parameters */ #define IOC_INOUT (IOC_IN|IOC_OUT) #define IOC_DIRMASK (IOC_VOID|IOC_OUT|IOC_IN) +#define IOC_VARIABLE IOCPARM_MASK /* parameters size in parameters */ #define _IOC(inout,group,num,len) ((unsigned long) \ ((inout) | (((len) & IOCPARM_MASK) << 16) | ((group) << 8) | (num))) @@ -59,6 +60,9 @@ #define _IOW(g,n,t) _IOC(IOC_IN, (g), (n), sizeof(t)) /* this should be _IORW, but stdio got there first */ #define _IOWR(g,n,t) _IOC(IOC_INOUT, (g), (n), sizeof(t)) +#define _IORE(g,n) _IOC(IOC_OUT, (g), (n), IOC_VARIABLE) +#define _IOWE(g,n) _IOC(IOC_IN, (g), (n), IOC_VARIABLE) +#define _IOWRE(g,n) _IOC(IOC_INOUT, (g), (n), IOC_VARIABLE) #ifdef _KERNEL