Commit 67d7ae33 authored by Jakub Kicinski's avatar Jakub Kicinski
Browse files
Pablo Neira Ayuso says:

====================
Netfilter fixes for net

The following patchset contains Netfilter fixes for net:

1) IEEE1394 ARP payload contains no target hardware address in the
   ARP packet. Apparently, arp_tables was never updated to deal with
   IEEE1394 ARP properly. To deal with this, return no match in case
   the target hardware address selector is used, either for inverse or
   normal match. Moreover, arpt_mangle disallows mangling of the target
   hardware and IP address because, it is not worth to adjust the
   offset calculation to fix this, we suspect no users of arp_tables
   for this family.

2) Use list_del_rcu() to delete device hooks in nf_tables, this hook
   list is RCU protected, concurrent netlink dump readers can be
   walking on this list, fix it by adding a helper function and use it
   for consistency. From Florian Westphal.

3) Add list_splice_rcu(), this is useful for joining the local list of
   new device hooks to the RCU protected hook list in chain and
   flowtable. Reviewed by Paul E. McKenney.

4) Use list_splice_rcu() to publish the new device hooks in chain and
   flowtable to fix concurrent netlink dump traversal.

5) Add a new hook transaction object to track device hook deletions.
   The current approach moves device hooks to be deleted around during
   the preparation phase, this breaks concurrent RCU reader via netlink
   dump. This new hook transaction is combined with NFT_HOOK_REMOVE
   flag to annotate hooks for removal in the preparation phase.

6) xt_policy inbound policy check in strict mode can lead to
   out-of-bound access of the secpath array due to incorrect.
   The iteration over the secpath needs to be reversed in the inbound
   to check for the human readable policy, expecting inner in first
   position and outer in second position, the secpath from inbound
   actually stores outer in first position then in second position.
   From Jiexun Wang.

7) Fix possible zero shift in nft_bitwise triggering UBSAN splat,
   reject zero shift from control plane, from Kai Ma.

8) Replace simple_strtoul() in the conntrack SIP helper since it relies
   on nul-terminated strings. From Florian Westphal.

* tag 'nf-26-04-28' of git://git.kernel.org/pub/scm/linux/kernel/git/netfilter/nf:
  netfilter: nf_conntrack_sip: don't use simple_strtoul
  netfilter: reject zero shift in nft_bitwise
  netfilter: xt_policy: fix strict mode inbound policy matching
  netfilter: nf_tables: add hook transactions for device deletions
  netfilter: nf_tables: join hook list via splice_list_rcu() in commit phase
  rculist: add list_splice_rcu() for private lists
  netfilter: nf_tables: use list_del_rcu for netlink hooks
  netfilter: arp_tables: fix IEEE1394 ARP payload parsing
====================

Link: https://patch.msgid.link/20260428095840.51961-1-pablo@netfilter.org


Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parents 46f74a3f 8cf6809c
Loading
Loading
Loading
Loading
+29 −0
Original line number Diff line number Diff line
@@ -261,6 +261,35 @@ static inline void list_replace_rcu(struct list_head *old,
	old->prev = LIST_POISON2;
}

static inline void __list_splice_rcu(struct list_head *list,
				     struct list_head *prev,
				     struct list_head *next)
{
	struct list_head *first = list->next;
	struct list_head *last = list->prev;

	last->next = next;
	first->prev = prev;
	next->prev = last;
	rcu_assign_pointer(list_next_rcu(prev), first);
}

/**
 * list_splice_rcu - splice a non-RCU list into an RCU-protected list,
 *                   designed for stacks.
 * @list:	the non RCU-protected list to splice
 * @head:	the place in the existing RCU-protected list to splice
 *
 * The list pointed to by @head can be RCU-read traversed concurrently with
 * this function.
 */
static inline void list_splice_rcu(struct list_head *list,
				   struct list_head *head)
{
	if (!list_empty(list))
		__list_splice_rcu(list, head, head->next);
}

/**
 * __list_splice_init_rcu - join an RCU-protected list into an existing list.
 * @list:	the RCU-protected list to splice
+13 −0
Original line number Diff line number Diff line
@@ -1204,12 +1204,15 @@ struct nft_stats {
	struct u64_stats_sync	syncp;
};

#define NFT_HOOK_REMOVE	(1 << 0)

struct nft_hook {
	struct list_head	list;
	struct list_head	ops_list;
	struct rcu_head		rcu;
	char			ifname[IFNAMSIZ];
	u8			ifnamelen;
	u8			flags;
};

struct nf_hook_ops *nft_hook_find_ops(const struct nft_hook *hook,
@@ -1664,6 +1667,16 @@ struct nft_trans {
	u8				put_net:1;
};

/**
 * struct nft_trans_hook - nf_tables hook update in transaction
 * @list: used internally
 * @hook: struct nft_hook with the device hook
 */
