svn commit: r265327 - in user/dchagin/lemul/sys: amd64/linux amd64/linux32 compat/linux conf i386/linux modules/linux modules/linux64

[Chagin Dmitry]

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;
 
 	switch (args->op) {




> Example:
> 
> linux_epoll_ctl
>         if (args->epfd == args->fd)
>                 return (EINVAL);
> [..]
>         error = fget(td, args->epfd, &rights, &epfp);
>         if (error != 0)
>                 return (error);
> 
>          /* Protect user data vector from incorrectly supplied fd. */
>         error = fget(td, args->fd, &rights, &fp);
>         if (error != 0)
>                 goto leave1;
> 
> fds could resolve to the same fp right from the start and even if that
> was note done an attacker could arrange that after the check.
> 
> 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?

-- 
Have fun!
chd
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://lists.freebsd.org/pipermail/svn-src-user/attachments/20140505/5988d944/attachment.sig>