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
[EMAIL PROTECTED]
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
