diff mbox series

[net-next] net: use the %px format to display sock

Message ID 20220505130826.40914-1-kerneljasonxing@gmail.com (mailing list archive)
State Rejected
Delegated to: Netdev Maintainers
Headers show
Series [net-next] net: use the %px format to display sock | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix success Link
netdev/cover_letter success Single patches do not need cover letters
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 3 this patch: 3
netdev/cc_maintainers warning 1 maintainers not CCed: edumazet@google.com
netdev/build_clang success Errors and warnings before: 9 this patch: 9
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 3 this patch: 3
netdev/checkpatch warning WARNING: Using vsprintf specifier '%px' potentially exposes the kernel memory layout, if you don't really need the address please consider using '%p'.
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Jason Xing May 5, 2022, 1:08 p.m. UTC 
From: Jason Xing <xingwanli@kuaishou.com>

I found that the current socket address, say 000000009842d952, cannot be
searched in messages because %p format function hashes and converts it
into an unique identifier which is currently useless for debugging.

Signed-off-by: Jason Xing <xingwanli@kuaishou.com>
---
 net/ipv4/af_inet.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Peilin Ye May 6, 2022, 6:56 p.m. UTC | #1 
Hi Jason,

On Thu, May 05, 2022 at 09:08:26PM +0800, kerneljasonxing@gmail.com wrote:
> -		pr_err("Attempt to release TCP socket in state %d %p\n",
> +		pr_err("Attempt to release TCP socket in state %d %px\n",

I think we cannot use %px here for security reasons?  checkpatch is also
warning about it:

WARNING: Using vsprintf specifier '%px' potentially exposes the kernel memory layout, if you don't really need the address please consider using '%p'.
#21: FILE: net/ipv4/af_inet.c:142:
+		pr_err("Attempt to release TCP socket in state %d %px\n",
 		       sk->sk_state, sk);

Thanks,
Peilin Ye
Jason Xing May 7, 2022, 1:26 a.m. UTC | #2 
On Sat, May 7, 2022 at 2:56 AM Peilin Ye <yepeilin.cs@gmail.com> wrote:
>
> Hi Jason,
>
> On Thu, May 05, 2022 at 09:08:26PM +0800, kerneljasonxing@gmail.com wrote:
> > -             pr_err("Attempt to release TCP socket in state %d %p\n",
> > +             pr_err("Attempt to release TCP socket in state %d %px\n",
>
> I think we cannot use %px here for security reasons?  checkpatch is also
> warning about it:
>

I noticed this warning before submitting. Since the %p format doesn't
print the real address, printing the address here will be helpless and
we cannot trace what exactly the bad socket is.

What do you suggest?

Thanks,
Jason

> WARNING: Using vsprintf specifier '%px' potentially exposes the kernel memory layout, if you don't really need the address please consider using '%p'.
> #21: FILE: net/ipv4/af_inet.c:142:
> +               pr_err("Attempt to release TCP socket in state %d %px\n",
>                        sk->sk_state, sk);
>
> Thanks,
> Peilin Ye
>
Andrew Lunn May 7, 2022, 2:22 p.m. UTC | #3 
On Sat, May 07, 2022 at 09:26:07AM +0800, Jason Xing wrote:
> On Sat, May 7, 2022 at 2:56 AM Peilin Ye <yepeilin.cs@gmail.com> wrote:
> >
> > Hi Jason,
> >
> > On Thu, May 05, 2022 at 09:08:26PM +0800, kerneljasonxing@gmail.com wrote:
> > > -             pr_err("Attempt to release TCP socket in state %d %p\n",
> > > +             pr_err("Attempt to release TCP socket in state %d %px\n",
> >
> > I think we cannot use %px here for security reasons?  checkpatch is also
> > warning about it:
> >
> 
> I noticed this warning before submitting. Since the %p format doesn't
> print the real address, printing the address here will be helpless and
> we cannot trace what exactly the bad socket is.
> 
> What do you suggest?

How is a socket identified in places like /proc/<PID>/net/tcp ?
Could you print the local and remote port to identify the socket?

How does the address of the structure actually help you? Do you see
this address somewhere else?

     Andrew
diff mbox series

Patch

diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index 72fde28..b17a4d4 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -139,12 +139,12 @@  void inet_sock_destruct(struct sock *sk)
 	sk_mem_reclaim_final(sk);
 
 	if (sk->sk_type == SOCK_STREAM && sk->sk_state != TCP_CLOSE) {
-		pr_err("Attempt to release TCP socket in state %d %p\n",
+		pr_err("Attempt to release TCP socket in state %d %px\n",
 		       sk->sk_state, sk);
 		return;
 	}
 	if (!sock_flag(sk, SOCK_DEAD)) {
-		pr_err("Attempt to release alive inet socket %p\n", sk);
+		pr_err("Attempt to release alive inet socket %px\n", sk);
 		return;
 	}