Skip to content

Commit

Permalink
ip: fix error queue empty skb handling
Browse files Browse the repository at this point in the history 
When reading from the error queue, msg_name and msg_control are only
populated for some errors. A new exception for empty timestamp skbs
added a false positive on icmp errors without payload.

`traceroute -M udpconn` only displayed gateways that return payload
with the icmp error: the embedded network headers are pulled before
sock_queue_err_skb, leaving an skb with skb->len == 0 otherwise.

Fix this regression by refining when msg_name and msg_control
branches are taken. The solutions for the two fields are independent.

msg_name only makes sense for errors that configure serr->port and
serr->addr_offset. Test the first instead of skb->len. This also fixes
another issue. saddr could hold the wrong data, as serr->addr_offset
is not initialized  in some code paths, pointing to the start of the
network header. It is only valid when serr->port is set (non-zero).

msg_control support differs between IPv4 and IPv6. IPv4 only honors
requests for ICMP and timestamps with SOF_TIMESTAMPING_OPT_CMSG. The
skb->len test can simply be removed, because skb->dev is also tested
and never true for empty skbs. IPv6 honors requests for all errors
aside from local errors and timestamps on empty skbs.

In both cases, make the policy more explicit by moving this logic to
a new function that decides whether to process msg_control and that
optionally prepares the necessary fields in skb->cb[]. After this
change, the IPv4 and IPv6 paths are more similar.

The last case is rxrpc. Here, simply refine to only match timestamps.

Fixes: 49ca0d8 ("net-timestamp: no-payload option")

Reported-by: Jan Niehusmann <[email protected]>
Signed-off-by: Willem de Bruijn <[email protected]>

----

Changes
  v1->v2
  - fix local origin test inversion in ip6_datagram_support_cmsg
  - make v4 and v6 code paths more similar by introducing analogous
    ipv4_datagram_support_cmsg
  - fix compile bug in rxrpc
Signed-off-by: David S. Miller <[email protected]>
  • Loading branch information
@wdebruij @davem330
wdebruij authored and davem330 committed Mar 9, 2015
1 parent 8edfe3b commit c247f05
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 23 deletions.
33 changes: 23 additions & 10 deletions net/ipv4/ip_sockglue.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -432,17 +432,32 @@ void ip_local_error(struct sock *sk, int err, __be32 daddr, __be16 port, u32 inf
kfree_skb(skb);
}