struct nft_trans_hook {
	struct list_head		list;
	struct nft_hook			*hook;
};

/**
 * struct nft_trans_binding - nf_tables object with binding support in transaction
 * @nft_trans:    base structure, MUST be first member
+15 −3
Original line number Diff line number Diff line
@@ -110,13 +110,25 @@ static inline int arp_packet_match(const struct arphdr *arphdr,
	arpptr += dev->addr_len;
	memcpy(&src_ipaddr, arpptr, sizeof(u32));
	arpptr += sizeof(u32);

	if (IS_ENABLED(CONFIG_FIREWIRE_NET) && dev->type == ARPHRD_IEEE1394) {
		if (unlikely(memchr_inv(arpinfo->tgt_devaddr.mask, 0,
					sizeof(arpinfo->tgt_devaddr.mask))))
			return 0;

		tgt_devaddr = NULL;
	} else {
		tgt_devaddr = arpptr;
		arpptr += dev->addr_len;
	}
	memcpy(&tgt_ipaddr, arpptr, sizeof(u32));

	if (NF_INVF(arpinfo, ARPT_INV_SRCDEVADDR,
		    arp_devaddr_compare(&arpinfo->src_devaddr, src_devaddr,
					dev->addr_len)) ||
					dev->addr_len)))
		return 0;

	if (tgt_devaddr &&
	    NF_INVF(arpinfo, ARPT_INV_TGTDEVADDR,
		    arp_devaddr_compare(&arpinfo->tgt_devaddr, tgt_devaddr,
					dev->addr_len)))
+8 −0
Original line number Diff line number Diff line
@@ -40,6 +40,10 @@ target(struct sk_buff *skb, const struct xt_action_param *par)
	}
	arpptr += pln;
	if (mangle->flags & ARPT_MANGLE_TDEV) {
		if (unlikely(IS_ENABLED(CONFIG_FIREWIRE_NET) &&
			     skb->dev->type == ARPHRD_IEEE1394))
			return NF_DROP;

		if (ARPT_DEV_ADDR_LEN_MAX < hln ||
		   (arpptr + hln > skb_tail_pointer(skb)))
			return NF_DROP;
@@ -47,6 +51,10 @@ target(struct sk_buff *skb, const struct xt_action_param *par)
	}
	arpptr += hln;
	if (mangle->flags & ARPT_MANGLE_TIP) {
		if (unlikely(IS_ENABLED(CONFIG_FIREWIRE_NET) &&
			     skb->dev->type == ARPHRD_IEEE1394))
			return NF_DROP;

		if (ARPT_MANGLE_ADDR_LEN_MAX < pln ||
		   (arpptr + pln > skb_tail_pointer(skb)))
			return NF_DROP;
+118 −34
Original line number Diff line number Diff line
@@ -181,6 +181,57 @@ static int sip_parse_addr(const struct nf_conn *ct, const char *cp,
	return 1;
}

/* Parse optional port number after IP address.
 * Returns false on malformed input, true otherwise.
 * If port is non-NULL, stores parsed port in network byte order.
 * If no port is present, sets *port to default SIP port.
 */
static bool sip_parse_port(const char *dptr, const char **endp,
			   const char *limit, __be16 *port)
{
	unsigned int p = 0;
	int len = 0;

	if (dptr >= limit)
		return false;

	if (*dptr != ':') {
		if (port)
			*port = htons(SIP_PORT);
		if (endp)
			*endp = dptr;
		return true;
	}

	dptr++; /* skip ':' */

	while (dptr < limit && isdigit(*dptr)) {
		p = p * 10 + (*dptr - '0');
		dptr++;
		len++;
		if (len > 5) /* max "65535" */
			return false;
	}

	if (len == 0)
		return false;

	/* reached limit while parsing port */
	if (dptr >= limit)
		return false;

	if (p < 1024 || p > 65535)
		return false;

	if (port)
		*port = htons(p);

	if (endp)
		*endp = dptr;

	return true;
}

