Re: ioctl arg passing

[Petr Vandrovec]

On 23 Apr 01 at 17:06, Matt wrote:
> struct instruction_t {
>     __s16 code;
>     __s16 rxlen;
>     __s16 *rxbuf;
>     __s16 txlen;
>     __s16 *txbuf;
> };

You should reorder fields, starting with largest fields and going down
to smaller ones. That ways you'll not have troubles with alignment when
someone decides to play with alignment rules...

> struct instruction_t local;
> __s16 *temp;
> 
> copy_from_user( &local, ( struct instruction_t * ) arg, sizeof( struct instruction_t ) );
> temp = kmalloc( sizeof( __s16 ) * local.rxlen, GFP_KERNEL );
> copy_from_user( temp, arg, sizeof( __s16 ) * local.rxlen );
> local.rxbuf = temp;
> temp = kmalloc( sizeof( __s16 ) * local.txlen, GFP_KERNEL );
> ...

As you are using signed value for rxlen/txlen, you should check
for value < 0 ... And there is very low chance that kmalloc() for 
anything bigger than 4KB will succeed. You should either use
vmalloc unconditionally, or at least as fallback. And some error
checking (copy_from_user returns 0 if everything went OK) also
makes driver safer.
                                        Best regards,
                                            Petr Vandrovec
                                            [e-mail]