Re: [Qemu-devel] Re: [PATCH] efault - add data type to put_user()/get_user()

[Thayne Harbaugh]

On Sat, 2007-11-03 at 20:05 +0100, Fabrice Bellard wrote:
> I think that using host addresses in __put_user and __get_user is not
> logical. They should use target addresses as get_user and put_user. As
> Paul said, It is not worth mixing get/put/copy and lock/unlock functions.

Please see the "RFC: x86_64 Best way to fix 'cast to pointer'" email for
some discussion of get/put/copy and lock/unlock.  {get,put}_user() is
used for individual ints or other atomically writable types that are
passed as pointers into a syscall.  copy_{to,from}_user_<struct>() are
used for structures that are passed to a syscall.  lock/unlock() will be
used internally in these because lock/unlock does address translation.
lock/unlock() are still needed and are independent.  __{get,put}_user()
will operate internally in these functions on structure data members
where lock/unlock() access_ok() have already been called.

> The ultimate goal of such cleanup is not only to generate -EFAULT
> correctly but also to be able to have arbitrary address space changes.

Yes.  This will be possible once all my clean-ups are pushed.

> In fact it would be good to be able to introduce an arbitrary address
> space change (such as a translation as Paul did) so that we can verify
> that all the Linux emulation stills works in this case.

I'll be testing this way.

> Regards,
> 
> Fabrice.
> 
> Thayne Harbaugh wrote:
> > On Wed, 2007-10-31 at 16:44 -0600, Thayne Harbaugh wrote:
> >> This patch updates get_user() and put_user() to take a third argument of
> >> data type.  get_user() and put_user() use target address which are
> >> target_ulong and don't reflect the data type pointed to in target
> >> memory.
> >>
> >> Simply casting the target_ulong to a type before passing to
> >> get/put_user() is poor because target_ulong isn't always a simple cast
> >> to a host type (consider 32 bit on 64 bit where address are either
> >> extended or truncate).  Also, simple casting of the argument to
> >> get/put_user() results in several warnings when target and long pointer
> >> sizes don't match.
> >>
> >> This patch has additional updates to fix places where get/put_user() are
> >> already used.
> > 
> > This is an updated patch that doesn't conflict with the
> > abi_long/abi_ulong changes from a couple weeks ago.