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
vandrove@vc.cvut.cz
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
This archive was generated by hypermail 2b29 : Mon Apr 23 2001 - 21:00:46 EST