All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kees Cook <keescook@chromium.org>
To: Cong Wang <xiyou.wangcong@gmail.com>,
	Qitao Xu <qitao.xu@bytedance.com>,
	"David S. Miller" <davem@davemloft.net>
Cc: netdev@vger.kernel.org, Cong Wang <cong.wang@bytedance.com>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	linux-hardening@vger.kernel.org
Subject: Re: [Patch net-next resend v2] net: use %px to print skb address in trace_netif_receive_skb
Date: Fri, 23 Jul 2021 00:09:15 -0700	[thread overview]
Message-ID: <202107230000.B52B102@keescook> (raw)
In-Reply-To: <20210715055923.43126-1-xiyou.wangcong@gmail.com>

On Wed, Jul 14, 2021 at 10:59:23PM -0700, Cong Wang wrote:
> From: Qitao Xu <qitao.xu@bytedance.com>
> 
> The print format of skb adress in tracepoint class net_dev_template
> is changed to %px from %p, because we want to use skb address
> as a quick way to identify a packet.

No; %p was already hashed to uniquely identify unique addresses. This
is needlessly exposing kernel addresses with no change in utility. See
[1] for full details on when %px is justified (almost never).

> Note, trace ring buffer is only accessible to privileged users,
> it is safe to use a real kernel address here.

That's not accurate either; there is a difference between uid 0 and
kernel mode privilege levels.

Please revert these:

	851f36e40962408309ad2665bf0056c19a97881c
	65875073eddd24d7b3968c1501ef29277398dc7b

And adjust this to replace %px with %p:

	70713dddf3d25a02d1952f8c5d2688c986d2f2fb

Thanks!

-Kees

[1] https://www.kernel.org/doc/html/latest/process/deprecated.html#p-format-specifier

> 
> Reviewed-by: Cong Wang <cong.wang@bytedance.com>
> Signed-off-by: Qitao Xu <qitao.xu@bytedance.com>
> ---
>  include/trace/events/net.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/trace/events/net.h b/include/trace/events/net.h
> index 2399073c3afc..78c448c6ab4c 100644
> --- a/include/trace/events/net.h
> +++ b/include/trace/events/net.h
> @@ -136,7 +136,7 @@ DECLARE_EVENT_CLASS(net_dev_template,
>  		__assign_str(name, skb->dev->name);
>  	),
>  
> -	TP_printk("dev=%s skbaddr=%p len=%u",
> +	TP_printk("dev=%s skbaddr=%px len=%u",
>  		__get_str(name), __entry->skbaddr, __entry->len)
>  )
>  
> -- 
> 2.27.0
> 

-- 
Kees Cook

  parent reply	other threads:[~2021-07-23  7:09 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-15  5:59 [Patch net-next resend v2] net: use %px to print skb address in trace_netif_receive_skb Cong Wang
2021-07-15 17:40 ` patchwork-bot+netdevbpf
2021-07-23  7:09 ` Kees Cook [this message]
2021-07-28  3:17   ` Cong Wang
2021-07-28 15:13   ` tracepoints and %p [was: Re: [Patch net-next resend v2] net: use %px to print skb address in trace_netif_receive_skb] Jann Horn
2021-07-28 15:56     ` Steven Rostedt
2021-07-28 18:48       ` Kees Cook
2021-07-28 18:56         ` Jann Horn
2021-07-28 20:41         ` Steven Rostedt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=202107230000.B52B102@keescook \
    --to=keescook@chromium.org \
    --cc=cong.wang@bytedance.com \
    --cc=davem@davemloft.net \
    --cc=linux-hardening@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=qitao.xu@bytedance.com \
    --cc=torvalds@linux-foundation.org \
    --cc=xiyou.wangcong@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.