Subject: RE: COPYIN/COPYOUT macro problems Re: IOCTL implementation and kernel/userland addresses
To: Reinoud Zandijk <reinoud@netbsd.org>
From: Gordon Waidhofer <gww@traakan.com>
List: tech-kern
Date: 08/25/2005 14:14:46
Don't take this too seriously. It's just thought food.

Some machine architectures have multiple address spaces.
An easy example was the separate I&D (Instruction, Data)
spaces on the PDP-11. The IBM EA architecture (don't know
much about it) has multiple address spaces and an instruction
set to allow "far" accesses.

On a VM project I worked on a long, long time ago, our version
of copyin/out took a pointer to the address space being operated
upon. Think of this as a pointer to the vm_map anchor. Familiar
interfaces, uiomove() et al, were bandaided at the time, and let
the EA guys figure out how to do it right.

This ioctl() question came up and I remembered all the long
ago grapling.

The upshot is that it might be helpful to think of the address
space selector a little more broadly than a kernel/user flag.
Perhaps these interfaces being discussed could take a parameter
for address space rather than a flag. I always thought that
the struct uio uio_segflg should be such a parameter rather
than a flag or enumeration.

Again, just thought food. I couldn't make a real case for it.
I wouldn't even try.

Regards,
	-gww


> -----Original Message-----
> From: tech-kern-owner@NetBSD.org [mailto:tech-kern-owner@NetBSD.org]On
> Behalf Of Reinoud Zandijk
> Sent: Thursday, August 25, 2005 1:14 PM
> To: Jason Thorpe
> Cc: YAMAMOTO Takashi; Rui Paulo; tech-kern@netbsd.org
> Subject: Re: COPYIN/COPYOUT macro problems Re: IOCTL implementation and
> kernel/userland addresses
> 
> 
> Dear folks,
> 
> On Thu, Aug 25, 2005 at 12:44:52PM -0700, Jason Thorpe wrote:
> > No, kcopy() is NOT aliased to memcpy().  It performs similar error  
> > checking to copyin()/copyout().
> 
> *sigh* where was my mind.
> 
> > >We could opt for a more generic kcopyin() function :
> > >
> > >int kucopyin(int kernelspace, void *uaddr, void *kaddr, int len) {
> > 
> > void *from, void *to, size_t len
> > 
> > >int retval;
> > >
> > >if (!kernelspace) {
> > >return copyin(uaddr, kaddr, len);
> > >};
> > >
> > >/*
> > >* returning anything other than 0 is meaningless for it would have
> > >* panicked the kernel allready if the address was not valid.
> > >*/
> > >kcopy(uaddr, kaddr, len);
> > >return 0;
> > 
> > No, return the error here.  kcopy() is defined to return an error  
> > code, and will.  UBC relies on this.
> 
> ok.
> 
> > >where the flag `kernelspace' being non-zero means use in-kernel  
> > >transfers.
> > >This way checking for the FKIOCTL flag and/or using uio's uio_seg {
> > >UIO_USERSPACE, UIO_SYSSPACE } would be consistent.
> > 
> > I would prefer to see the UIO_*SPACE enums to be used, rather than  
> > just a boolean.
> 
> but then you loose the advantage of just calling
> 	kucopyin(flags & KFIOCTL, from, to, len);
> 	kucopyin(uio.uio_segflg == UIO_SYSSPACE, from, to, len);
> 	kucopyin(ISSET(flags, ...), from, to, len);
> or even 
> 	kucopyin(uio.uio_segflg, from, to, len);
> when its allowed to make the assumption that UIO_SYSSPACE != 0.
> 
> or go for `aliases' that check specific values in
> 	ioctl_copyin(flag, ...)
> 	uio_copyin(flag, ...)
> functions.
> 
> thoughts?
> Reinoud
>