[RFC] vsprintf: compile error on %09pK

[RFC] vsprintf: compile error on %09pK

[Jaewon Kim]

Hello

I've just changed %09lx to %09pK on my driver code to hide the address, but I
faced compiler error. The %9pK without 0 worked.

Is there restriction on %pK which does now allow %0 ? I've wondered whether I
did wrong or it is a printk problem.

To show easily I tried to add pr_info("%09pK\n", nodemask); in page_alloc.c
Then here's what I did.

$ ARCH=x86 make x86_64_defconfig ; make mm/page_alloc.o
#
# No change to .config
#
  CALL    scripts/checksyscalls.sh
  DESCEND objtool
  INSTALL libsubcmd_headers
  CC      mm/page_alloc.o
In file included from ./include/asm-generic/bug.h:22:0,
                 from ./arch/x86/include/asm/bug.h:87,
                 from ./include/linux/bug.h:5,
                 from ./include/linux/mmdebug.h:5,
                 from ./include/linux/mm.h:6,
                 from mm/page_alloc.c:19:
mm/page_alloc.c: In function ‘__alloc_pages’:
./include/linux/kern_levels.h:5:18: error: '0' flag used with ‘%p’ gnu_printf format [-Werror=format=]
 #define KERN_SOH "\001"  /* ASCII Start Of Header */
                  ^
