Netdev List
 help / color / mirror / Atom feed
* [PATCH net] cipso_v4: fail the walk on malformed options
@ 2026-10-21  9:35 omniameasunt
  2026-10-21  9:39 ` netdev-bot+sinfo
  2026-10-21 21:14 ` Paul Moore
  0 siblings, 2 replies; 5+ messages in thread
From: omniameasunt @ 2026-10-21  9:35 UTC (permalink / raw)
  To: netdev@vger.kernel.org; +Cc: paul@paul-moore.com

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 56-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.

Fixes: 014ab19a69c3 ("selinux: Set socket NetLabel based on connection endpoint")
Signed-off-by: Lucas Dupré--Rabinovici <omniameasunt@proton.me>

---
 net/ipv4/cipso_ipv4.c | 22 +++++++++++++++--------
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/net/ipv4/cipso_ipv4.c b/net/ipv4/cipso_ipv4.c
--- 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;
 		}
 	}
@@ -2028,6 +2032,13 @@
 
 		optlen_new = cipso_v4_get_actual_opt_len(opt->opt.__data,
 							 opt->opt.optlen);
+		if (optlen_new < 0) {
+			/* malformed option stream, drop all the options */
+			*opt_ptr = NULL;
+			hdr_delta = opt->opt.optlen;
+			kfree_rcu(opt, rcu);
+			return hdr_delta;
+		}
 		hdr_delta = opt->opt.optlen;
 		opt->opt.optlen = (optlen_new + 3) & ~3;
 		hdr_delta -= opt->opt.optlen;
@@ -2249,7 +2260,7 @@
 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;
+	    hdr_len_delta, optlen;
 	struct iphdr *iph;
 	struct ip_options *opt = &IPCB(skb)->opt;
 	unsigned char *cipso_ptr;
@@ -2266,9 +2277,12 @@
 	cipso_ptr = (unsigned char *)iph + opt->cipso;
 	cipso_len = cipso_ptr[1];
 
-	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)
+		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;


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net] cipso_v4: fail the walk on malformed options
@ 2026-10-21  9:39 ` netdev-bot+sinfo
  2026-10-21 15:44   ` omniameasunt
  0 siblings, 1 reply; 5+ messages in thread
From: netdev-bot+sinfo @ 2026-10-21  9:39 UTC (permalink / raw)
  To: omniameasunt; +Cc: netdev@vger.kernel.org, paul@paul-moore.com

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.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net] cipso_v4: fail the walk on malformed options
  2026-10-21  9:39 ` netdev-bot+sinfo
@ 2026-10-21 15:44   ` omniameasunt
  0 siblings, 0 replies; 5+ messages in thread
From: omniameasunt @ 2026-10-21 15:44 UTC (permalink / raw)
  To: netdev-bot+sinfo; +Cc: netdev@vger.kernel.org, paul@paul-moore.com

Discovery: found with Clang static analysis using a custom checker, then verified against the affected code paths.

Triggered: yes. On an unmodified v7.3-rc6 kernel with CIPSO labeling configured.

The information-disclosure impact was demonstrated, and on a build without stack protector the overflow was demonstrated to be exploitable.  The full analysis and a POC were sent privately to the security team and the maintainer before this patch was posted here at the maintainer's request.



-------- Original Message --------
On Wednesday, 10/21/26 at 11:39 netdev-bot+sinfo@kernel.org wrote:
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.


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net] cipso_v4: fail the walk on malformed options
  2026-10-21  9:35 omniameasunt
  2026-10-21  9:39 ` netdev-bot+sinfo
@ 2026-10-21 21:14 ` Paul Moore
  1 sibling, 0 replies; 5+ messages in thread
From: Paul Moore @ 2026-10-21 21:14 UTC (permalink / raw)
  To: omniameasunt; +Cc: netdev@vger.kernel.org

