* [PATCH net 2/2] tcp: mark skbs with SCM_TIMESTAMPING_OPT_STATS
2017-03-18 21:02 [PATCH net 1/2] tcp: fix SCM_TIMESTAMPING_OPT_STATS for normal skbs Soheil Hassas Yeganeh
@ 2017-03-18 21:03 ` Soheil Hassas Yeganeh
2017-03-22 1:45 ` David Miller
2017-03-22 1:44 ` [PATCH net 1/2] tcp: fix SCM_TIMESTAMPING_OPT_STATS for normal skbs David Miller
1 sibling, 1 reply; 4+ messages in thread
From: Soheil Hassas Yeganeh @ 2017-03-18 21:03 UTC (permalink / raw)
To: davem, netdev
Cc: zzoru007, Soheil Hassas Yeganeh, Eric Dumazet, Willem de Bruijn
From: Soheil Hassas Yeganeh <soheil@google.com>
SOF_TIMESTAMPING_OPT_STATS can be enabled and disabled
while packets are collected on the error queue.
So, checking SOF_TIMESTAMPING_OPT_STATS in sk->sk_tsflags
is not enough to safely assume that the skb contains
OPT_STATS data.
Add a bit in sock_exterr_skb to indicate whether the
skb contains opt_stats data.
Fixes: 1c885808e456 ("tcp: SOF_TIMESTAMPING_OPT_STATS option for SO_TIMESTAMPING")
Reported-by: JongHwan Kim <zzoru007@gmail.com>
Signed-off-by: Soheil Hassas Yeganeh <soheil@google.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: Willem de Bruijn <willemb@google.com>
---
include/linux/errqueue.h | 2 ++
net/core/skbuff.c | 17 +++++++++++------
net/socket.c | 2 +-
3 files changed, 14 insertions(+), 7 deletions(-)
diff --git a/include/linux/errqueue.h b/include/linux/errqueue.h
index 9ca23fcfb5d7..6fdfc884fdeb 100644
--- a/include/linux/errqueue.h
+++ b/include/linux/errqueue.h
@@ -20,6 +20,8 @@ struct sock_exterr_skb {
struct sock_extended_err ee;
u16 addr_offset;
__be16 port;
+ u8 opt_stats:1,
+ unused:7;
};
#endif
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index b1fbd1958eb6..9f781092fda9 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -3793,16 +3793,20 @@ EXPORT_SYMBOL(skb_clone_sk);
static void __skb_complete_tx_timestamp(struct sk_buff *skb,
struct sock *sk,
- int tstype)
+ int tstype,
+ bool opt_stats)
{
struct sock_exterr_skb *serr;
int err;
+ BUILD_BUG_ON(sizeof(struct sock_exterr_skb) > sizeof(skb->cb));
+
serr = SKB_EXT_ERR(skb);
memset(serr, 0, sizeof(*serr));
serr->ee.ee_errno = ENOMSG;
serr->ee.ee_origin = SO_EE_ORIGIN_TIMESTAMPING;
serr->ee.ee_info = tstype;
+ serr->opt_stats = opt_stats;
if (sk->sk_tsflags & SOF_TIMESTAMPING_OPT_ID) {
serr->ee.ee_data = skb_shinfo(skb)->tskey;
if (sk->sk_protocol == IPPROTO_TCP &&
@@ -3843,7 +3847,7 @@ void skb_complete_tx_timestamp(struct sk_buff *skb,
*/
if (likely(atomic_inc_not_zero(&sk->sk_refcnt))) {
*skb_hwtstamps(skb) = *hwtstamps;
- __skb_complete_tx_timestamp(skb, sk, SCM_TSTAMP_SND);
+ __skb_complete_tx_timestamp(skb, sk, SCM_TSTAMP_SND, false);
sock_put(sk);
}
}
@@ -3854,7 +3858,7 @@ void __skb_tstamp_tx(struct sk_buff *orig_skb,
struct sock *sk, int tstype)
{
struct sk_buff *skb;
- bool tsonly;
+ bool tsonly, opt_stats = false;
if (!sk)
return;
@@ -3867,9 +3871,10 @@ void __skb_tstamp_tx(struct sk_buff *orig_skb,
#ifdef CONFIG_INET
if ((sk->sk_tsflags & SOF_TIMESTAMPING_OPT_STATS) &&
sk->sk_protocol == IPPROTO_TCP &&
- sk->sk_type == SOCK_STREAM)
+ sk->sk_type == SOCK_STREAM) {
skb = tcp_get_timestamping_opt_stats(sk);
- else
+ opt_stats = true;
+ } else
#endif
skb = alloc_skb(0, GFP_ATOMIC);
} else {
@@ -3888,7 +3893,7 @@ void __skb_tstamp_tx(struct sk_buff *orig_skb,
else
skb->tstamp = ktime_get_real();
- __skb_complete_tx_timestamp(skb, sk, tstype);
+ __skb_complete_tx_timestamp(skb, sk, tstype, opt_stats);
}
EXPORT_SYMBOL_GPL(__skb_tstamp_tx);
diff --git a/net/socket.c b/net/socket.c
index 692d6989d2c2..985ef06792d6 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -706,7 +706,7 @@ void __sock_recv_timestamp(struct msghdr *msg, struct sock *sk,
SCM_TIMESTAMPING, sizeof(tss), &tss);
if (skb_is_err_queue(skb) && skb->len &&
- (sk->sk_tsflags & SOF_TIMESTAMPING_OPT_STATS))
+ SKB_EXT_ERR(skb)->opt_stats)
put_cmsg(msg, SOL_SOCKET, SCM_TIMESTAMPING_OPT_STATS,
skb->len, skb->data);
}
--
2.12.0.367.g23dc2f6d3c-goog
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH net 1/2] tcp: fix SCM_TIMESTAMPING_OPT_STATS for normal skbs
2017-03-18 21:02 [PATCH net 1/2] tcp: fix SCM_TIMESTAMPING_OPT_STATS for normal skbs Soheil Hassas Yeganeh
2017-03-18 21:03 ` [PATCH net 2/2] tcp: mark skbs with SCM_TIMESTAMPING_OPT_STATS Soheil Hassas Yeganeh
@ 2017-03-22 1:44 ` David Miller
1 sibling, 0 replies; 4+ messages in thread
From: David Miller @ 2017-03-22 1:44 UTC (permalink / raw)
To: soheil.kdev; +Cc: netdev, zzoru007, soheil, edumazet, willemb
From: Soheil Hassas Yeganeh <soheil.kdev@gmail.com>
Date: Sat, 18 Mar 2017 17:02:59 -0400
> From: Soheil Hassas Yeganeh <soheil@google.com>
>
> __sock_recv_timestamp can be called for both normal skbs (for
> receive timestamps) and for skbs on the error queue (for transmit
> timestamps).
>
> Commit 1c885808e456
> (tcp: SOF_TIMESTAMPING_OPT_STATS option for SO_TIMESTAMPING)
> assumes any skb passed to __sock_recv_timestamp are from
> the error queue, containing OPT_STATS in the content of the skb.
> This results in accessing invalid memory or generating junk
> data.
>
> To fix this, set skb->pkt_type to PACKET_OUTGOING for packets
> on the error queue. This is safe because on the receive path
> on local sockets skb->pkt_type is never set to PACKET_OUTGOING.
> With that, copy OPT_STATS from a packet, only if its pkt_type
> is PACKET_OUTGOING.
>
> Fixes: 1c885808e456 ("tcp: SOF_TIMESTAMPING_OPT_STATS option for SO_TIMESTAMPING")
> Reported-by: JongHwan Kim <zzoru007@gmail.com>
> Signed-off-by: Soheil Hassas Yeganeh <soheil@google.com>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Signed-off-by: Willem de Bruijn <willemb@google.com>
Applied and queued up for -stable.
^ permalink raw reply [flat|nested] 4+ messages in thread