Netdev List
 help / color / mirror / Atom feed
* [PATCH net 4/4] netlabel: validate CIPSO option against skb tail in netlbl_skbuff_getattr
@ 2026-05-14 16:51 Qi Tang
  2026-05-15  2:18 ` Paul Moore
  0 siblings, 1 reply; 3+ messages in thread
From: Qi Tang @ 2026-05-14 16:51 UTC (permalink / raw)
  To: davem, kuba, pabeni, edumazet
  Cc: netdev, lyutoon, stable, Qi Tang, Paul Moore, Simon Horman,
	linux-security-module

netlbl_skbuff_getattr() locates the CIPSO option in the IPv4 IP header
via cipso_v4_optptr() and hands the bare pointer to cipso_v4_getattr().
The consumer re-reads cipso[1] (option length), cipso[6] (tag type),
and then cipso_v4_parsetag_*() re-reads further bytes from the skb.

__ip_options_compile() validates these bytes only at parse time.  An
nftables LOCAL_IN payload write reachable from an unprivileged user
namespace can rewrite them after parse and before the SELinux/Smack
peer-label consume path (selinux_sock_rcv_skb_compat ->
selinux_netlbl_sock_rcv_skb -> netlbl_skbuff_getattr).  This is the
IPv4 analogue of the CALIPSO IPv6 trust-after-modification fixed in
the previous patch: the tag parsers walk the option using attacker-
controlled length bytes, producing slab-out-of-bounds reads whose
contents feed into the MLS access decision.

Validate the option fits within skb_tail_pointer(skb) before invoking
cipso_v4_getattr().

Runtime confirmation (Smack peer-label policy + nft LOCAL_IN
mutation of tag_len): UdpInDatagrams increments to 1 and recvfrom
returns the payload, showing netlbl_skbuff_getattr ->
cipso_v4_getattr -> cipso_v4_parsetag_rbm -> netlbl_bitmap_walk runs
end-to-end past the option's true bound; with this patch the
consume path short-circuits at the bounds check and the counter
stays 0.

Reported-by: Qi Tang <tpluszz77@gmail.com>
Reported-by: Tong Liu <lyutoon@gmail.com>
Fixes: 04f81f0154e4 ("cipso: don't use IPCB() to locate the CIPSO IP option")
Signed-off-by: Qi Tang <tpluszz77@gmail.com>
---
 net/netlabel/netlabel_kapi.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/net/netlabel/netlabel_kapi.c b/net/netlabel/netlabel_kapi.c
index 4af8ab76964e0..ace561a2904a4 100644
--- a/net/netlabel/netlabel_kapi.c
+++ b/net/netlabel/netlabel_kapi.c
@@ -1393,11 +1393,21 @@ int netlbl_skbuff_getattr(const struct sk_buff *skb,
 	unsigned char *ptr;
 
 	switch (family) {
-	case AF_INET:
+	case AF_INET: {
+		const unsigned char *tail = skb_tail_pointer(skb);
+		u8 opt_len, tag_len;
+
 		ptr = cipso_v4_optptr(skb);
-		if (ptr && cipso_v4_getattr(ptr, secattr) == 0)
+		if (!ptr || ptr + 8 > tail)
+			break;
+		opt_len = ptr[1];	/* total CIPSO option length */
+		tag_len = ptr[7];	/* first tag length */
+		if (ptr + opt_len > tail || ptr + 6 + tag_len > tail)
+			break;
+		if (cipso_v4_getattr(ptr, secattr) == 0)
 			return 0;
 		break;
+	}
 #if IS_ENABLED(CONFIG_IPV6)
 	case AF_INET6: {
 		const unsigned char *tail = skb_tail_pointer(skb);
-- 
2.47.3


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

* Re: [PATCH net 4/4] netlabel: validate CIPSO option against skb tail in netlbl_skbuff_getattr
  2026-05-14 16:51 [PATCH net 4/4] netlabel: validate CIPSO option against skb tail in netlbl_skbuff_getattr Qi Tang
@ 2026-05-15  2:18 ` Paul Moore
  2026-05-15  2:42   ` Qi Tang
  0 siblings, 1 reply; 3+ messages in thread
From: Paul Moore @ 2026-05-15  2:18 UTC (permalink / raw)
  To: Qi Tang
  Cc: davem, kuba, pabeni, edumazet, netdev, lyutoon, stable,
	Simon Horman, linux-security-module

On Thu, May 14, 2026 at 12:52 PM Qi Tang <tpluszz77@gmail.com> wrote:
>
> netlbl_skbuff_getattr() locates the CIPSO option in the IPv4 IP header
> via cipso_v4_optptr() and hands the bare pointer to cipso_v4_getattr().
> The consumer re-reads cipso[1] (option length), cipso[6] (tag type),
> and then cipso_v4_parsetag_*() re-reads further bytes from the skb.
>
> __ip_options_compile() validates these bytes only at parse time.  An
> nftables LOCAL_IN payload write reachable from an unprivileged user
> namespace can rewrite them after parse and before the SELinux/Smack
> peer-label consume path (selinux_sock_rcv_skb_compat ->
> selinux_netlbl_sock_rcv_skb -> netlbl_skbuff_getattr).  This is the
> IPv4 analogue of the CALIPSO IPv6 trust-after-modification fixed in
> the previous patch: the tag parsers walk the option using attacker-
> controlled length bytes, producing slab-out-of-bounds reads whose
> contents feed into the MLS access decision.
>
> Validate the option fits within skb_tail_pointer(skb) before invoking
> cipso_v4_getattr().
>
> Runtime confirmation (Smack peer-label policy + nft LOCAL_IN
> mutation of tag_len): UdpInDatagrams increments to 1 and recvfrom
> returns the payload, showing netlbl_skbuff_getattr ->
> cipso_v4_getattr -> cipso_v4_parsetag_rbm -> netlbl_bitmap_walk runs
> end-to-end past the option's true bound; with this patch the
> consume path short-circuits at the bounds check and the counter
> stays 0.
>
> Reported-by: Qi Tang <tpluszz77@gmail.com>
> Reported-by: Tong Liu <lyutoon@gmail.com>
> Fixes: 04f81f0154e4 ("cipso: don't use IPCB() to locate the CIPSO IP option")
> Signed-off-by: Qi Tang <tpluszz77@gmail.com>
> ---
>  net/netlabel/netlabel_kapi.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/net/netlabel/netlabel_kapi.c b/net/netlabel/netlabel_kapi.c
> index 4af8ab76964e0..ace561a2904a4 100644
> --- a/net/netlabel/netlabel_kapi.c
> +++ b/net/netlabel/netlabel_kapi.c
> @@ -1393,11 +1393,21 @@ int netlbl_skbuff_getattr(const struct sk_buff *skb,
>         unsigned char *ptr;
>
>         switch (family) {
> -       case AF_INET:
> +       case AF_INET: {
> +               const unsigned char *tail = skb_tail_pointer(skb);
> +               u8 opt_len, tag_len;
> +
>                 ptr = cipso_v4_optptr(skb);
> -               if (ptr && cipso_v4_getattr(ptr, secattr) == 0)
> +               if (!ptr || ptr + 8 > tail)
> +                       break;

Similar to my CALIPSO comments, I suspect we would want to return an
error here, yes?

Also, how did you arrive at the magic number of '8' above?

> +               opt_len = ptr[1];       /* total CIPSO option length */
> +               tag_len = ptr[7];       /* first tag length */
> +               if (ptr + opt_len > tail || ptr + 6 + tag_len > tail)
> +                       break;
> +               if (cipso_v4_getattr(ptr, secattr) == 0)
>                         return 0;
>                 break;
> +       }
>  #if IS_ENABLED(CONFIG_IPV6)
>         case AF_INET6: {
>                 const unsigned char *tail = skb_tail_pointer(skb);
> --
> 2.47.3

-- 
paul-moore.com

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

* Re: [PATCH net 4/4] netlabel: validate CIPSO option against skb tail in netlbl_skbuff_getattr
  2026-05-15  2:18 ` Paul Moore
@ 2026-05-15  2:42   ` Qi Tang
  0 siblings, 0 replies; 3+ messages in thread
From: Qi Tang @ 2026-05-15  2:42 UTC (permalink / raw)
  To: paul
  Cc: davem, kuba, pabeni, edumazet, netdev, lyutoon, horms,
	linux-security-module, Qi Tang

Agreed on the return value, same reasoning as on 3/4: a length
mismatch here means post-parse mutation, and the unlabeled
fallback is the wrong default for that.  v2 returns -EINVAL on
all three CIPSO bounds checks.

The 8 is the offset of the first tag's length byte.  CIPSO option
header is type(1) + length(1) + DOI(4) = 6, plus the first tag
header type(1) + length(1) = 2.  We need ptr+8 readable before
dereferencing ptr[7].  v2 will document this inline, and use
CIPSO_V4_HDR_LEN if it's exposed in the header.

Qi

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

end of thread, other threads:[~2026-05-15  2:42 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-14 16:51 [PATCH net 4/4] netlabel: validate CIPSO option against skb tail in netlbl_skbuff_getattr Qi Tang
2026-05-15  2:18 ` Paul Moore
2026-05-15  2:42   ` Qi Tang

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