linux-kernel - Re: Bisected regression: iterate_fd() selinux change affects flash plugin

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CACLa4pvixB3Ehz-Ds+opvMF73pNwQJxpNx=02qy94ufJag2qwA@mail.gmail.com>
Date:	Fri, 16 Nov 2012 14:58:46 -0500
From:	Eric Paris <eparis@...isplace.org>
To:	Pavel Roskin <proski@....org>
Cc:	Al Viro <viro@...iv.linux.org.uk>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	SE-Linux <selinux@...ho.nsa.gov>
Subject: Re: Bisected regression: iterate_fd() selinux change affects flash plugin

On Mon, Nov 12, 2012 at 11:57 AM, Pavel Roskin <proski@....org> wrote:
> Quoting Eric Paris <eparis@...isplace.org>:
>
>> OMG this +1 -1 stuff is nuts...

Ping, Al.

int iterate_fd(struct files_struct *files, unsigned n,
[snip]
        while (!res && n < fdt->max_fds) {
                file = rcu_dereference_check_fdtable(files, fdt->fd[n++]);
                if (file)
                        res = f(p, file, n);
        }
        spin_unlock(&files->file_lock);
        return res;

So we increment n (the file descriptor number) in the dereference,
then pass that (wrong) number to f().

Every single f() (including SELinux, the cause of this bug) returns
fd+1 (so now we are up by 2).  Then all of the users of iterate fd
actually use fd-1 (which is wrong)

Why not have iterate_fd return -ENOENT on no entries and stop all of
the stupid games?  We fix the real bug (the above function should do
the n++ after the f() call, and the interface is sane to design
against...

-Eric
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/