Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 6 May 2014 00:25:33 +0400
From:      Chagin Dmitry <dchagin@freebsd.org>
To:        Mateusz Guzik <mjguzik@gmail.com>
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:  <20140505202533.GA9747@dchagin.static.corbina.net>
In-Reply-To: <20140505153218.GA17831@dft-labs.eu>
References:  <201405041559.s44FxWdj053353@svn.freebsd.org> <20140504180749.GA17835@dft-labs.eu> <20140505050204.GA1307@dchagin.static.corbina.net> <20140505153218.GA17831@dft-labs.eu>

next in thread | previous in thread | raw e-mail | index | archive | help

--/9DWx/yDrRhgMJTb
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Mon, May 05, 2014 at 05:32:18PM +0200, Mateusz Guzik wrote:
> 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
> > > >=20
> > > > Log:
> > > >   Implement epoll family system calls. This is a tiny wrapper aroun=
d 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.
> > > >  =20
> > > >   Initial patch developed by rdivacky in 2007, then extended by
> > > >   Yuri Victorovich @ r255672 and finished by me.
> > > >=20
> > >=20
> > > I'm not happy with this. The code is full of fp <-> fd lookup races.
> > >=20
> >=20
> > Hi, Mateusz. Thanks for the reply.
> >=20
> > 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?:
> >=20
> >=20
> > diff --git a/sys/compat/linux/linux_event.c b/sys/compat/linux/linux_ev=
ent.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_epo=
ll_ctl_args *args)
> >  	int nchanges =3D 0;
> >  	int error;
> > =20
> > -	if (args->epfd =3D=3D args->fd)
> > -		return (EINVAL);
> > -
> >  	if (args->op !=3D LINUX_EPOLL_CTL_DEL) {
> >  		error =3D copyin(args->event, &le, sizeof(le));
> >  		if (error !=3D 0)
> > @@ -359,6 +356,12 @@ linux_epoll_ctl(struct thread *td, struct linux_ep=
oll_ctl_args *args)
> >  	if (error !=3D 0)
> >  		goto leave1;
> > =20
> > +	/* Linux disallows spying on himself */
> > +	if (epfp =3D=3D fp) {
> > +		error =3D EINVAL;
> > +		goto leave0;
> > +	}
> > +
> >  	ciargs.changelist =3D kev;
> > =20
>=20
> yeah, that's much better :)
>=20

commited, thanks! I was really concentrated on the main path and missied
this obvious detail.




> > > There is no validation you got epoll fd either.
> > >=20
> > > 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 se=
t:
> > >                  * EVFILT_READ and EVFILT_WRITE, ignoring any errors
> > >                  */
> > >                 error =3D epoll_delete_all_events(td, epfp, args->fd);
> > >=20
> > > Again a lookup.
> > >=20
> > > Whether this particular problem could be used to do something nasty I=
 don't
> > > know, but playing like this is asking for trouble.
> > >=20
> > > The only solution I see is to modify kqueue functions to accept fps.
> > >=20
> >=20
> > reason? to prevent extra fget? or something else?
> >=20
>=20
> 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.
>=20
> That said, the concern is that a malicious user could try to work
> something out from this.
>=20

Hi, may be enough to keep file ref?

--=20
Have fun!
chd

--/9DWx/yDrRhgMJTb
Content-Type: application/pgp-signature

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1

iEYEARECAAYFAlNn870ACgkQ0t2Tb3OO/O3BnwCfSR37JojvEPBFdDb66E2aESFv
AOgAnjeCPWCg9LSqSey1OSJWeeMETowh
=307q
-----END PGP SIGNATURE-----

--/9DWx/yDrRhgMJTb--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20140505202533.GA9747>