linux-kernel - Re: [PATCH 16/32] staging: gasket: always allow root open for write

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 for Android: free password hash cracker in your pocket

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

Date:   Tue, 17 Jul 2018 12:04:31 -0700
From:   Todd Poynor <toddpoynor@...il.com>
To:     Joe Perches <joe@...ches.com>
Cc:     Dan Carpenter <dan.carpenter@...cle.com>,
        Rob Springer <rspringer@...gle.com>,
        John Joseph <jnjoseph@...gle.com>,
        Ben Chan <benchan@...omium.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        devel@...verdev.osuosl.org, Zhongze Hu <frankhu@...omium.org>,
        lkml <linux-kernel@...r.kernel.org>,
        Simon Que <sque@...omium.org>,
        Guenter Roeck <groeck@...omium.org>,
        Todd Poynor <toddpoynor@...gle.com>,
        Dmitry Torokhov <dtor@...omium.org>
Subject: Re: [PATCH 16/32] staging: gasket: always allow root open for write

On Tue, Jul 17, 2018 at 11:50 AM, Joe Perches <joe@...ches.com> wrote:
> On Tue, 2018-07-17 at 13:22 +0300, Dan Carpenter wrote:
>> On Mon, Jul 16, 2018 at 07:09:10PM -0700, Todd Poynor wrote:
>> > --- a/drivers/staging/gasket/apex_driver.c
>> > +++ b/drivers/staging/gasket/apex_driver.c
>> > @@ -630,13 +630,10 @@ static bool is_gcb_in_reset(struct gasket_dev *gasket_dev)
>> >  static uint apex_ioctl_check_permissions(struct file *filp, uint cmd)
>>
>> This function name is a bit of out of date.
>>
>> >  {
>> >     struct gasket_dev *gasket_dev = filp->private_data;
>> > -   int root = capable(CAP_SYS_ADMIN);
>> > -   int is_owner = gasket_dev->dev_info.ownership.is_owned &&
>> > -                  current->tgid == gasket_dev->dev_info.ownership.owner;
>> > +   fmode_t write;
>> >
>> > -   if (root || is_owner)
>> > -           return 1;
>> > -   return 0;
>> > +   write = filp->f_mode & FMODE_WRITE;
>> > +   return write;
>>
>> This doesn't match the comment because it returns 0x2 or zero.

Will fix, I noticed this earlier and forgot to address it.

>>
>>       return !!(filp->f_mode & FMODE_WRITE);
>
> Or maybe change the functions to return bool

And there's another patch farther down in the stack that does that,
too, will keep them separate and fix up this intermediate patch for
now.

Thanks -- Todd