./include/linux/printk.h:427:11: note: in definition of macro ‘printk_index_wrap’
   _p_func(_fmt, ##__VA_ARGS__);    \
           ^
./include/linux/printk.h:528:2: note: in expansion of macro ‘printk’
  printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
  ^
./include/linux/kern_levels.h:14:19: note: in expansion of macro ‘KERN_SOH’
 #define KERN_INFO KERN_SOH "6" /* informational */
                   ^
./include/linux/printk.h:528:9: note: in expansion of macro ‘KERN_INFO’
  printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
         ^
mm/page_alloc.c:5564:2: note: in expansion of macro ‘pr_info’
  pr_info("%09pK\n", nodemask);
  ^

Jaewon Kim

Re: [RFC] vsprintf: compile error on %09pK

[Sergey Senozhatsky]

On (23/04/03 19:46), Jaewon Kim wrote:
> I've just changed %09lx to %09pK on my driver code to hide the address, but I
> faced compiler error. The %9pK without 0 worked.
> 
> Is there restriction on %pK which does now allow %0 ? I've wondered whether I
> did wrong or it is a printk problem.

I don't think this is %pK limitation. %p should not take modification flags.
E.g. %3p doesn't make sense, we still should print the entire pointer.

Re: [RFC] vsprintf: compile error on %09pK

[Petr Mladek]

On Mon 2023-04-03 19:46:17, Jaewon Kim wrote:
> Hello
> 
> I've just changed %09lx to %09pK on my driver code to hide the address, but I
> faced compiler error. The %9pK without 0 worked.

What exactly do you want to achieve, please?

Note that printk() hashes pointers by default. It means that %p does not
print the value but a hash based on the value.

If you print the same pointer twice, you will see the same hash, so
you know that the pointer is the same. But you do not see the address
so that you could not use the value for a security attack.

See Documentation/core-api/printk-formats.rst

Anyway, the main question if it makes sense to print the pointer value
at all. The address is not useful if it can't be compared with
other pointers or if the data on the address could not be checked.

> Is there restriction on %pK which does now allow %0 ? I've wondered whether I
> did wrong or it is a printk problem.
> 
> To show easily I tried to add pr_info("%09pK\n", nodemask); in page_alloc.c
> Then here's what I did.
> 
> $ ARCH=x86 make x86_64_defconfig ; make mm/page_alloc.o
> #
> # No change to .config
> #
>   CALL    scripts/checksyscalls.sh
>   DESCEND objtool
>   INSTALL libsubcmd_headers
>   CC      mm/page_alloc.o
> In file included from ./include/asm-generic/bug.h:22:0,
>                  from ./arch/x86/include/asm/bug.h:87,
>                  from ./include/linux/bug.h:5,
>                  from ./include/linux/mmdebug.h:5,
>                  from ./include/linux/mm.h:6,
>                  from mm/page_alloc.c:19:
> mm/page_alloc.c: In function ‘__alloc_pages’:
> ./include/linux/kern_levels.h:5:18: error: '0' flag used with ‘%p’ gnu_printf format [-Werror=format=]
>  #define KERN_SOH "\001"  /* ASCII Start Of Header */

As Sergey already wrote. %p does not support any modification flags.

Best Regards,
Petr

Re: [RFC] vsprintf: compile error on %09pK

[Jaewon Kim]

2023년 4월 3일 (월) 오후 9:53, Petr Mladek <pmladek@suse.com>님이 작성:
>
> On Mon 2023-04-03 19:46:17, Jaewon Kim wrote:
> > Hello
> >
> > I've just changed %09lx to %09pK on my driver code to hide the address, but I
> > faced compiler error. The %9pK without 0 worked.
>
> What exactly do you want to achieve, please?

Hello

Thank you for your comment.

I wanted to print phys_addr_t  type value only when kptr_restrict sysctl is
allowed. So I thought I could use %pK for that purpose. And the physical
address is not that long. I wanted to make that length short like 9 hex.

>
> Note that printk() hashes pointers by default. It means that %p does not
> print the value but a hash based on the value.
>
> If you print the same pointer twice, you will see the same hash, so
> you know that the pointer is the same. But you do not see the address
> so that you could not use the value for a security attack.
>
> See Documentation/core-api/printk-formats.rst
>
> Anyway, the main question if it makes sense to print the pointer value
> at all. The address is not useful if it can't be compared with
> other pointers or if the data on the address could not be checked.
>
> > Is there restriction on %pK which does now allow %0 ? I've wondered whether I
> > did wrong or it is a printk problem.
> >
> > To show easily I tried to add pr_info("%09pK\n", nodemask); in page_alloc.c
> > Then here's what I did.
> >
> > $ ARCH=x86 make x86_64_defconfig ; make mm/page_alloc.o
> > #
> > # No change to .config
> > #
> >   CALL    scripts/checksyscalls.sh
> >   DESCEND objtool
> >   INSTALL libsubcmd_headers
> >   CC      mm/page_alloc.o
> > In file included from ./include/asm-generic/bug.h:22:0,
> >                  from ./arch/x86/include/asm/bug.h:87,
> >                  from ./include/linux/bug.h:5,
> >                  from ./include/linux/mmdebug.h:5,
> >                  from ./include/linux/mm.h:6,
> >                  from mm/page_alloc.c:19:
> > mm/page_alloc.c: In function ‘__alloc_pages’:
> > ./include/linux/kern_levels.h:5:18: error: '0' flag used with ‘%p’ gnu_printf format [-Werror=format=]
> >  #define KERN_SOH "\001"  /* ASCII Start Of Header */
>
> As Sergey already wrote. %p does not support any modification flags.

Okay, then we can't use %09pK. I've just wondered because %9pK works.

BR
Jaewon Kim

>
> Best Regards,
> Petr

Re: [RFC] vsprintf: compile error on %09pK

[Sergey Senozhatsky]

On (23/04/03 23:39), Jaewon Kim wrote:
[..]
> 
> Okay, then we can't use %09pK. I've just wondered because %9pK works.

This is not per printk / kernel, Wformat warnings come from the
compiler.

Per C11 (7.21.6 6):
 0  For d, i, o, u, x, X, a, A, e, E, f, F, g, and G conversions, leading zeros
    (following any indication of sign or base) are used to pad to the field width
    rather than performing space padding
    ...
    For other conversions, the behavior is undefined.

So using 0 for p should trigger an undefined behavior as far as the
standard C concerned. Unless I'm missing something.

RE: [RFC] vsprintf: compile error on %09pK

[Jaewon Kim]

>[..]
>> 
>> Okay, then we can't use %09pK. I've just wondered because %9pK works.
>
>This is not per printk / kernel, Wformat warnings come from the
>compiler.
>
>Per C11 (7.21.6 6):
> 0  For d, i, o, u, x, X, a, A, e, E, f, F, g, and G conversions, leading zeros
>    (following any indication of sign or base) are used to pad to the field width
>    rather than performing space padding
>    ...
>    For other conversions, the behavior is undefined.
>
>So using 0 for p should trigger an undefined behavior as far as the
>standard C concerned. Unless I'm missing something.

Thank your for the information about the leading 0 case. By the way do you know
if there is policy for none 0 digit like %9pK?

Jaewon Kim

RE: [RFC] vsprintf: compile error on %09pK

[David Laight]

From: Jaewon Kim
> Sent: 03 April 2023 15:40
...
> I wanted to print phys_addr_t  type value only when kptr_restrict sysctl is
> allowed. So I thought I could use %pK for that purpose. And the physical
> address is not that long. I wanted to make that length short like 9 hex.

Isn't that is the wrong format for physical addresses anyway?
They can be larger than virtual ones (eg x86 with PAE).

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

RE: [RFC] vsprintf: compile error on %09pK

[Jaewon Kim]

>From: Jaewon Kim
>> Sent: 03 April 2023 15:40
>...
>> I wanted to print phys_addr_t  type value only when kptr_restrict sysctl is
>> allowed. So I thought I could use %pK for that purpose. And the physical
>> address is not that long. I wanted to make that length short like 9 hex.
>
>Isn't that is the wrong format for physical addresses anyway?
>They can be larger than virtual ones (eg x86 with PAE).

Yeah, correct. I just used %pK to hide physical address, I thought it could be
leak in security perspective. Could you give me advice how I can hide the
address by default and look the address if kptr_restrict allow it?

Jaewon Kim

>
>	David

Re: [RFC] vsprintf: compile error on %09pK

[Petr Mladek]

On Tue 2023-04-04 19:12:01, Jaewon Kim wrote:
> >From: Jaewon Kim
> >> Sent: 03 April 2023 15:40
> >...
> >> I wanted to print phys_addr_t  type value only when kptr_restrict sysctl is
> >> allowed. So I thought I could use %pK for that purpose. And the physical
> >> address is not that long. I wanted to make that length short like 9 hex.
> >
> >Isn't that is the wrong format for physical addresses anyway?
> >They can be larger than virtual ones (eg x86 with PAE).
> 
> Yeah, correct. I just used %pK to hide physical address, I thought it could be
> leak in security perspective. Could you give me advice how I can hide the
> address by default and look the address if kptr_restrict allow it?

Could you please send a patch that would show what value you want to print?

The initial mail mentioned converting %09lx to %09pK in some driver.
Then it included a warning when compiling from mm/page_alloc.o

Honestly, I think that you could just use %pK or %p. IMHO, it does not
make sense to optimize it for the length.

Anyway, there is still the question if the address is really worth
printing. Will it really help to locate a potential problem?

Best Regards,
Petr

RE: [RFC] vsprintf: compile error on %09pK

[Jaewon Kim]

>On Tue 2023-04-04 19:12:01, Jaewon Kim wrote:
>> >From: Jaewon Kim
>> >> Sent: 03 April 2023 15:40
>> >...
>> >> I wanted to print phys_addr_t  type value only when kptr_restrict sysctl is
>> >> allowed. So I thought I could use %pK for that purpose. And the physical
>> >> address is not that long. I wanted to make that length short like 9 hex.
>> >
>> >Isn't that is the wrong format for physical addresses anyway?
>> >They can be larger than virtual ones (eg x86 with PAE).
>> 
>> Yeah, correct. I just used %pK to hide physical address, I thought it could be
>> leak in security perspective. Could you give me advice how I can hide the
>> address by default and look the address if kptr_restrict allow it?
>
>Could you please send a patch that would show what value you want to print?
>
>The initial mail mentioned converting %09lx to %09pK in some driver.
>Then it included a warning when compiling from mm/page_alloc.o
>
                                                                                                                                                       O
Oh I just found something I'm interested. I was printing rmem->base and
rmem->name and wanted to hide the rmem->base. The commit aeb9267eb6b1 
("of: reserved-mem: print out reserved-mem details during boot - v6.3-rc1")
just used %pa. If %pa is not good enough in security perspecitve, I think
I can use %pa.

Yes it was just my curiosity. I don't actually need the length.

I'm OK now.
Thank you 

>Honestly, I think that you could just use %pK or %p. IMHO, it does not
>make sense to optimize it for the length.
>
>Anyway, there is still the question if the address is really worth
>printing. Will it really help to locate a potential problem?
>
>Best Regards,
>Petr