From owner-svn-src-user@FreeBSD.ORG Mon May 5 15:32:26 2014 Return-Path: Delivered-To: svn-src-user@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 819604C7; Mon, 5 May 2014 15:32:26 +0000 (UTC) Received: from mail-wi0-x231.google.com (mail-wi0-x231.google.com [IPv6:2a00:1450:400c:c05::231]) (using TLSv1 with cipher ECDHE-RSA-RC4-SHA (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id C36FD1A3D; Mon, 5 May 2014 15:32:25 +0000 (UTC) Received: by mail-wi0-f177.google.com with SMTP id f8so1945463wiw.10 for ; Mon, 05 May 2014 08:32:22 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=date:from:to:cc:subject:message-id:references:mime-version :content-type:content-disposition:in-reply-to:user-agent; bh=jqxqBr1j1d1PIyoYyLoFV9FCauVhzIUg/nNi0MMS9dA=; b=Y+8sK564oCl19dmW/U2jeiFV5F3knAECnMTI2JSL6Ul9Sq43s8nne02+8k9I3UgYc2 uMBWoRF2ugN+kn59dWb9YAz8wnUhRcOiayojF3TvsVIvrBJMzjVgqxoyEJx0u5zCG2e/ u788J/5IBZ+RDvMjStkT0MBE65R1H6eqUXQLcSeDkxVWDMey/4dgz8nyo0KxY4c7gh0z 5EM4QIKjtMyb1bsi06gSspqScBOqwD51yMM14qGf2Yd5hom1paYPVqFgsny7zmOO5I3B +yoGzBnHNJSGhmsWx/qnBux0k5/jBUNNuYIVXgUC68btxlE380AWIKzYyFLA89XF4Nod rUHA== X-Received: by 10.194.59.43 with SMTP id w11mr2390303wjq.65.1399303942547; Mon, 05 May 2014 08:32:22 -0700 (PDT) Received: from dft-labs.eu (n1x0n-1-pt.tunnel.tserv5.lon1.ipv6.he.net. [2001:470:1f08:1f7::2]) by mx.google.com with ESMTPSA id f11sm7116078wiv.1.2014.05.05.08.32.20 for (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Mon, 05 May 2014 08:32:21 -0700 (PDT) Date: Mon, 5 May 2014 17:32:18 +0200 From: Mateusz Guzik To: Chagin Dmitry Subject: Re: svn commit: r265327 - in user/dchagin/lemul/sys: amd64/linux amd64/linux32 compat/linux conf i386/linux modules/linux modules/linux64 Message-ID: <20140505153218.GA17831@dft-labs.eu> References: <201405041559.s44FxWdj053353@svn.freebsd.org> <20140504180749.GA17835@dft-labs.eu> <20140505050204.GA1307@dchagin.static.corbina.net> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20140505050204.GA1307@dchagin.static.corbina.net> User-Agent: Mutt/1.5.21 (2010-09-15) Cc: src-committers@freebsd.org, svn-src-user@freebsd.org X-BeenThere: svn-src-user@freebsd.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: "SVN commit messages for the experimental " user" src tree" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 05 May 2014 15:32:26 -0000 On Mon, May 05, 2014 at 09:02:04AM +0400, Chagin Dmitry wrote: > On Sun, May 04, 2014 at 08:07:49PM +0200, Mateusz Guzik wrote: > > On Sun, May 04, 2014 at 03:59:32PM +0000, Dmitry Chagin wrote: > > > Author: dchagin > > > Date: Sun May 4 15:59:32 2014 > > > New Revision: 265327 > > > URL: http://svnweb.freebsd.org/changeset/base/265327 > > > > > > Log: > > > Implement epoll family system calls. This is a tiny wrapper around kqueue > > > to implement epoll subset of functionality. The kqueue user data are > > > 32bit on i386 which is not enough for epoll user data, so we keep user > > > data in the proc emuldata. > > > > > > Initial patch developed by rdivacky in 2007, then extended by > > > Yuri Victorovich @ r255672 and finished by me. > > > > > > > I'm not happy with this. The code is full of fp <-> fd lookup races. > > > > Hi, Mateusz. Thanks for the reply. > > As far as I understand you, check epfd against fd is useless. > FreeBSD does not check that it's the same file.But we must. > Is it correct?: > > > diff --git a/sys/compat/linux/linux_event.c b/sys/compat/linux/linux_event.c > index ee70b9c..054df14 100644 > --- a/sys/compat/linux/linux_event.c > +++ b/sys/compat/linux/linux_event.c > @@ -339,9 +339,6 @@ linux_epoll_ctl(struct thread *td, struct linux_epoll_ctl_args *args) > int nchanges = 0; > int error; > > - if (args->epfd == args->fd) > - return (EINVAL); > - > if (args->op != LINUX_EPOLL_CTL_DEL) { > error = copyin(args->event, &le, sizeof(le)); > if (error != 0) > @@ -359,6 +356,12 @@ linux_epoll_ctl(struct thread *td, struct linux_epoll_ctl_args *args) > if (error != 0) > goto leave1; > > + /* Linux disallows spying on himself */ > + if (epfp == fp) { > + error = EINVAL; > + goto leave0; > + } > + > ciargs.changelist = kev; > yeah, that's much better :) > > There is no validation you got epoll fd either. > > > > Further down: > > switch (args->op) { > > case LINUX_EPOLL_CTL_MOD: > > /* > > * We don't memorize which events were set for this FD > > * on this level, so just delete all we could have set: > > * EVFILT_READ and EVFILT_WRITE, ignoring any errors > > */ > > error = epoll_delete_all_events(td, epfp, args->fd); > > > > Again a lookup. > > > > Whether this particular problem could be used to do something nasty I don't > > know, but playing like this is asking for trouble. > > > > The only solution I see is to modify kqueue functions to accept fps. > > > > reason? to prevent extra fget? or something else? > Having multpiple lookups for the same fd number may lead to different fps, which may or may not be used to cause inconsistencies which in turn may or may not be exploitable to either crash the kernel or escalate privileges. That said, the concern is that a malicious user could try to work something out from this. -- Mateusz Guzik