Re: [PATCH 2/2] Revert "pstore: Honor dmesg_restrict sysctl on dmesg dumps"

[Kees Cook]

On Wed, Aug 16, 2017 at 12:59 AM, Sergey Senozhatsky
<sergey.senozhatsky.work@xxxxxxxxx> 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