netdev - Re: [PATCH v3 bpf-next 0/8] bpf, printk: add BTF-based type printing

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] [day] [month] [year] [list]

Date:   Tue, 30 Jun 2020 20:31:37 +0900
From:   Sergey Senozhatsky <sergey.senozhatsky@...il.com>
To:     Alan Maguire <alan.maguire@...cle.com>
Cc:     ast@...nel.org, daniel@...earbox.net, yhs@...com, andriin@...com,
        arnaldo.melo@...il.com, kafai@...com, songliubraving@...com,
        john.fastabend@...il.com, kpsingh@...omium.org,
        linux@...musvillemoes.dk, joe@...ches.com, pmladek@...e.com,
        rostedt@...dmis.org, sergey.senozhatsky@...il.com,
        andriy.shevchenko@...ux.intel.com, corbet@....net,
        bpf@...r.kernel.org, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-doc@...r.kernel.org
Subject: Re: [PATCH v3 bpf-next 0/8] bpf, printk: add BTF-based type printing

On (20/06/23 13:07), Alan Maguire wrote:
>   struct sk_buff *skb = alloc_skb(64, GFP_KERNEL);
> 
>   pr_info("%pT", BTF_PTR_TYPE(skb, "struct sk_buff"));
> 
> ...gives us:
> 
> (struct sk_buff){
>  .transport_header = (__u16)65535,
>  .mac_header = (__u16)65535,
>  .end = (sk_buff_data_t)192,
>  .head = (unsigned char *)000000007524fd8b,
>  .data = (unsigned char *)000000007524fd8b,
>  .truesize = (unsigned int)768,
>  .users = (refcount_t){
>   .refs = (atomic_t){
>    .counter = (int)1,
>   },
>  },
> }

Hmm. So this can expose the kernel memory layout (IOW do you print out real
%px pointers and so on)? If so, then I'd suggest not to use printk.
Unprivileged /dev/kmsg or /proc/kmsg reads are really OK thing for printk()
log buffer. And if you are going to print pointer hashes instead,

  .transport_header = (__u16)65535,
  .mac_header = (__u16)65535,
  .end = (sk_buff_data_t)192,
  .head = (unsigned char *)34897918740,   // pointer_hash
  .data = (unsigned char *)23942384983,   // pointer hash
  .truesize = (unsigned int)768,
  .users = (refcount_t){
   .refs = (atomic_t){
    .counter = (int)1,
   },
  },

then the value of such printouts becomes a bit unclear to me, sorry.

Probably, something like a seq print into a file somewhere in
/sys/kernel/debug/foo, from which only privileged processes can
read, would be a better approach? My 5 cents.

	-ss