Date: Mon, 9 Jun 2014 01:00:59 +0200 From: Mateusz Guzik <mjguzik@gmail.com> To: Adrian Chadd <adrian@freebsd.org> Cc: Robert Watson <rwatson@freebsd.org>, "freebsd-arch@freebsd.org" <freebsd-arch@freebsd.org> Subject: Re: capability races (was: Re: Seeing ENOTCAPABLE from write FDs in kqueue?) Message-ID: <20140608230059.GB5030@dft-labs.eu> In-Reply-To: <20140608220000.GA5030@dft-labs.eu> References: <CAJ-VmonJaqg=WV0eTxknOQr51E5qdhDU=MdoCywz-hwZ57jj6w@mail.gmail.com> <20140608220000.GA5030@dft-labs.eu>
next in thread | previous in thread | raw e-mail | index | archive | help
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 <mjguzik gmail.com>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20140608230059.GB5030>