[PATCH net v2] cipso_v4: fail the walk on malformed options

[PATCH net v2] cipso_v4: fail the walk on malformed options

[omniameasunt]

cipso_v4_get_actual_opt_len() walks a compacted IP option buffer with no
validation of the option it is standing on: it reads data[iter + 1]
without checking that the length byte is inside the buffer, it accepts a
zero length byte (spinning forever), and it never clamps the walked
offset to len, so the returned length can exceed the 40-byte MAX_IPOPTLEN
bound that every caller assumes.

After cipso_v4_delopt() memmove-compacts a socket's options, the bytes
past the compacted region are stale and attacker-controlled via
setsockopt(IP_OPTIONS).  An unprivileged user (CAP_NET_RAW in a user
namespace, CIPSO labeling configured) can trigger the walk with
connect(AF_UNSPEC), which reaches cipso_v4_sock_delattr() through the
SELinux NetLabel hook.  The poisoned optlen is later used in
do_ip_getsockopt(), where it causes

  * an out-of-bounds read of the adjacent heap object copied back to
    userspace (kernel pointer disclosure, KASLR defeat), and
  * a stack buffer overflow when optlen exceeds the 60-byte scratch
    buffer (return-address overwrite; exploitable for privilege
    escalation on kernels built without stack protector).

The same helper is also called from cipso_v4_skbuff_delattr() when a
label is removed from output or forwarded packets; the option bytes there
have already been validated by ip_options_compile(), so the new checks
are defensive for that caller.

Fix the defects by making the walk fail with -EINVAL as soon as an
option header is truncated or malformed (length byte missing, zero, or
one) or an option would run past len.  cipso_v4_delopt() then drops the
socket's entire option set (reusing the existing branch that frees the
ip_options_rcu when CIPSO was the only option), and
cipso_v4_skbuff_delattr() leaves a malformed packet unchanged instead
of rewriting its header based on untrustworthy bytes.

Well-formed option streams are unaffected: a socket whose options
survive the compaction intact keeps them; only malformed streams now
lose all options instead of carrying a partially parsed set.

The function also compacted the option object in place while the
ip_options_rcu remained published.  Lockless readers such as
getsockopt(IP_OPTIONS), UDP sendmsg(), raw sendmsg(), and ping sendmsg()
copy the object under rcu_read_lock(), so they can observe a mixture of
old and new offsets or option bytes.  Build the compacted object in a
replacement ip_options_rcu, modify only the replacement, publish it with
rcu_assign_pointer(), and retire the old object with kfree_rcu().  If the
replacement cannot be allocated, leave the old object untouched and
return -ENOMEM.

Fixes: 014ab19a69c3 ("selinux: Set socket NetLabel based on connection endpoint")
Signed-off-by: Lucas Dupré--Rabinovici <omniameasunt@proton.me>
---
Previous version:
https://lore.kernel.org/netdev/CBLsHx48w8Bn1k_Xg6kJFliLNkxLCqi8SnkifapxYYpVtll4w-W_2JJePtbk9ssIS701IFJvi0Q2n5l-2GFFe8hNHU9Y_CgD_sdzcAbILVA=@proton.me/

--- a/net/ipv4/cipso_ipv4.c
+++ b/net/ipv4/cipso_ipv4.c
@@ -1824,7 +1824,11 @@
 		} else if (data[iter] == IPOPT_NOP) {
 			iter++;
 		} else {
+			if (iter + 1 >= len || data[iter + 1] < 2)
+				return -EINVAL;
 			iter += data[iter + 1];
+			if (iter > len)
+				return -EINVAL;
 			optlen = iter;
 		}
 	}
@@ -1992,53 +1996,79 @@
  *
  * Description:
  * Deletes the CIPSO IP option from a set of IP options and makes the necessary
