* [PATCH] bpf: Don't use %pK through printk @ 2025-08-11 12:08 Thomas Weißschuh 2025-08-12 22:19 ` Andrii Nakryiko 2025-08-13 22:40 ` patchwork-bot+netdevbpf 0 siblings, 2 replies; 5+ messages in thread From: Thomas Weißschuh @ 2025-08-11 12:08 UTC (permalink / raw) To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman, Song Liu, Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa Cc: bpf, linux-kernel, Thomas Weißschuh In the past %pK was preferable to %p as it would not leak raw pointer values into the kernel log. Since commit ad67b74d2469 ("printk: hash addresses printed with %p") the regular %p has been improved to avoid this issue. Furthermore, restricted pointers ("%pK") were never meant to be used through printk(). They can still unintentionally leak raw pointers or acquire sleeping locks in atomic contexts. Switch to the regular pointer formatting which is safer and easier to reason about. Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de> --- include/linux/filter.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/linux/filter.h b/include/linux/filter.h index 1e7fd3ee759e07534eee7d8b48cffa1dfea056fb..52fecb7a1fe36d233328aabbe5eadcbd7e07cc5a 100644 --- a/include/linux/filter.h +++ b/include/linux/filter.h @@ -1296,7 +1296,7 @@ void bpf_jit_prog_release_other(struct bpf_prog *fp, struct bpf_prog *fp_other); static inline void bpf_jit_dump(unsigned int flen, unsigned int proglen, u32 pass, void *image) { - pr_err("flen=%u proglen=%u pass=%u image=%pK from=%s pid=%d\n", flen, + pr_err("flen=%u proglen=%u pass=%u image=%p from=%s pid=%d\n", flen, proglen, pass, image, current->comm, task_pid_nr(current)); if (image) --- base-commit: 8f5ae30d69d7543eee0d70083daf4de8fe15d585 change-id: 20250811-restricted-pointers-bpf-04da04ea1b8a Best regards, -- Thomas Weißschuh <thomas.weissschuh@linutronix.de> ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] bpf: Don't use %pK through printk 2025-08-11 12:08 [PATCH] bpf: Don't use %pK through printk Thomas Weißschuh @ 2025-08-12 22:19 ` Andrii Nakryiko 2025-08-13 5:34 ` Thomas Weißschuh 2025-08-13 22:40 ` patchwork-bot+netdevbpf 1 sibling, 1 reply; 5+ messages in thread From: Andrii Nakryiko @ 2025-08-12 22:19 UTC (permalink / raw) To: Thomas Weißschuh Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman, Song Liu, Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, bpf, linux-kernel On Mon, Aug 11, 2025 at 5:08 AM Thomas Weißschuh <thomas.weissschuh@linutronix.de> wrote: > > In the past %pK was preferable to %p as it would not leak raw pointer > values into the kernel log. > Since commit ad67b74d2469 ("printk: hash addresses printed with %p") > the regular %p has been improved to avoid this issue. > Furthermore, restricted pointers ("%pK") were never meant to be used > through printk(). They can still unintentionally leak raw pointers or > acquire sleeping locks in atomic contexts. > > Switch to the regular pointer formatting which is safer and > easier to reason about. > > Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de> > --- > include/linux/filter.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/include/linux/filter.h b/include/linux/filter.h > index 1e7fd3ee759e07534eee7d8b48cffa1dfea056fb..52fecb7a1fe36d233328aabbe5eadcbd7e07cc5a 100644 > --- a/include/linux/filter.h > +++ b/include/linux/filter.h > @@ -1296,7 +1296,7 @@ void bpf_jit_prog_release_other(struct bpf_prog *fp, struct bpf_prog *fp_other); > static inline void bpf_jit_dump(unsigned int flen, unsigned int proglen, > u32 pass, void *image) > { > - pr_err("flen=%u proglen=%u pass=%u image=%pK from=%s pid=%d\n", flen, > + pr_err("flen=%u proglen=%u pass=%u image=%p from=%s pid=%d\n", flen, > proglen, pass, image, current->comm, task_pid_nr(current)); this particular printk won't ever be called from atomic context, so I don't think the leak from atomic context matters much here. On the other hand, %pK behavior is controlled by kptr_restrict and that might be useful for debugging, so not sure there is much of a benefit to switching to always obfuscated %p? Or am I missing something else here? > > if (image) > > --- > base-commit: 8f5ae30d69d7543eee0d70083daf4de8fe15d585 > change-id: 20250811-restricted-pointers-bpf-04da04ea1b8a > > Best regards, > -- > Thomas Weißschuh <thomas.weissschuh@linutronix.de> > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] bpf: Don't use %pK through printk 2025-08-12 22:19 ` Andrii Nakryiko @ 2025-08-13 5:34 ` Thomas Weißschuh 2025-08-13 22:36 ` Andrii Nakryiko 0 siblings, 1 reply; 5+ messages in thread From: Thomas Weißschuh @ 2025-08-13 5:34 UTC (permalink / raw) To: Andrii Nakryiko Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman, Song Liu, Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, bpf, linux-kernel On Tue, Aug 12, 2025 at 03:19:45PM -0700, Andrii Nakryiko wrote: > On Mon, Aug 11, 2025 at 5:08 AM Thomas Weißschuh > <thomas.weissschuh@linutronix.de> wrote: > > > > In the past %pK was preferable to %p as it would not leak raw pointer > > values into the kernel log. > > Since commit ad67b74d2469 ("printk: hash addresses printed with %p") > > the regular %p has been improved to avoid this issue. > > Furthermore, restricted pointers ("%pK") were never meant to be used > > through printk(). They can still unintentionally leak raw pointers or > > acquire sleeping locks in atomic contexts. > > > > Switch to the regular pointer formatting which is safer and > > easier to reason about. > > > > Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de> > > --- > > include/linux/filter.h | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/include/linux/filter.h b/include/linux/filter.h > > index 1e7fd3ee759e07534eee7d8b48cffa1dfea056fb..52fecb7a1fe36d233328aabbe5eadcbd7e07cc5a 100644 > > --- a/include/linux/filter.h > > +++ b/include/linux/filter.h > > @@ -1296,7 +1296,7 @@ void bpf_jit_prog_release_other(struct bpf_prog *fp, struct bpf_prog *fp_other); > > static inline void bpf_jit_dump(unsigned int flen, unsigned int proglen, > > u32 pass, void *image) > > { > > - pr_err("flen=%u proglen=%u pass=%u image=%pK from=%s pid=%d\n", flen, > > + pr_err("flen=%u proglen=%u pass=%u image=%p from=%s pid=%d\n", flen, > > proglen, pass, image, current->comm, task_pid_nr(current)); > > this particular printk won't ever be called from atomic context, so I > don't think the leak from atomic context matters much here. On the > other hand, %pK behavior is controlled by kptr_restrict and that might > be useful for debugging, so not sure there is much of a benefit to > switching to always obfuscated %p? Or am I missing something else > here? As %pK is so easy to abuse and the breakage is very non-obvious, I want to rework it to enforce its usage from "file context". For that, the printk users need to be gone first. For debugging, there is still "no_hash_pointers". How would the image pointer be used for debugging? It is logged from nowhere else. And the raw image is dumped right after anyways. > > > > > if (image) > > > > --- > > base-commit: 8f5ae30d69d7543eee0d70083daf4de8fe15d585 > > change-id: 20250811-restricted-pointers-bpf-04da04ea1b8a > > > > Best regards, > > -- > > Thomas Weißschuh <thomas.weissschuh@linutronix.de> > > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] bpf: Don't use %pK through printk 2025-08-13 5:34 ` Thomas Weißschuh @ 2025-08-13 22:36 ` Andrii Nakryiko 0 siblings, 0 replies; 5+ messages in thread From: Andrii Nakryiko @ 2025-08-13 22:36 UTC (permalink / raw) To: Thomas Weißschuh Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman, Song Liu, Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, bpf, linux-kernel On Tue, Aug 12, 2025 at 10:34 PM Thomas Weißschuh <thomas.weissschuh@linutronix.de> wrote: > > On Tue, Aug 12, 2025 at 03:19:45PM -0700, Andrii Nakryiko wrote: > > On Mon, Aug 11, 2025 at 5:08 AM Thomas Weißschuh > > <thomas.weissschuh@linutronix.de> wrote: > > > > > > In the past %pK was preferable to %p as it would not leak raw pointer > > > values into the kernel log. > > > Since commit ad67b74d2469 ("printk: hash addresses printed with %p") > > > the regular %p has been improved to avoid this issue. > > > Furthermore, restricted pointers ("%pK") were never meant to be used > > > through printk(). They can still unintentionally leak raw pointers or > > > acquire sleeping locks in atomic contexts. > > > > > > Switch to the regular pointer formatting which is safer and > > > easier to reason about. > > > > > > Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de> > > > --- > > > include/linux/filter.h | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/include/linux/filter.h b/include/linux/filter.h > > > index 1e7fd3ee759e07534eee7d8b48cffa1dfea056fb..52fecb7a1fe36d233328aabbe5eadcbd7e07cc5a 100644 > > > --- a/include/linux/filter.h > > > +++ b/include/linux/filter.h > > > @@ -1296,7 +1296,7 @@ void bpf_jit_prog_release_other(struct bpf_prog *fp, struct bpf_prog *fp_other); > > > static inline void bpf_jit_dump(unsigned int flen, unsigned int proglen, > > > u32 pass, void *image) > > > { > > > - pr_err("flen=%u proglen=%u pass=%u image=%pK from=%s pid=%d\n", flen, > > > + pr_err("flen=%u proglen=%u pass=%u image=%p from=%s pid=%d\n", flen, > > > proglen, pass, image, current->comm, task_pid_nr(current)); > > > > this particular printk won't ever be called from atomic context, so I > > don't think the leak from atomic context matters much here. On the > > other hand, %pK behavior is controlled by kptr_restrict and that might > > be useful for debugging, so not sure there is much of a benefit to > > switching to always obfuscated %p? Or am I missing something else > > here? > > As %pK is so easy to abuse and the breakage is very non-obvious, I want to > rework it to enforce its usage from "file context". > For that, the printk users need to be gone first. > For debugging, there is still "no_hash_pointers". > > How would the image pointer be used for debugging? I chatted with Daniel about this offline, and it seems like this is not that critical nowadays. So I went ahead and applied the patch to bpf-next. > It is logged from nowhere else. > And the raw image is dumped right after anyways. > > > > > > > > > if (image) > > > > > > --- > > > base-commit: 8f5ae30d69d7543eee0d70083daf4de8fe15d585 > > > change-id: 20250811-restricted-pointers-bpf-04da04ea1b8a > > > > > > Best regards, > > > -- > > > Thomas Weißschuh <thomas.weissschuh@linutronix.de> > > > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] bpf: Don't use %pK through printk 2025-08-11 12:08 [PATCH] bpf: Don't use %pK through printk Thomas Weißschuh 2025-08-12 22:19 ` Andrii Nakryiko @ 2025-08-13 22:40 ` patchwork-bot+netdevbpf 1 sibling, 0 replies; 5+ messages in thread From: patchwork-bot+netdevbpf @ 2025-08-13 22:40 UTC (permalink / raw) To: =?utf-8?q?Thomas_Wei=C3=9Fschuh_=3Cthomas=2Eweissschuh=40linutronix=2Ede=3E?= Cc: ast, daniel, andrii, martin.lau, eddyz87, song, yonghong.song, john.fastabend, kpsingh, sdf, haoluo, jolsa, bpf, linux-kernel Hello: This patch was applied to bpf/bpf-next.git (master) by Andrii Nakryiko <andrii@kernel.org>: On Mon, 11 Aug 2025 14:08:04 +0200 you wrote: > In the past %pK was preferable to %p as it would not leak raw pointer > values into the kernel log. > Since commit ad67b74d2469 ("printk: hash addresses printed with %p") > the regular %p has been improved to avoid this issue. > Furthermore, restricted pointers ("%pK") were never meant to be used > through printk(). They can still unintentionally leak raw pointers or > acquire sleeping locks in atomic contexts. > > [...] Here is the summary with links: - bpf: Don't use %pK through printk https://git.kernel.org/bpf/bpf-next/c/2caa6b88e0ba You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-08-13 22:39 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-08-11 12:08 [PATCH] bpf: Don't use %pK through printk Thomas Weißschuh 2025-08-12 22:19 ` Andrii Nakryiko 2025-08-13 5:34 ` Thomas Weißschuh 2025-08-13 22:36 ` Andrii Nakryiko 2025-08-13 22:40 ` patchwork-bot+netdevbpf
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).