- From: Omri Schwarz <ocschwar_at_MIT.EDU>
- Date: Mon, 21 Jun 2004 16:36:38 -0400
I am looking at the code here and wondering if there
might be a better way to organize this: [ comedi_fops.c, line 173...]
if(it.options[COMEDI_DEVCONF_AUX_DATA] &&
it.options[COMEDI_DEVCONF_AUX_DATA_LENGTH]){
aux_len = it.options[COMEDI_DEVCONF_AUX_DATA_LENGTH];
if(aux_len<0)return -EFAULT;
aux_data = kmalloc(aux_len, GFP_KERNEL);
if(!aux_data)return -EFAULT;
if(copy_from_user(aux_data,
(void *)it.options[COMEDI_DEVCONF_AUX_DATA], aux_len)){
kfree(aux_data);
return -EFAULT;
}
it.options[COMEDI_DEVCONF_AUX_DATA] = (unsigned long)aux_data;
}
In AMD 64, sizeof(void*) is 8 bytes, not 4, so the (void *)
cast is bad and may be the cause of the crashes I am seeing.
I'm not exactly clear on what is happening in this area.
Apart from the members COMEDI_DEVCONF_AUX_DATA and
COMEDI_DEVCONF_AUX_DATA_LENGTH,
are the other members of the options array in the struct comedi_devconfig
used at all?
If that structure in comedi.h is changed to read:
struct comedi_devconfig_struct{
char board_name[COMEDI_NAMELEN];
size_t options[COMEDI_NDEVCONFOPTS];
};
and the code from above changed to read:
if((it.options[COMEDI_DEVCONF_AUX_DATA] != NULL) &&
(it.options[COMEDI_DEVCONF_AUX_DATA_LENGTH] >0) ){
aux_len = it.options[COMEDI_DEVCONF_AUX_DATA_LENGTH];
if(aux_len<0)return -EFAULT;
aux_data = kmalloc(aux_len, GFP_KERNEL);
if(!aux_data)return -EFAULT;
if(copy_from_user(aux_data,
(void *)it.options[COMEDI_DEVCONF_AUX_DATA], aux_len)){
kfree(aux_data);
return -EFAULT;
}
it.options[COMEDI_DEVCONF_AUX_DATA] = (unsigned long)aux_data;
}
Then the result is slightly more protable, although there
then comes the problem of 32 bit compatibility on 64 bit systems,
which I need to read up on how to do correctly.
Received on 2004-06-21Z19:36:38