On Wed, Mar 15, 2000 at 01:18:43AM +1100, Darren Reed wrote:

 > Is the below patch something we should do for safety reasons ?
 > My concern is someone assuming that the data pointer passed to
 > the ioctl handler is actually 32 or 64bit aligned (given we don't
 > know what was actually passed up)...whilst this may work for us
 > now, it would seem to me that counting on it not breaking would
 > be compiler-dependant ...

Well, it is currently safe.  All of the stack members in your example
are sized correctly to provide the correct alignment.  This isn't
something that's compiler dependent, but rather architecture/ABI
dependent.

I wouldn't use "void *" there; I'd use u_long instead.  But I don't think
it's really necessary to make this change.

On the other hand, I wouldn't exactly object to the change being made; I'm
kind of indifferent to it :-)

 > 
 > Darren
 > 
 > *** /sys/kern/sys_generic.c.orig	Wed Mar 15 01:10:11 2000
 > --- /sys/kern/sys_generic.c	Wed Mar 15 01:11:36 2000
 > ***************
 > *** 491,497 ****
 >   	caddr_t data, memp;
 >   	int tmp;
 >   #define STK_PARAMS	128
 > ! 	char stkbuf[STK_PARAMS];
 >   
 >   	fdp = p->p_fd;
 >   	if ((u_int)SCARG(uap, fd) >= fdp->fd_nfiles ||
 > --- 491,497 ----
 >   	caddr_t data, memp;
 >   	int tmp;
 >   #define STK_PARAMS	128
 > ! 	void *stkbuf[STK_PARAMS/sizeof(void *)];
 >   
 >   	fdp = p->p_fd;
 >   	if ((u_int)SCARG(uap, fd) >= fdp->fd_nfiles ||
 > ***************
 > *** 522,528 ****
 >   		memp = (caddr_t)malloc((u_long)size, M_IOCTLOPS, M_WAITOK);
 >   		data = memp;
 >   	} else
 > ! 		data = stkbuf;
 >   	if (com&IOC_IN) {
 >   		if (size) {
 >   			error = copyin(SCARG(uap, data), data, size);
 > --- 522,528 ----
 >   		memp = (caddr_t)malloc((u_long)size, M_IOCTLOPS, M_WAITOK);
 >   		data = memp;
 >   	} else
 > ! 		data = (caddr_t)stkbuf;
 >   	if (com&IOC_IN) {
 >   		if (size) {
 >   			error = copyin(SCARG(uap, data), data, size);
 > 

-- 
        -- Jason R. Thorpe <thorpej@zembu.com>