Re: [PATCH v2] vsprintf: kptr_restrict is okay in IRQ when 2

[Kees Cook]

On Fri, Feb 5, 2016 at 2:03 PM, Jason A. Donenfeld <Jason@xxxxxxxxx> wrote:
> The kptr_restrict flag, when set to 1, only prints the kernel
> address when the user has CAP_SYSLOG. When it is set to 2, the
> kernel address is always printed as zero. When set to 1, this
> needs to check whether or not we're in IRQ. However, when set to
> 2, this check is unneccessary, and produces confusing results
> in dmesg. Thus, only make sure we're not in IRQ when mode 1 is
> used, but not mode 2.
>
> Signed-off-by: Jason A. Donenfeld <Jason@xxxxxxxxx>

Thanks!

Acked-by: Kees Cook <keescook@xxxxxxxxxxxx>

-Kees

> ---
>  lib/vsprintf.c | 26 +++++++++++++-------------
>  1 file changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> index 80d8ce5..ee1e24e 100644
> --- a/lib/vsprintf.c
> +++ b/lib/vsprintf.c
> @@ -1605,22 +1605,23 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr,
>                         return buf;
>                 }
>         case 'K':
> -               /*
> -                * %pK cannot be used in IRQ context because its test
> -                * for CAP_SYSLOG would be meaningless.
> -                */
> -               if (kptr_restrict && (in_irq() || in_serving_softirq() ||
> -                                     in_nmi())) {
> -                       if (spec.field_width == -1)
> -                               spec.field_width = default_width;
> -                       return string(buf, end, "pK-error", spec);
> -               }
> -
>                 switch (kptr_restrict) {
>                 case 0:
>                         /* Always print %pK values */
>                         break;
>                 case 1: {
> +                       const struct cred *cred;
> +
> +                       /*
> +                        * kptr_restrict==2 cannot be used in IRQ context because
> +                        * its test for CAP_SYSLOG would be meaningless.
> +                        */
> +                       if (in_irq() || in_serving_softirq() || in_nmi()) {
> +                               if (spec.field_width == -1)
> +                                       spec.field_width = default_width;
> +                               return string(buf, end, "pK-error", spec);
> +                       }
> +
>                         /*
>                          * Only print the real pointer value if the current
>                          * process has CAP_SYSLOG and is running with the
> @@ -1630,8 +1631,7 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr,
>                          * leak pointer values if a binary opens a file using
>                          * %pK and then elevates privileges before reading it.
>                          */
> -                       const struct cred *cred = current_cred();
> -
> +                       cred = current_cred();
>                         if (!has_capability_noaudit(current, CAP_SYSLOG) ||
>                             !uid_eq(cred->euid, cred->uid) ||
>                             !gid_eq(cred->egid, cred->gid))
> --
> 2.7.0
>



-- 
Kees Cook
Chrome OS & Brillo Security