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

[Mateusz Guzik]

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.

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.

-- 
Mateusz Guzik