Date: Fri, 15 Aug 2014 02:55:10 +0200 From: Mateusz Guzik <mjguzik@gmail.com> To: freebsd-arch@freebsd.org Cc: Robert Watson <rwatson@FreeBSD.org>, Johan Schuijt <johan@transip.nl>, Konstantin Belousov <kib@FreeBSD.org> Subject: [PATCH 0/2] plug capability races Message-ID: <1408064112-573-1-git-send-email-mjguzik@gmail.com>
next in thread | raw e-mail | index | archive | help
fget_unlocked currently reads 'fde' which is a structure consisting of serveral fields. In effect the read is inatomic and may result in obtaining file pointer with stale or incorrect capabilities. Example race is with dup2. Side effect is that capability checks can be circumvented. Proposed way to fix it is with the help of sequence counters. Patchset assumes stuff from 'Getting rid of atomic_load_acq_int(&fdp->fd_nfiles)) from fget_unlocked' ( http://lists.freebsd.org/pipermail/freebsd-arch/2014-July/015550.html ) is applied. There is no technical dependency between patches (apart from READ_ONCE), but this patch amortizes performance hit introduced with seqlock. So this introduces a measurable hit with a microbenchmark (16 threads reading from a pipe which fails with EAGAIN), but is still much faster than current code with atomic_load_acq_int(&fdp->fd_nfiles). x propernoacq-readpipe-run-sum + seq2-noacq-readpipe-run-sum N Min Max Median Avg Stddev x 20 59479718 59527286 59496714 59499504 13752.968 + 20 54520752 54920054 54829539 54773480 136842.96 Difference at 95.0% confidence -4.72602e+06 +/- 62244.4 -7.94296% +/- 0.104613% (Student's t, pooled s = 97250) There is still one theoretical race unfixed, but I don't believe it matters much. The race is: fp gets reallocated before refcount check. this resuls in returning fp regardless of new caps, but I don't see how this particular race could be exploited. It could be fixed by re-reading entire fde and checking if it changed. -- 2.0.2
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?1408064112-573-1-git-send-email-mjguzik>