linux-kernel - Re: FUSE: regression when clearing setuid bits on chown

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: <CAJfpegve7w0YbZy56S1jMrvC2N69JUUvh+W+t=yM3_1eA85nrg@mail.gmail.com>
Date:   Tue, 6 Dec 2016 15:51:34 +0100
From:   Miklos Szeredi <miklos@...redi.hu>
To:     Jeff Layton <jlayton@...hat.com>
Cc:     linux-fsdevel <linux-fsdevel@...r.kernel.org>,
        open list <linux-kernel@...r.kernel.org>,
        Al Viro <viro@...iv.linux.org.uk>
Subject: Re: FUSE: regression when clearing setuid bits on chown

On Tue, Dec 6, 2016 at 3:45 PM, Jeff Layton <jlayton@...hat.com> wrote:

>> @@ -1739,8 +1739,6 @@ static int fuse_setattr(struct dentry *e
>>                * This should be done on write(), truncate() and chown().
>>                */
>>               if (!fc->handle_killpriv) {
>
> One more thing too. I don't think we really want to monkey with the mode
> at all if there is a request to set the mode already in the request. So
> maybe this should be:
>
>     if (!fc->handle_killpriv && !(attr->ia_mode & ATTR_MODE))
>
> Granted that won't generally happen from normal process context, but we
> could have knfsd in here too and I think that's possible from there.

Apparently this can't happen even from knfsd; notify_change() has this comment:

    /*
     * We now pass ATTR_KILL_S*ID to the lower level setattr function so
     * that the function has the ability to reinterpret a mode change
     * that's due to these bits. This adds an implicit restriction that
     * no function will ever call notify_change with both ATTR_MODE and
     * ATTR_KILL_S*ID set.
     */
    if ((ia_valid & (ATTR_KILL_SUID|ATTR_KILL_SGID)) &&
        (ia_valid & ATTR_MODE))
        BUG();

Thanks,
Miklos