static bool ipv4_pktinfo_prepare_errqueue(const struct sock *sk,
const struct sk_buff *skb,
int ee_origin)
/* IPv4 supports cmsg on all imcp errors and some timestamps
*
* Timestamp code paths do not initialize the fields expected by cmsg:
* the PKTINFO fields in skb->cb[]. Fill those in here.
*/
static bool ipv4_datagram_support_cmsg(const struct sock *sk,
struct sk_buff *skb,
int ee_origin)
{
struct in_pktinfo *info = PKTINFO_SKB_CB(skb);
struct in_pktinfo *info;

if (ee_origin == SO_EE_ORIGIN_ICMP)
return true;

if ((ee_origin != SO_EE_ORIGIN_TIMESTAMPING) ||
(!(sk->sk_tsflags & SOF_TIMESTAMPING_OPT_CMSG)) ||
if (ee_origin == SO_EE_ORIGIN_LOCAL)
return false;

/* Support IP_PKTINFO on tstamp packets if requested, to correlate
* timestamp with egress dev. Not possible for packets without dev
* or without payload (SOF_TIMESTAMPING_OPT_TSONLY).
*/
if ((!(sk->sk_tsflags & SOF_TIMESTAMPING_OPT_CMSG)) ||
(!skb->dev))
return false;

info = PKTINFO_SKB_CB(skb);
info->ipi_spec_dst.s_addr = ip_hdr(skb)->saddr;
info->ipi_ifindex = skb->dev->ifindex;
return true;
Expand Down Expand Up @@ -483,7 +498,7 @@ int ip_recv_error(struct sock *sk, struct msghdr *msg, int len, int *addr_len)

serr = SKB_EXT_ERR(skb);

if (sin && skb->len) {
if (sin && serr->port) {
sin->sin_family = AF_INET;
sin->sin_addr.s_addr = *(__be32 *)(skb_network_header(skb) +
serr->addr_offset);
Expand All @@ -496,9 +511,7 @@ int ip_recv_error(struct sock *sk, struct msghdr *msg, int len, int *addr_len)
sin = &errhdr.offender;
memset(sin, 0, sizeof(*sin));

if (skb->len &&
(serr->ee.ee_origin == SO_EE_ORIGIN_ICMP ||
ipv4_pktinfo_prepare_errqueue(sk, skb, serr->ee.ee_origin))) {
if (ipv4_datagram_support_cmsg(sk, skb, serr->ee.ee_origin)) {
sin->sin_family = AF_INET;
sin->sin_addr.s_addr = ip_hdr(skb)->saddr;
if (inet_sk(sk)->cmsg_flags)
Expand Down
39 changes: 28 additions & 11 deletions net/ipv6/datagram.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -325,14 +325,34 @@ void ipv6_local_rxpmtu(struct sock *sk, struct flowi6 *fl6, u32 mtu)
kfree_skb(skb);
}

static void ip6_datagram_prepare_pktinfo_errqueue(struct sk_buff *skb)
/* IPv6 supports cmsg on all origins aside from SO_EE_ORIGIN_LOCAL.
*
* At one point, excluding local errors was a quick test to identify icmp/icmp6
* errors. This is no longer true, but the test remained, so the v6 stack,
* unlike v4, also honors cmsg requests on all wifi and timestamp errors.
*
* Timestamp code paths do not initialize the fields expected by cmsg:
* the PKTINFO fields in skb->cb[]. Fill those in here.
*/
static bool ip6_datagram_support_cmsg(struct sk_buff *skb,
struct sock_exterr_skb *serr)
{
int ifindex = skb->dev ? skb->dev->ifindex : -1;
if (serr->ee.ee_origin == SO_EE_ORIGIN_ICMP ||
serr->ee.ee_origin == SO_EE_ORIGIN_ICMP6)
return true;

if (serr->ee.ee_origin == SO_EE_ORIGIN_LOCAL)
return false;

if (!skb->dev)
return false;

if (skb->protocol == htons(ETH_P_IPV6))
IP6CB(skb)->iif = ifindex;
IP6CB(skb)->iif = skb->dev->ifindex;
else
PKTINFO_SKB_CB(skb)->ipi_ifindex = ifindex;
PKTINFO_SKB_CB(skb)->ipi_ifindex = skb->dev->ifindex;

return true;
}

/*
Expand Down Expand Up @@ -369,7 +389,7 @@ int ipv6_recv_error(struct sock *sk, struct msghdr *msg, int len, int *addr_len)

serr = SKB_EXT_ERR(skb);

if (sin && skb->len) {
if (sin && serr->port) {
const unsigned char *nh = skb_network_header(skb);
sin->sin6_family = AF_INET6;
sin->sin6_flowinfo = 0;
Expand All @@ -394,14 +414,11 @@ int ipv6_recv_error(struct sock *sk, struct msghdr *msg, int len, int *addr_len)
memcpy(&errhdr.ee, &serr->ee, sizeof(struct sock_extended_err));
sin = &errhdr.offender;
memset(sin, 0, sizeof(*sin));
if (serr->ee.ee_origin != SO_EE_ORIGIN_LOCAL && skb->len) {

if (ip6_datagram_support_cmsg(skb, serr)) {
sin->sin6_family = AF_INET6;
if (np->rxopt.all) {
if (serr->ee.ee_origin != SO_EE_ORIGIN_ICMP &&
serr->ee.ee_origin != SO_EE_ORIGIN_ICMP6)
ip6_datagram_prepare_pktinfo_errqueue(skb);
if (np->rxopt.all)
ip6_datagram_recv_common_ctl(sk, msg, skb);
}
if (skb->protocol == htons(ETH_P_IPV6)) {
sin->sin6_addr = ipv6_hdr(skb)->saddr;
if (np->rxopt.all)
Expand Down
4 changes: 2 additions & 2 deletions net/rxrpc/ar-error.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,15 +42,15 @@ void rxrpc_UDP_error_report(struct sock *sk)
_leave("UDP socket errqueue empty");
return;
}
if (!skb->len) {
serr = SKB_EXT_ERR(skb);
if (!skb->len && serr->ee.ee_origin == SO_EE_ORIGIN_TIMESTAMPING) {
_leave("UDP empty message");
kfree_skb(skb);
return;
}

rxrpc_new_skb(skb);

serr = SKB_EXT_ERR(skb);
addr = *(__be32 *)(skb_network_header(skb) + serr->addr_offset);
port = serr->port;

Expand Down

0 comments on commit c247f05

Please sign in to comment.