net: use the %px format to display sock

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

Posted by kerneljasonxing@gmail.com 4 years ago

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(-)

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;
 	}
 
-- 
1.8.3.1

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

Posted by Peilin Ye 4 years ago

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

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

Posted by Jason Xing 4 years ago

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
>

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

Posted by Andrew Lunn 4 years ago

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