On Sun, Jan 20, 2013 at 6:50 PM, Yuri <y...@rawbw.com> wrote:
> On 01/20/2013 16:59, m...@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


This would be fine for a local patch but it breaks existing (valid)
uses that have exactly 8191 bytes of data, so it wouldn't be suitable
for the main FreeBSD repository.  Also, in general one wants to have
limits on syscalls that can force a kernel malloc of any size, as it
leads to denial of service attacks or crashes by requesting the kernel
over-allocate memory.

Cheers,
matthew
_______________________________________________
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to "freebsd-hackers-unsubscr...@freebsd.org"

Reply via email to