Re: [PATCH net-next] net: Convert %pK back to %p

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] Replacing `%pK` with `%p` breaks userspace tracing ABI and `kptr_restrict` runtime configurability.
- [Medium] Incomplete refactoring misses `net/bluetooth/af_bluetooth.c`, leaving inconsistent pointer formatting behavior.
--

commit b2c57f416d95fe882e14b5fd13915b835f475930
Author: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

net: Convert %pK back to %p

This commit reverts a 2011 change that converted %p usage to %pK in the
networking subsystem. It replaces %pK with %p across various procfs output
files, stating that %p now defaults to a hashed pointer anyway.

> diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
> index 209ef7522508f..aa31af06e5e3b 100644
> --- a/net/ipv4/tcp_ipv4.c
> +++ b/net/ipv4/tcp_ipv4.c

[ ... ]

> @@ -2806,7 +2806,7 @@ static void get_tcp4_sock(struct sock *sk, struct seq_file *f, int i)
>  				      READ_ONCE(tp->copied_seq), 0);
>  
>  	seq_printf(f, "%4d: %08X:%04X %08X:%04X %02X %08X:%08X %02X:%08lX "
> -			"%08X %5u %8d %llu %d %pK %lu %lu %u %u %d",
> +			"%08X %5u %8d %llu %d %p %lu %lu %u %u %d",

[Severity: Critical]
Does replacing %pK with %p in get_tcp4_sock() break the userspace tracing
ABI and bypass the kernel.kptr_restrict sysctl?

Under kptr_restrict=1 (the default on most distributions), %pK returns the
raw kernel pointer to users with the syslog capability. By reverting to %p,
a hashed pointer is unconditionally returned instead. 

While the commit message notes that hash_pointers can be disabled at boot,
does this break the runtime configurability provided by the
kernel.kptr_restrict sysctl? 

Will existing tracing tools that correlate /proc sockets with kernel memory
receive hashes and fail instead of raw pointers?

[ ... ]

> diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
> index f7a9d55eee8a1..6a8174977c87a 100644
> --- a/net/unix/af_unix.c
> +++ b/net/unix/af_unix.c

[ ... ]

> @@ -3554,7 +3554,7 @@ static int unix_seq_show(struct seq_file *seq, void *v)
>  		struct unix_sock *u = unix_sk(s);
>  		unix_state_lock(s);
>  
> -		seq_printf(seq, "%pK: %08X %08X %08X %04X %02X %5llu",
> +		seq_printf(seq, "%p: %08X %08X %08X %04X %02X %5llu",

[Severity: Critical]
Does this also expose unix socket structures directly as hashed pointers in
unix_seq_show(), bypassing sysctl restrictions?

Similar to the tcp change above, will tracing tools no longer be able to
resolve these pointers when kptr_restrict=1 is set?

[ ... ]

[Severity: Medium]
Does this commit leave the networking subsystem in an inconsistent state by
missing net/bluetooth/af_bluetooth.c?

Looking at bt_seq_show() in net/bluetooth/af_bluetooth.c, it still uses %pK:

    seq_printf(f, "%pK %-6d %-6u %-6u %-6u %-6llu %-6llu",

Is this the only remaining use of %pK in the net/ subsystem that was missed
by the refactoring? This appears to leave bluetooth procfs files respecting
kptr_restrict while the other protocols do not.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260706073824.xixrLxoD@linutronix.de?part=1