From owner-freebsd-arch@FreeBSD.ORG Sun Jun 8 23:01:05 2014 Return-Path: Delivered-To: freebsd-arch@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 A11A726C; Sun, 8 Jun 2014 23:01:05 +0000 (UTC) Received: from mail-wi0-x233.google.com (mail-wi0-x233.google.com [IPv6:2a00:1450:400c:c05::233]) (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 C0ACC216F; Sun, 8 Jun 2014 23:01:04 +0000 (UTC) Received: by mail-wi0-f179.google.com with SMTP id bs8so3408470wib.0 for ; Sun, 08 Jun 2014 16:01:02 -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=+jd8s9noM/r+xbXZ96bheCPPHxmwrp3C3DBbPJ/mNGI=; b=XBybRoTp+QUTYUeIDFJGpFvtky9hksvM4op8H0e2w1IidbkvxXD8BNOF7TpJpYWBiE vxoToSFWg7/hCz6Es/EjP3JdQNvEXpMN7XW1UWX0gr7FSXIYJ1QLry0tgr7ZxhR3XVUi R53YZ6rZKsHAQADYL/FEv0xGV/kKlOeXymQ3ARSMh5L3DOVYjgEtxIW2x6v6GjNZhhIC i4UNHrZbxCks+DyKKjop9zGbzYMatJiPIcAQ92R7pAr33+NvM+OygsGTfOLqSTEN07lQ qGESmquSB97SJrvhQ/a46feLhKY5M7g99ZKscMAcb+WcZGBOzIvADi25MLWZjol8pmL2 O2jg== X-Received: by 10.180.38.38 with SMTP id d6mr24009720wik.12.1402268462018; Sun, 08 Jun 2014 16:01:02 -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 s10sm22386691wjs.29.2014.06.08.16.01.00 for (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Sun, 08 Jun 2014 16:01:01 -0700 (PDT) Date: Mon, 9 Jun 2014 01:00:59 +0200 From: Mateusz Guzik To: Adrian Chadd Subject: Re: capability races (was: Re: Seeing ENOTCAPABLE from write FDs in kqueue?) Message-ID: <20140608230059.GB5030@dft-labs.eu> References: <20140608220000.GA5030@dft-labs.eu> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20140608220000.GA5030@dft-labs.eu> User-Agent: Mutt/1.5.21 (2010-09-15) Cc: Robert Watson , "freebsd-arch@freebsd.org" X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 08 Jun 2014 23:01:05 -0000 On Mon, Jun 09, 2014 at 12:00:00AM +0200, Mateusz Guzik wrote: > Current support for capabilities is racy in respect to somewhat > misbehaving programs. > > When fp is being installed under given fd number it is available before > capabilities are copied. > > This is because fget_unlocked only gets fp atomatically, but caps can be > modified irrespectively to that. > > This has 2 problems: > - if the code is trying to access fd before the syscall returns, it may > get ENOTCAPABLE (minor) > - if the code is racing with dup2 it can access given fp in a way > prevented by not-yet-installed caps (needs fixing) > > In url below I provide two reproducers: > http://people.freebsd.org/~mjg/patches/caps-race/ > > One will open+close in 1 thread + read in 7 threads trying to get > ENOTCAPABLE. > > Second one will create a fd with stripped caps and dup2 capped/uncapped > fd in 1 thread + read in 7 threads trying to succeed. > > I propose to fix the problem by introducing sequence counters. > > I tested the patch below succesfully with my reproducers. > > Note that the patch is somewhat incomplete (ioctls are not handled) and > has stuff in wrong places, I can do necessary tidy ups if it is agreed > the approach taken is ok. > [..] > +static inline seq_t > +seq_read(seq_t *seqp) > +{ > + seq_t ret; > + > + ret = *seqp; > + rmb(); > + > + return (ret); > +} Doh, rmb() should be before read. As you can see my memory-barrier-fu is not too strong. > for (;;) { > - fp = fdp->fd_ofiles[fd].fde_file; > + seq1 = seq_read(&fdp->fd_ofiles[fd].fde_seq); > + if (seq_in_modify(seq1)) > + continue; ... and this should try to yield some cpu cycles (pause?) So, benchmarks: http://people.freebsd.org/~mjg/patches/caps-race/dup2-bench.c 1 thread does dup2 in a loop 7 threads fget() the same fd and fail quickly on a crappy 8-way machine I get the following: x with-barriers-ops-2 + without-barriers-ops +--------------------------------------------------------------------------------------------------------------------------------------+ | x + | | x + + ++ | | xxxx + ++++ | | xxxxxx + +++++ | | x xxxxxx ++ +++++ | | xx xxxxxx +++ +++++ | | xx xxxxxxxx x + +++ +++++ | | xxxxxxxxxxxx x + + +++++++++++ + | |xx xx xxxxxxxxxxxx x x x x +++ + +++++++++++ ++ + | |xx xxxxxxxxxxxxxxx x x xxx x xxx x x ++++++++ +++++++++++ +++ ++ + ++ +| | |_______MA________| |______A______| | +--------------------------------------------------------------------------------------------------------------------------------------+ N Min Max Median Avg Stddev x 100 11735712 12597730 12040847 12072225 186613.68 + 100 13732889 14499008 14051852 14042094 145074.11 Difference at 95.0% confidence 1.96987e+06 +/- 46328.7 16.3174% +/- 0.383763% (Student's t, pooled s = 167139) It would be good if someone with some time and access to higher-cpu-count machine could test it better. -- Mateusz Guzik