On Wed, Oct 21, 2026 at 5:35 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 56-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.
>
> Fixes: 014ab19a69c3 ("selinux: Set socket NetLabel based on connection endpoint")
> Signed-off-by: Lucas Dupré--Rabinovici <omniameasunt@proton.me>
>
> ---
>  net/ipv4/cipso_ipv4.c | 22 +++++++++++++++--------
>  1 file changed, 18 insertions(+), 4 deletions(-)

Thanks Lucas!

Acked-by: Paul Moore <paul@paul-moore.com>

> diff --git a/net/ipv4/cipso_ipv4.c b/net/ipv4/cipso_ipv4.c
> --- 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;
>                 }
>         }
> @@ -2028,6 +2032,13 @@
>
>                 optlen_new = cipso_v4_get_actual_opt_len(opt->opt.__data,
>                                                          opt->opt.optlen);
> +               if (optlen_new < 0) {
> +                       /* malformed option stream, drop all the options */
> +                       *opt_ptr = NULL;
> +                       hdr_delta = opt->opt.optlen;
> +                       kfree_rcu(opt, rcu);
> +                       return hdr_delta;
> +               }

Future work could, and likely should, find a way to combine the error
handling above with the CIPSO-only code path in the same function, but
the separate approach taken above should make backporting easier.

>                 hdr_delta = opt->opt.optlen;
>                 opt->opt.optlen = (optlen_new + 3) & ~3;
>                 hdr_delta -= opt->opt.optlen;
> @@ -2249,7 +2260,7 @@
>  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;
> +           hdr_len_delta, optlen;
>         struct iphdr *iph;
>         struct ip_options *opt = &IPCB(skb)->opt;
>         unsigned char *cipso_ptr;
> @@ -2266,9 +2277,12 @@
>         cipso_ptr = (unsigned char *)iph + opt->cipso;
>         cipso_len = cipso_ptr[1];
>
> -       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)
> +               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;
>

-- 
paul-moore.com

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net] cipso_v4: fail the walk on malformed options
@ 2026-10-25 11:31 netdev-bot+sashiko
  0 siblings, 0 replies; 5+ messages in thread
From: netdev-bot+sashiko @ 2026-10-25 11:31 UTC (permalink / raw)
  To: omniameasunt; +Cc: netdev, paul, kuba

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.

Critical: 0 · High: 2 (1) · Medium: 0 · Low: 0 (1)

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] cipso_v4_delopt() (net/ipv4/cipso_ipv4.c) still walks the option
  buffer with the old, uncompacted length.
- [High] cipso_v4: RCU race from in-place option mutation in
  cipso_v4_delopt()

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but on its normal
  path cipso_v4_delopt() (net/ipv4/cipso_ipv4.c) changes an…
- [Low] The kernel-doc of cipso_v4_delopt() says 'Returns zero on
  success, negative values on failure', but the function only ever…

--

cipso_v4: fail the walk on malformed options

This commit adds checks to cipso_v4_get_actual_opt_len(). The option walk
now returns -EINVAL when an option is truncated, has a length byte of zero
or one, or runs past len. On that error, cipso_v4_delopt() drops the whole
option set and cipso_v4_skbuff_delattr() leaves the packet unchanged.

> diff --git a/net/ipv4/cipso_ipv4.c b/net/ipv4/cipso_ipv4.c
> index 1aacbbeffbc64..d2d95b3a745f0 100644
> --- a/net/ipv4/cipso_ipv4.c
> +++ b/net/ipv4/cipso_ipv4.c

[ ... ]