- * adjustments to the IP option structure.  Returns zero on success, negative
- * values on failure.
+ * adjustments to the IP option structure.  Returns zero or a positive header
+ * length delta on success, negative values on failure.
  *
  */
 static int cipso_v4_delopt(struct ip_options_rcu __rcu **opt_ptr)
 {
 	struct ip_options_rcu *opt = rcu_dereference_protected(*opt_ptr, 1);
+	struct ip_options_rcu *new;
 	int hdr_delta = 0;
+	u8 cipso_len, cipso_off;
+	unsigned char *cipso_ptr;
+	int optlen, optlen_new;
 
 	if (!opt || opt->opt.cipso == 0)
 		return 0;
-	if (opt->opt.srr || opt->opt.rr || opt->opt.ts || opt->opt.router_alert) {
-		u8 cipso_len;
-		u8 cipso_off;
-		unsigned char *cipso_ptr;
-		int optlen_new;
-
-		cipso_off = opt->opt.cipso - sizeof(struct iphdr);
-		cipso_ptr = &opt->opt.__data[cipso_off];
-		cipso_len = cipso_ptr[1];
-
-		if (opt->opt.srr > opt->opt.cipso)
-			opt->opt.srr -= cipso_len;
-		if (opt->opt.rr > opt->opt.cipso)
-			opt->opt.rr -= cipso_len;
-		if (opt->opt.ts > opt->opt.cipso)
-			opt->opt.ts -= cipso_len;
-		if (opt->opt.router_alert > opt->opt.cipso)
-			opt->opt.router_alert -= cipso_len;
-		opt->opt.cipso = 0;
 
-		memmove(cipso_ptr, cipso_ptr + cipso_len,
-			opt->opt.optlen - cipso_off - cipso_len);
-
-		optlen_new = cipso_v4_get_actual_opt_len(opt->opt.__data,
-							 opt->opt.optlen);
-		hdr_delta = opt->opt.optlen;
-		opt->opt.optlen = (optlen_new + 3) & ~3;
-		hdr_delta -= opt->opt.optlen;
-	} else {
+	if (!opt->opt.srr && !opt->opt.rr && !opt->opt.ts &&
+	    !opt->opt.router_alert) {
 		/* only the cipso option was present on the socket so we can
 		 * remove the entire option struct */
-		*opt_ptr = NULL;
+		rcu_assign_pointer(*opt_ptr, NULL);
 		hdr_delta = opt->opt.optlen;
 		kfree_rcu(opt, rcu);
+		return hdr_delta;
 	}
 
+	new = kzalloc(sizeof(*new) + opt->opt.optlen, GFP_ATOMIC);
+	if (!new)
+		return -ENOMEM;
+	memcpy(&new->opt, &opt->opt, sizeof(new->opt) + opt->opt.optlen);
+
+	cipso_off = new->opt.cipso - sizeof(struct iphdr);
+	if (cipso_off + 1 >= new->opt.optlen)
+		goto drop_all;
+	cipso_ptr = &new->opt.__data[cipso_off];
+	cipso_len = cipso_ptr[1];
+	if (cipso_len < 2 || cipso_len > new->opt.optlen - cipso_off)
+		goto drop_all;
+
+	if (new->opt.srr > new->opt.cipso)
+		new->opt.srr -= cipso_len;
+	if (new->opt.rr > new->opt.cipso)
+		new->opt.rr -= cipso_len;
+	if (new->opt.ts > new->opt.cipso)
+		new->opt.ts -= cipso_len;
+	if (new->opt.router_alert > new->opt.cipso)
+		new->opt.router_alert -= cipso_len;
+	new->opt.cipso = 0;
+
+	memmove(cipso_ptr, cipso_ptr + cipso_len,
+		new->opt.optlen - cipso_off - cipso_len);
+
+	/* Only the compacted region is live.  The old tail is stale and
+	 * must not take part in the walk or remain visible as IP options.
+	 */
+	optlen = new->opt.optlen - cipso_len;
+	memset(new->opt.__data + optlen, IPOPT_END, cipso_len);
+	optlen_new = cipso_v4_get_actual_opt_len(new->opt.__data, optlen);
+	if (optlen_new < 0)
+		goto drop_all;
+
+	hdr_delta = opt->opt.optlen;
+	new->opt.optlen = (optlen_new + 3) & ~3;
+	hdr_delta -= new->opt.optlen;
+	rcu_assign_pointer(*opt_ptr, new);
+	kfree_rcu(opt, rcu);
+	return hdr_delta;
+
+drop_all:
+	kfree(new);
+	rcu_assign_pointer(*opt_ptr, NULL);
+	hdr_delta = opt->opt.optlen;
+	kfree_rcu(opt, rcu);
 	return hdr_delta;
 }
 
@@ -2248,14 +2278,18 @@
  */
 int cipso_v4_skbuff_delattr(struct sk_buff *skb)
 {
-	int ret_val, cipso_len, hdr_len_actual, new_hdr_len_actual, new_hdr_len,
-	    hdr_len_delta;
+	int ret_val, cipso_len, cipso_off, hdr_len_actual, new_hdr_len_actual;
+	int new_hdr_len, hdr_len_delta, optlen;
 	struct iphdr *iph;
 	struct ip_options *opt = &IPCB(skb)->opt;
 	unsigned char *cipso_ptr;
 
 	if (opt->cipso == 0)
 		return 0;
+	if (opt->cipso < sizeof(struct iphdr) ||
+	    opt->cipso - sizeof(struct iphdr) + 1 >= opt->optlen)
+		return -EINVAL;
+	cipso_off = opt->cipso - sizeof(struct iphdr);
 
 	/* since we are changing the packet we should make a copy */
 	ret_val = skb_cow(skb, skb_headroom(skb));
@@ -2263,12 +2297,19 @@
 		return ret_val;
 
 	iph = ip_hdr(skb);
+	if (opt->optlen > (iph->ihl << 2) - sizeof(struct iphdr))
+		return -EINVAL;
 	cipso_ptr = (unsigned char *)iph + opt->cipso;
 	cipso_len = cipso_ptr[1];
+	if (cipso_len < 2 || cipso_len > opt->optlen - cipso_off)
+		return -EINVAL;
 
-	hdr_len_actual = sizeof(struct iphdr) +
-			 cipso_v4_get_actual_opt_len((unsigned char *)(iph + 1),
-						     opt->optlen);
+	optlen = cipso_v4_get_actual_opt_len((unsigned char *)(iph + 1),
+					     opt->optlen);
+	/* malformed option stream, leave the packet unchanged */
+	if (optlen < 0 || cipso_off + cipso_len > optlen)
+		return -EINVAL;
+	hdr_len_actual = sizeof(struct iphdr) + optlen;
 	new_hdr_len_actual = hdr_len_actual - cipso_len;
 	new_hdr_len = (new_hdr_len_actual + 3) & ~3;
 	hdr_len_delta = (iph->ihl << 2) - new_hdr_len;

Re: [PATCH net v2] cipso_v4: fail the walk on malformed options

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by code inspection). If it was triggered please include
   the symptoms, like the stack trace or error messages.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

Re: [PATCH net v2] cipso_v4: fail the walk on malformed options

[Paul Moore]

On Tue, Oct 27, 2026 at 7:16 AM <omniameasunt@proton.me> wrote:
>
> cipso_v4_get_actual_opt_len() walks a compacted IP option buffer with no
> validation of the option it is standing on: it reads data[iter + 1]
> without checking that the length byte is inside the buffer, it accepts a
> zero length byte (spinning forever), and it never clamps the walked
> offset to len, so the returned length can exceed the 40-byte MAX_IPOPTLEN
> bound that every caller assumes.
>
> After cipso_v4_delopt() memmove-compacts a socket's options, the bytes
> past the compacted region are stale and attacker-controlled via
> setsockopt(IP_OPTIONS).  An unprivileged user (CAP_NET_RAW in a user
> namespace, CIPSO labeling configured) can trigger the walk with
> connect(AF_UNSPEC), which reaches cipso_v4_sock_delattr() through the
> SELinux NetLabel hook.  The poisoned optlen is later used in
> do_ip_getsockopt(), where it causes
>
>   * an out-of-bounds read of the adjacent heap object copied back to
>     userspace (kernel pointer disclosure, KASLR defeat), and
>   * a stack buffer overflow when optlen exceeds the 60-byte scratch
>     buffer (return-address overwrite; exploitable for privilege
>     escalation on kernels built without stack protector).
>
> The same helper is also called from cipso_v4_skbuff_delattr() when a
> label is removed from output or forwarded packets; the option bytes there
> have already been validated by ip_options_compile(), so the new checks
> are defensive for that caller.
>
> Fix the defects by making the walk fail with -EINVAL as soon as an
> option header is truncated or malformed (length byte missing, zero, or
> one) or an option would run past len.  cipso_v4_delopt() then drops the
> socket's entire option set (reusing the existing branch that frees the
> ip_options_rcu when CIPSO was the only option), and
> cipso_v4_skbuff_delattr() leaves a malformed packet unchanged instead
> of rewriting its header based on untrustworthy bytes.
>
> Well-formed option streams are unaffected: a socket whose options
> survive the compaction intact keeps them; only malformed streams now
> lose all options instead of carrying a partially parsed set.
>
> The function also compacted the option object in place while the
> ip_options_rcu remained published.  Lockless readers such as
> getsockopt(IP_OPTIONS), UDP sendmsg(), raw sendmsg(), and ping sendmsg()
> copy the object under rcu_read_lock(), so they can observe a mixture of
> old and new offsets or option bytes.  Build the compacted object in a
> replacement ip_options_rcu, modify only the replacement, publish it with
> rcu_assign_pointer(), and retire the old object with kfree_rcu().  If the
> replacement cannot be allocated, leave the old object untouched and
> return -ENOMEM.
>
> Fixes: 014ab19a69c3 ("selinux: Set socket NetLabel based on connection endpoint")
> Signed-off-by: Lucas Dupré--Rabinovici <omniameasunt@proton.me>
> ---
> Previous version:
> https://lore.kernel.org/netdev/CBLsHx48w8Bn1k_Xg6kJFliLNkxLCqi8SnkifapxYYpVtll4w-W_2JJePtbk9ssIS701IFJvi0Q2n5l-2GFFe8hNHU9Y_CgD_sdzcAbILVA=@proton.me/

Hi Lucas,

Thanks again for your help with this.

Qihang (added to the CC line) independently reported, and has been
working on a fix for some of the issues Sashiko identified in your
first patchset.  I expect that Qihang will be posting their patch
publicly very soon, perhaps you can take a look once it is posted?  I
suspect that basing your patch on top of Qihang's patch may simplify
some things.

-- 
paul-moore.com