netdev - Re: [PATCH net-next 2/2] trace: tcp: fully support trace_tcp_send

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date: Mon, 11 Mar 2024 05:09:01 +0000
From: Ratheesh Kannoth <rkannoth@...vell.com>
To: Jason Xing <kerneljasonxing@...il.com>
CC: "edumazet@...gle.com" <edumazet@...gle.com>,
        "mhiramat@...nel.org"
	<mhiramat@...nel.org>,
        "mathieu.desnoyers@...icios.com"
	<mathieu.desnoyers@...icios.com>,
        "rostedt@...dmis.org"
	<rostedt@...dmis.org>,
        "kuba@...nel.org" <kuba@...nel.org>,
        "pabeni@...hat.com" <pabeni@...hat.com>,
        "davem@...emloft.net"
	<davem@...emloft.net>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-trace-kernel@...r.kernel.org" <linux-trace-kernel@...r.kernel.org>,
        Jason Xing <kernelxing@...cent.com>
Subject: Re: [PATCH net-next 2/2] trace: tcp: fully support
 trace_tcp_send_reset

> From: Jason Xing <kerneljasonxing@...il.com>
> Sent: Monday, March 11, 2024 10:30 AM
> To: Ratheesh Kannoth <rkannoth@...vell.com>
> Cc: edumazet@...gle.com; mhiramat@...nel.org;
> mathieu.desnoyers@...icios.com; rostedt@...dmis.org; kuba@...nel.org;
> pabeni@...hat.com; davem@...emloft.net; netdev@...r.kernel.org; linux-
> trace-kernel@...r.kernel.org; Jason Xing <kernelxing@...cent.com>
> Subject: [EXTERNAL] Re: [PATCH net-next 2/2] trace: tcp: fully support
> trace_tcp_send_reset
> 
> > > +     ),
> > > +
> > > +     TP_printk("skbaddr=%p skaddr=%p src=%pISpc dest=%pISpc
> > > + state=%s",
> > Could you consider using %px ? is it permitted ? it will be easy to track skb.
> 
> I prefer not to use %px because we cannot make use of the real address of
> skb. Besides, using %px would leak kernel addresses.
> 
> Here is the Documentation (see Documentation/core-api/printk-formats.rst):
> "Pointers printed without a specifier extension (i.e unadorned %p) are
> hashed to give a unique identifier without leaking kernel addresses to user
> space."
> 
> Perhaps, that's the reason why all the tracepoints didn't print in %px format:)
> 
ACK, I understand that.  In offload cases, or in mostly in IOVA space, we often end up changing this locally to %px to debug and track skb. 
That is why I asked the question. 
 
> Thanks,
> Jason
> 
> >
> > > +               __entry->skbaddr, __entry->skaddr,
> > > +               __entry->saddr, __entry->daddr,
> > > +               __entry->state ? show_tcp_state_name(__entry->state)
> > > + : "UNKNOWN")
> > >  );
> > >
> > >  /*
> > > diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c index
> > > a22ee5838751..d5c4a969c066 100644
> > > --- a/net/ipv4/tcp_ipv4.c
> > > +++ b/net/ipv4/tcp_ipv4.c
> > > @@ -868,10 +868,10 @@ static void tcp_v4_send_reset(const struct sock
> *sk, struct sk_buff *skb)
> > >        */
> > >       if (sk) {
> > >               arg.bound_dev_if = sk->sk_bound_dev_if;
> > > -             if (sk_fullsock(sk))
> > > -                     trace_tcp_send_reset(sk, skb);
> > >       }
> > >
> > > +     trace_tcp_send_reset(sk, skb);
> > > +
> > >       BUILD_BUG_ON(offsetof(struct sock, sk_bound_dev_if) !=
> > >                    offsetof(struct inet_timewait_sock,
> > > tw_bound_dev_if));
> > >
> > > diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c index
> > > 3f4cba49e9ee..8e9c59b6c00c 100644
> > > --- a/net/ipv6/tcp_ipv6.c
> > > +++ b/net/ipv6/tcp_ipv6.c
> > > @@ -1113,7 +1113,6 @@ static void tcp_v6_send_reset(const struct sock
> *sk, struct sk_buff *skb)
> > >       if (sk) {
> > >               oif = sk->sk_bound_dev_if;
> > >               if (sk_fullsock(sk)) {
> > > -                     trace_tcp_send_reset(sk, skb);
> > >                       if (inet6_test_bit(REPFLOW, sk))
> > >                               label = ip6_flowlabel(ipv6h);
> > >                       priority = READ_ONCE(sk->sk_priority); @@
> > > -1129,6 +1128,8 @@ static void tcp_v6_send_reset(const struct sock *sk,
> struct sk_buff *skb)
> > >                       label = ip6_flowlabel(ipv6h);
> > >       }
> > >
> > > +     trace_tcp_send_reset(sk, skb);
> > > +
> > >       tcp_v6_send_response(sk, skb, seq, ack_seq, 0, 0, 0, oif, 1,
> > >                            ipv6_get_dsfield(ipv6h), label, priority, txhash,
> > >                            &key);
> > > --
> > > 2.37.3
> > >