From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1I8F7D-0002uD-7M for qemu-devel@nongnu.org; Tue, 10 Jul 2007 08:46:51 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1I8F7B-0002tr-NW for qemu-devel@nongnu.org; Tue, 10 Jul 2007 08:46:50 -0400 Received: from [199.232.76.173] (helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1I8F7B-0002tl-JN for qemu-devel@nongnu.org; Tue, 10 Jul 2007 08:46:49 -0400 Received: from mail.codesourcery.com ([65.74.133.4]) by monty-python.gnu.org with esmtps (TLS-1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1I8EzZ-0004fK-DY for qemu-devel@nongnu.org; Tue, 10 Jul 2007 08:38:57 -0400 From: Paul Brook Subject: Re: [Qemu-devel] [PATCH] linux-user EFAULT implementation Date: Tue, 10 Jul 2007 13:38:43 +0100 References: <4692A55D.5050102@bellard.org> In-Reply-To: MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200707101338.44885.paul@codesourcery.com> Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: Stuart Anderson 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