Re: [PATCH v3] kptr_restrict for hiding kernel pointers

[Eric Paris <eparis@parisplace.org>]

On Sat, Dec 18, 2010 at 4:07 PM, Eric Paris <eparis@parisplace.org> wrote:
> On Sat, Dec 18, 2010 at 12:20 PM, Dan Rosenberg
> <drosenberg@vsecurity.com> wrote:
>
>> @@ -1035,6 +1038,26 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr,
>>                return buf + vsnprintf(buf, end - buf,
>>                                       ((struct va_format *)ptr)->fmt,
>>                                       *(((struct va_format *)ptr)->va));
>> +       case 'K':
>> +               /*
>> +                * %pK cannot be used in IRQ context because it tests
>> +                * CAP_SYSLOG.
>> +                */
>> +               if (in_irq() || in_serving_softirq() || in_nmi())
>> +                       WARN_ONCE(1, "%%pK used in interrupt context.\n");
>> +
>> +               if (!kptr_restrict)
>> +                       break;          /* %pK does not obscure pointers */
>> +
>> +               if ((kptr_restrict != 2) && capable(CAP_SYSLOG))
>> +                       break;          /* privileged apps expose pointers,
>> +                                          unless kptr_restrict is 2 */
>
> I would suggest has_capability_noaudit() since a failure here is not a
> security policy violation it is just a code path choice.
>
> I was confused also by the comment about CAP_SYSLOG and IRQ context.
> You can check CAP_SYSLOG in IRQ context, it's just that the result is
> not going to have any relation to the work being done.  This function
> in general doesn't make sense in that context and I don't think saying
> that has anything to do with CAP_SYSLOG makes that clear....  Unless
> I'm misunderstanding...

Just went back and reread akpm's comments on -v2.  I guess we see it
the same way, I just thought this comment on first glance indicated
that capable() wasn't IRQ safe (it is) not that it just was
meaningless...   I don't think rewriting the comment is necessary.
Sorry for that half of the message....