/* skip ip address. returns its length. */
static int epaddr_len(const struct nf_conn *ct, const char *dptr,
		      const char *limit, int *shift)
@@ -193,11 +244,8 @@ static int epaddr_len(const struct nf_conn *ct, const char *dptr,
		return 0;
	}

	/* Port number */
	if (*dptr == ':') {
		dptr++;
		dptr += digits_len(ct, dptr, limit, shift);
	}
	if (!sip_parse_port(dptr, &dptr, limit, NULL))
		return 0;
	return dptr - aux;
}

@@ -228,6 +276,51 @@ static int skp_epaddr_len(const struct nf_conn *ct, const char *dptr,
	return epaddr_len(ct, dptr, limit, shift);
}

/* simple_strtoul stops after first non-number character.
 * But as we're not dealing with c-strings, we can't rely on
 * hitting \r,\n,\0 etc. before moving past end of buffer.
 *
 * This is a variant of simple_strtoul, but doesn't require
 * a c-string.
 *
 * If value exceeds UINT_MAX, 0 is returned.
 */
static unsigned int sip_strtouint(const char *cp, unsigned int len, char **endp)
{
	const unsigned int max = sizeof("4294967295");
	unsigned int olen = len;
	const char *s = cp;
	u64 result = 0;

	if (len > max)
		len = max;

	while (olen > 0 && isdigit(*s)) {
		unsigned int value;

		if (len == 0)
			goto err;

		value = *s - '0';
		result = result * 10 + value;

		if (result > UINT_MAX)
			goto err;
		s++;
		len--;
		olen--;
	}

	if (endp)
		*endp = (char *)s;

	return result;
err:
	if (endp)
		*endp = (char *)cp;
	return 0;
}

/* Parse a SIP request line of the form:
 *
 * Request-Line = Method SP Request-URI SP SIP-Version CRLF
@@ -241,7 +334,6 @@ int ct_sip_parse_request(const struct nf_conn *ct,
{
	const char *start = dptr, *limit = dptr + datalen, *end;
	unsigned int mlen;
	unsigned int p;
	int shift = 0;

	/* Skip method and following whitespace */
