Re: [Qemu-devel] RFC: [0/11] EFAULT patch

[J. Mayer]

On Tue, 2007-09-18 at 20:59 -0400, Stuart Anderson wrote:
> Following this message, are the 11 parts of the patch that implements
> EFAULT detection in the linux-user mode. Hopefully, this reflects what
> was discussed following the first RFC of this patch. Also, hopefully, it
> is easier to digest in smaller pieces like this.
> 
> In short, the (un)lock_user*() interfaces have been replaced by access_ok
> and copy_(to|from)_user() style interfaces. This code should now look
> more like some of the 32_on_64 code in the Linux kernel.
> 
> As a side effect of these changes, and the more thorough testing that
> went along with them, several other bugs have been fixed in areas such
> as IPC and sockets.
> 
> As before, the Linux Test Project test suite was used in an armel on
> x86_64 environment.
> 
> Your comments would be appreciated as I'd like to finish beating these
> changes into shape so they can be accepted into the repository.


The idea is great but there seem to be a problem in those patches:
you directly cast syscall arguments, which are (or should be)
target_ulong to pointers in the host environment. You should to use the
g2h / h2g macros to get the pointer in the host memory from the offset
in the target address space. Offset in the target address  space can not
be assumed to be the same size as an address in the host address space,
thus can never be casted directly to host pointer.
Then, the changes you've done, changing long arguments (which should be
target_long to be correct, you can take a look at the last patch I sent
on the list) to pointers, for example in function prototypes, are
incorrect.
This used to be handled by the lock_user functions and should be handled
in your patch too...

One thing I really dislike is multiple statements on the same line. I
know this is only cosmetics (and that coding style discussion usually
have no end), but code like:
if (xxxx) return -1;
can easily confuse any reader, imho, especially when the lines are long
then is to be avoided....

Regards.

-- 
J. Mayer <address@hidden>
Never organized