From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754153Ab3F0SoX (ORCPT ); Thu, 27 Jun 2013 14:44:23 -0400 Received: from mail.openrapids.net ([64.15.138.104]:56970 "EHLO blackscsi.openrapids.net" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751535Ab3F0SoW (ORCPT ); Thu, 27 Jun 2013 14:44:22 -0400 Date: Thu, 27 Jun 2013 14:44:17 -0400 From: Mathieu Desnoyers To: Greg Kroah-Hartman Cc: "David 'Digit' Turner" , Xiaohui Xin , Yunhong Jiang , Jun Nakajima , Tom Keel , Alan Cox , linux-kernel@vger.kernel.org Subject: Re: goldfish driver: Missing get_user_pages in goldfish_pipe_read_write() Message-ID: <20130627184417.GA31031@Krystal> References: <20130627173532.GA29405@Krystal> <20130627174651.GA19874@kroah.com> <20130627180344.GA29919@Krystal> <20130627181449.GA25159@kroah.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20130627181449.GA25159@kroah.com> X-Editor: vi X-Info: http://www.efficios.com User-Agent: Mutt/1.5.18 (2008-05-17) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org * Greg Kroah-Hartman (gregkh@linuxfoundation.org) wrote: > On Thu, Jun 27, 2013 at 02:03:44PM -0400, Mathieu Desnoyers wrote: > > * Greg Kroah-Hartman (gregkh@linuxfoundation.org) wrote: > > > On Thu, Jun 27, 2013 at 01:35:32PM -0400, Mathieu Desnoyers wrote: > > > > The following code snippet: > > > > > > > > drivers/platform/goldfish/goldfish_pipe.c: > > > > goldfish_pipe_read_write() > > > > > > > > /* Ensure that the corresponding page is properly mapped */ > > > > /* FIXME: this isn't safe or sufficient - use get_user_pages */ > > > > if (is_write) { > > > > char c; > > > > /* Ensure that the page is mapped and readable */ > > > > if (__get_user(c, (char __user *)address)) { > > > > if (!ret) > > > > ret = -EFAULT; > > > > break; > > > > } > > > > } else { > > > > /* Ensure that the page is mapped and writable */ > > > > if (__put_user(0, (char __user *)address)) { > > > > if (!ret) > > > > ret = -EFAULT; > > > > break; > > > > } > > > > } > > > > > > > > Seems to lack the kind of validation required to make it fail properly > > > > if the memory range is not fully populated. > > > > > > This is an emulated platform only, so it's probably not a big deal, > > > right? > > > > You're probably right. As long as it is not configured and somehow still > > accessible on a production device. > > There is no device that could ever be "production" for this emulated > hardware platform, unless a desktop system wants to "emulate" the > android userspace system somehow. Then it could be "real", but even > then, how would a user application get access to this codepath? I think it would depend on the user access rights given to /dev/qemu_pipe. Assuming it would be only accessible by root, this would minimize the risks even more. Thanks, Mathieu -- Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com