@@ -267,14 +359,8 @@ int ct_sip_parse_request(const struct nf_conn *ct,

	if (!sip_parse_addr(ct, dptr, &end, addr, limit, true))
		return -1;
	if (end < limit && *end == ':') {
		end++;
		p = simple_strtoul(end, (char **)&end, 10);
		if (p < 1024 || p > 65535)
	if (!sip_parse_port(end, &end, limit, port))
		return -1;
		*port = htons(p);
	} else
		*port = htons(SIP_PORT);

	if (end == dptr)
		return 0;
@@ -509,7 +595,6 @@ int ct_sip_parse_header_uri(const struct nf_conn *ct, const char *dptr,
			    union nf_inet_addr *addr, __be16 *port)
{
	const char *c, *limit = dptr + datalen;
	unsigned int p;
	int ret;

	ret = ct_sip_walk_headers(ct, dptr, dataoff ? *dataoff : 0, datalen,
@@ -520,14 +605,8 @@ int ct_sip_parse_header_uri(const struct nf_conn *ct, const char *dptr,

	if (!sip_parse_addr(ct, dptr + *matchoff, &c, addr, limit, true))
		return -1;
	if (*c == ':') {
		c++;
		p = simple_strtoul(c, (char **)&c, 10);
		if (p < 1024 || p > 65535)
	if (!sip_parse_port(c, &c, limit, port))
		return -1;
		*port = htons(p);
	} else
		*port = htons(SIP_PORT);

	if (dataoff)
		*dataoff = c - dptr;
@@ -609,7 +688,7 @@ int ct_sip_parse_numerical_param(const struct nf_conn *ct, const char *dptr,
		return 0;

	start += strlen(name);
	*val = simple_strtoul(start, &end, 0);
	*val = sip_strtouint(start, limit - start, (char **)&end);
	if (start == end)
		return -1;
	if (matchoff && matchlen) {
@@ -1064,6 +1143,8 @@ static int process_sdp(struct sk_buff *skb, unsigned int protoff,

	mediaoff = sdpoff;
	for (i = 0; i < ARRAY_SIZE(sdp_media_types); ) {
		char *end;

		if (ct_sip_get_sdp_header(ct, *dptr, mediaoff, *datalen,
					  SDP_HDR_MEDIA, SDP_HDR_UNSPEC,
					  &mediaoff, &medialen) <= 0)
@@ -1079,8 +1160,8 @@ static int process_sdp(struct sk_buff *skb, unsigned int protoff,
		mediaoff += t->len;
		medialen -= t->len;

		port = simple_strtoul(*dptr + mediaoff, NULL, 10);
		if (port == 0)
		port = sip_strtouint(*dptr + mediaoff, *datalen - mediaoff, (char **)&end);
		if (port == 0 || *dptr + mediaoff == end)
			continue;
		if (port < 1024 || port > 65535) {
			nf_ct_helper_log(skb, ct, "wrong port %u", port);
@@ -1254,7 +1335,7 @@ static int process_register_request(struct sk_buff *skb, unsigned int protoff,
	 */
	if (ct_sip_get_header(ct, *dptr, 0, *datalen, SIP_HDR_EXPIRES,
			      &matchoff, &matchlen) > 0)
		expires = simple_strtoul(*dptr + matchoff, NULL, 10);
		expires = sip_strtouint(*dptr + matchoff, *datalen - matchoff, NULL);

	ret = ct_sip_parse_header_uri(ct, *dptr, NULL, *datalen,
				      SIP_HDR_CONTACT, NULL,
@@ -1358,7 +1439,7 @@ static int process_register_response(struct sk_buff *skb, unsigned int protoff,

	if (ct_sip_get_header(ct, *dptr, 0, *datalen, SIP_HDR_EXPIRES,
			      &matchoff, &matchlen) > 0)
		expires = simple_strtoul(*dptr + matchoff, NULL, 10);
		expires = sip_strtouint(*dptr + matchoff, *datalen - matchoff, NULL);

	while (1) {
		unsigned int c_expires = expires;
@@ -1418,10 +1499,12 @@ static int process_sip_response(struct sk_buff *skb, unsigned int protoff,
	struct nf_conn *ct = nf_ct_get(skb, &ctinfo);
	unsigned int matchoff, matchlen, matchend;
	unsigned int code, cseq, i;
	char *end;

	if (*datalen < strlen("SIP/2.0 200"))
		return NF_ACCEPT;
	code = simple_strtoul(*dptr + strlen("SIP/2.0 "), NULL, 10);
	code = sip_strtouint(*dptr + strlen("SIP/2.0 "),
			     *datalen - strlen("SIP/2.0 "), NULL);
	if (!code) {
		nf_ct_helper_log(skb, ct, "cannot get code");
		return NF_DROP;
@@ -1432,8 +1515,8 @@ static int process_sip_response(struct sk_buff *skb, unsigned int protoff,
		nf_ct_helper_log(skb, ct, "cannot parse cseq");
		return NF_DROP;
	}
	cseq = simple_strtoul(*dptr + matchoff, NULL, 10);
	if (!cseq && *(*dptr + matchoff) != '0') {
	cseq = sip_strtouint(*dptr + matchoff, *datalen - matchoff, (char **)&end);
	if (*dptr + matchoff == end) {
		nf_ct_helper_log(skb, ct, "cannot get cseq");
		return NF_DROP;
	}
@@ -1482,6 +1565,7 @@ static int process_sip_request(struct sk_buff *skb, unsigned int protoff,

	for (i = 0; i < ARRAY_SIZE(sip_handlers); i++) {
		const struct sip_handler *handler;
		char *end;

		handler = &sip_handlers[i];
		if (handler->request == NULL)
@@ -1498,8 +1582,8 @@ static int process_sip_request(struct sk_buff *skb, unsigned int protoff,
			nf_ct_helper_log(skb, ct, "cannot parse cseq");
			return NF_DROP;
		}
		cseq = simple_strtoul(*dptr + matchoff, NULL, 10);
		if (!cseq && *(*dptr + matchoff) != '0') {
		cseq = sip_strtouint(*dptr + matchoff, *datalen - matchoff, (char **)&end);
		if (*dptr + matchoff == end) {
			nf_ct_helper_log(skb, ct, "cannot get cseq");
			return NF_DROP;
		}
@@ -1575,7 +1659,7 @@ static int sip_help_tcp(struct sk_buff *skb, unsigned int protoff,
				      &matchoff, &matchlen) <= 0)
			break;

		clen = simple_strtoul(dptr + matchoff, (char **)&end, 10);
		clen = sip_strtouint(dptr + matchoff, datalen - matchoff, (char **)&end);
		if (dptr + matchoff == end)
			break;

Loading