linux-kernel - Re: [PATCH 2/2] Revert "pstore: Honor dmesg

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]

Date:   Wed, 16 Aug 2017 08:38:59 -0700
From:   Kees Cook <keescook@...omium.org>
To:     Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>
Cc:     LKML <linux-kernel@...r.kernel.org>,
        Nick Kralevich <nnk@...gle.com>,
        Sebastian Schmidt <yath@...h.de>,
        Tony Luck <tony.luck@...el.com>,
        Anton Vorontsov <anton@...msg.org>,
        Colin Cross <ccross@...roid.com>,
        Petr Mladek <pmladek@...e.com>,
        Sergey Senozhatsky <sergey.senozhatsky@...il.com>,
        Steven Rostedt <rostedt@...dmis.org>,
        Patrick Tjin <pattjin@...gle.com>,
        Mark Salyzyn <salyzyn@...gle.com>
Subject: Re: [PATCH 2/2] Revert "pstore: Honor dmesg_restrict sysctl on dmesg dumps"

On Wed, Aug 16, 2017 at 12:59 AM, Sergey Senozhatsky
<sergey.senozhatsky.work@...il.com> wrote:
> On (08/10/17 13:36), Kees Cook wrote:
> [..]
>> -static int pstore_check_syslog_permissions(struct pstore_private *ps)
>> -{
>> -     switch (ps->record->type) {
>> -     case PSTORE_TYPE_DMESG:
>> -     case PSTORE_TYPE_CONSOLE:
>> -             return check_syslog_permissions(SYSLOG_ACTION_READ_ALL,
>> -                     SYSLOG_FROM_READER);
>> -     default:
>> -             return 0;
>> -     }
>> -}
>> -
>>  static ssize_t pstore_file_read(struct file *file, char __user *userbuf,
>>                                               size_t count, loff_t *ppos)
>>  {
>> @@ -163,10 +150,6 @@ static int pstore_file_open(struct inode *inode, struct file *file)
>>       int err;
>>       const struct seq_operations *sops = NULL;
>>
>> -     err = pstore_check_syslog_permissions(ps);
>> -     if (err)
>> -             return err;
>> -
>>       if (ps->record->type == PSTORE_TYPE_FTRACE)
>>               sops = &pstore_ftrace_seq_ops;
>>
>> @@ -204,11 +187,6 @@ static int pstore_unlink(struct inode *dir, struct dentry *dentry)
>>  {
>>       struct pstore_private *p = d_inode(dentry)->i_private;
>>       struct pstore_record *record = p->record;
>> -     int err;
>> -
>> -     err = pstore_check_syslog_permissions(p);
>> -     if (err)
>> -             return err;
>
> it's hard to review security related patches :)
>
> so, effectively, `dmesg_restrict' does not work for pstore anymore? wouldn't
> that be a problem? one more thing, doesn't it affect the consistency -- we
> respect the `dmesg_restrict' restrictions, except that we ignore it when
> access pstore? or do I completely misunderstand the change? sorry if so.

This revert is combined with the commit before it which changes the
perms of the pstorefs rootdir to 750. Privacy is retained, but now a
system owner can modify access to specific entirely unprivileged
groups (that do no require CAP_SYSLOG). Note, also, that pstore
(normally) shows only the prior boot's console and crash.

-Kees

-- 
Kees Cook
Pixel Security