> @@ -2028,6 +2032,13 @@ static int cipso_v4_delopt(struct ip_options_rcu __rcu **opt_ptr)
>  
>  		optlen_new = cipso_v4_get_actual_opt_len(opt->opt.__data,
>  							 opt->opt.optlen);
> +		if (optlen_new < 0) {
> +			/* malformed option stream, drop all the options */
> +			*opt_ptr = NULL;

[Severity: High]
This overlaps with the in-place modification question raised for the
sendmsg paths. getsockopt(IP_OPTIONS) looks like a more direct way to
corrupt memory through the same race, so it is probably worth covering
in the same fix.

cipso_v4_delopt() lowers opt->opt.srr, rr, ts and router_alert, clears
cipso and memmoves __data while *opt_ptr still publishes the object.
The new error branch only clears *opt_ptr after all of that has run,
and the normal path then shrinks opt->opt.optlen in place. kfree_rcu()
keeps the memory alive for readers already inside rcu_read_lock(), but
it does nothing to keep the contents consistent for them.

In this tree do_ip_getsockopt(IP_OPTIONS) is one such reader. It
copies sizeof(struct ip_options) + inet_opt->opt.optlen bytes into the
56-byte optbuf on the stack under rcu_read_lock() alone. It then calls
ip_options_undo() on the copy, which trusts the offsets to point at
real option headers:

	memmove(optptr + 7, optptr + 3, optptr[1] - 7);
	...
	optptr[2] -= 4;
	memset(&optptr[optptr[2] - 1], 0, 4);

Take CIPSO(13) followed by RR(7). Suppose the copy pairs the reduced rr
offset with __data that has not been moved yet. Then optptr points at
the old CIPSO type byte, and optptr[2] is the first DOI byte. A DOI
below 0x04000000 makes optptr[2] wrap to 252..255. The memset() then
writes four zero bytes around 200 bytes past the end of optbuf.

The srr case is similar. A stale length byte below 7 underflows the
memmove() size to a huge size_t, and a large one moves up to 248 bytes
inside a 56-byte buffer. The ts_needaddr/ts_needtime path has the same
wrap.

Either way this is a kernel stack overwrite. It is reachable from
getsockopt(IP_OPTIONS) racing connect(AF_UNSPEC), under the same
CAP_NET_RAW and CIPSO preconditions the commit message describes.

Most of this predates the patch. But the new branch already unpublishes
and frees the object. Would it make sense to do the compaction on a
freshly allocated copy, publish it with rcu_assign_pointer(), and
kfree_rcu() the old one? Readers would then only ever see the old or
the new option set. The socket and request_sock callers run in
different contexts, so the allocation flags would need some thought.

[Severity: High]
Does this walk still read the stale tail that the commit message names as
the attacker-controlled input?

The memmove() just above compacts the stream:

	memmove(cipso_ptr, cipso_ptr + cipso_len,
		opt->opt.optlen - cipso_off - cipso_len);

After it, the last cipso_len bytes of __data are stale copies of the
original bytes. Nothing refills them with IPOPT_END.

cipso_v4_get_actual_opt_len() is then called with the old, uncompacted
opt->opt.optlen. If the original stream length was already a multiple of
4, ip_options_get() added no END padding, so the walk runs into the stale
bytes. The new checks only bound the result to len. They can't tell a live
option from a stale one.

This seems to cause two problems.

First, valid options can be dropped. Take CIPSO(13) followed by RR(7), 20
bytes, no padding:

  86 0d <DOI x4> 01 07 00 <lvl> <bm x3> | 07 <len 7> ...

After compaction RR sits in __data[0..6]. __data[7] is the stale tag
length (0x07) and __data[8] is the stale RBM alignment octet (0x00). The
new "data[iter + 1] < 2" check returns -EINVAL, and this branch frees the
whole ip_options_rcu, valid RR included. CIPSO + NOP NOP + RA(4) behaves
the same way. Before this patch these inputs looped forever; now every
option is silently lost.

A user with CAP_NET_RAW in a user namespace can reach this on a socket in
the NLBL_REQSKB state. They set those options with IP_OPTIONS and then call
connect(AF_UNSPEC):

  selinux_netlbl_socket_connect_locked()
    -> netlbl_sock_delattr()
      -> cipso_v4_sock_delattr()
        -> cipso_v4_delopt()

Second, stale bytes can stay live. Take RR(7) followed by CIPSO(13), 20
bytes. The memmove() length is 0. The walk accepts the old CIPSO bytes as
an ordinary option (iter == len), so optlen_new == optlen and hdr_delta is
0.

opt.cipso is cleared, but the CIPSO bytes stay in __data, and
ip_options_build() keeps writing them into outgoing headers.

__ip_options_echo() always puts CIPSO last and only pads to a multiple of
4. So cipso_v4_req_delattr() on a request_sock built from a peer's SYN
could hit this whenever RR/TS/SRR plus CIPSO is already 4-byte aligned.

Both cases contradict this part of the commit message:

  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.

Would walking only the compacted region (opt->opt.optlen - cipso_len), or
filling the vacated tail with IPOPT_END, fix both cases? This -EINVAL
branch would then only fire on corrupt data.

> +			hdr_delta = opt->opt.optlen;
> +			kfree_rcu(opt, rcu);
> +			return hdr_delta;

[Severity: Low]
This isn't a bug introduced by this patch, but the kernel-doc above
cipso_v4_delopt() says:

  * adjustments to the IP option structure.  Returns zero on success, negative
  * values on failure.

The function only ever returns 0 or a non-negative header delta.
cipso_v4_sock_delattr() subtracts that delta from icsk_ext_hdr_len when
hdr_delta > 0. The positive value returned here is correct for that
caller.

Could the comment be fixed while another return path is added under it?

> +		}
>  		hdr_delta = opt->opt.optlen;
>  		opt->opt.optlen = (optlen_new + 3) & ~3;
>  		hdr_delta -= opt->opt.optlen;

