Date: Mon, 5 May 2014 17:32:18 +0200 From: Mateusz Guzik <mjguzik@gmail.com> To: Chagin Dmitry <dchagin@freebsd.org> Cc: src-committers@freebsd.org, svn-src-user@freebsd.org 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> In-Reply-To: <20140505050204.GA1307@dchagin.static.corbina.net> References: <201405041559.s44FxWdj053353@svn.freebsd.org> <20140504180749.GA17835@dft-labs.eu> <20140505050204.GA1307@dchagin.static.corbina.net>
next in thread | previous in thread | raw e-mail | index | archive | help
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 <mjguzik gmail.com>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20140505153218.GA17831>