On Tuesday 10 July 2007, Stuart Anderson wrote: > On Mon, 9 Jul 2007, Fabrice Bellard wrote: > > No. Ideally you should use the same conventions as the Linux kernel and > > assume that you cannot access the user data directly. > > That's what I had already started doing today. > > > For the time being, I would suggest to minimize the number of changes and > > just extend lock_user()/unlock_user() as you began to do to handle > > -EFAULT. The rest is mostly a question of cosmetics. > > The attached patch is my in-progress work of the complete overhaul to use > the kernel conventions. It needs some more work to finish the > conversion, but enough should be done to see how it is going to turn > out. Overall, I think the converted code is easier to read, especially > if you are familiar with reading kernel code. I also think it will end > up being more correct both becasue of the additional time now spent on > reviewing each syscall, as well as the kernel conventions tend to make > you be more thorough and explicite.
> { > struct target_rusage *target_rusage; > > - lock_user_struct(target_rusage, target_addr, 0); > + if( !access_ok(VERIFY_READ,target_addr,sizeof(*target_rusage)) ) > return -1; > + target_rusage = (struct target_rusage *)g2h(target_addr); Using g2h directly is bad. g2h is an implementation detail of one particular memory model. The whole point of the lock_user abstraction (or a similar copy_from_user abstraction) is that almost none of the code cares how "user" memory is accessed. One of the long-term goals of this abstraction is to allow the softmmu code to be used with userspace emulation. In this case a region may be split across multiple discontiguous host pages. The reason I used a locking paradigm rather than a copying one is that it allows a zero-copy implementation in the common case. I've no strong objections to a copying interface, however it must be implementation agnostic. Paul