[Severity: High]
This is a pre-existing issue, but is it safe for cipso_v4_delopt() to
modify the ip_options_rcu in place while inet->inet_opt still points to it?

On this path, holding only lock_sock, it does these steps in order:

  1. lowers srr/rr/ts/router_alert by cipso_len
  2. clears cipso
  3. memmoves __data
  4. shrinks optlen

Lockless readers copy the object under rcu_read_lock() alone. For example,
udp_sendmsg():

	rcu_read_lock();
	inet_opt = rcu_dereference(inet->inet_opt);
	if (inet_opt) {
		memcpy(opt_copy, inet_opt,
		       sizeof(*inet_opt) + inet_opt->opt.optlen);

raw_sendmsg() and ping_v4_sendmsg() follow the same pattern.

So if connect(AF_UNSPEC) and sendmsg() run at the same time, sendmsg()
could copy offsets that were already reduced together with bytes that were
not yet moved. ip_options_build() uses both to compute a write address:

	if (opt->srr)
		memcpy(iph + opt->srr + iph[opt->srr + 1] - 4, &daddr, 4);

With [CIPSO][Y][LSRR], the reduced srr points at Y's old offset.
iph[srr + 1] would then read a stale CIPSO payload byte, which can be as
large as 0xff. The 4-byte write could land well past the 60-byte maximum
IP header. The rr_needaddr and ts_needaddr/ts_needtime paths look similar.

The new error branch above unpublishes the object before kfree_rcu(), but
this compaction path still changes the live object that readers can see.

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/CBLsHx48w8Bn1k_Xg6kJFliLNkxLCqi8SnkifapxYYpVtll4w-W_2JJePtbk9ssIS701IFJvi0Q2n5l-2GFFe8hNHU9Y_CgD_sdzcAbILVA%3D%40proton.me

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-10-25 11:31 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-25 11:31 [PATCH net] cipso_v4: fail the walk on malformed options netdev-bot+sashiko
  -- strict thread matches above, loose matches on Subject: below --
2026-10-21  9:35 omniameasunt
2026-10-21  9:39 ` netdev-bot+sinfo
2026-10-21 15:44   ` omniameasunt
2026-10-21 21:14 ` Paul Moore

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox