[PATCH net] cipso: stop modifying live ip_options_rcu

[PATCH net] cipso: stop modifying live ip_options_rcu

[Jann Horn]

cipso_v4_delopt() operates on the ip_options_rcu of a socket or a request
socket, and tries to remove CIPSO options from the middle by moving all
subsequent options.
That's not allowed - ip_options_rcu can be read locklessly and must always
be consistent. For example, ip_options_undo() reads trusted offsets and
length values from the options buffer.
Fix it by doing the modification in a copy.

The core change is in cipso_v4_delopt(), the rest is ugly plumbing to
propagate memory allocation failures.

Triggering this bug requires that the system is configured to use CIPSO.
(Setting up CIPSO requires privileges in init_user_ns.)

Fixes: f6d8bd051c39 ("inet: add RCU protection to inet->opt")
Cc: stable@vger.kernel.org
Signed-off-by: Jann Horn <jannh@google.com>
---
I've sent a separate mail with a reproducer to Paul Moore.
---
 include/net/cipso_ipv4.h     | 11 ++++++-----
 include/net/netlabel.h       | 11 ++++++-----
 net/ipv4/cipso_ipv4.c        | 16 +++++++++++++---
 net/netlabel/netlabel_kapi.c | 24 ++++++++++--------------
 security/selinux/netlabel.c  |  5 +++--
 security/smack/smack_lsm.c   | 14 +++++++++-----
 6 files changed, 47 insertions(+), 34 deletions(-)

diff --git a/include/net/cipso_ipv4.h b/include/net/cipso_ipv4.h
index d6780d7903f4..b41672221797 100644
--- a/include/net/cipso_ipv4.h
+++ b/include/net/cipso_ipv4.h
@@ -185,12 +185,12 @@ int cipso_v4_sock_setattr(struct sock *sk,
 			  const struct cipso_v4_doi *doi_def,
 			  const struct netlbl_lsm_secattr *secattr,
 			  bool sk_locked);
-void cipso_v4_sock_delattr(struct sock *sk);
+int cipso_v4_sock_delattr(struct sock *sk);
 int cipso_v4_sock_getattr(struct sock *sk, struct netlbl_lsm_secattr *secattr);
 int cipso_v4_req_setattr(struct request_sock *req,
 			 const struct cipso_v4_doi *doi_def,
 			 const struct netlbl_lsm_secattr *secattr);
-void cipso_v4_req_delattr(struct request_sock *req);
+int cipso_v4_req_delattr(struct request_sock *req);
 int cipso_v4_skbuff_setattr(struct sk_buff *skb,
 			    const struct cipso_v4_doi *doi_def,
 			    const struct netlbl_lsm_secattr *secattr);
@@ -221,8 +221,9 @@ static inline int cipso_v4_sock_setattr(struct sock *sk,
 	return -ENOSYS;
 }
 
-static inline void cipso_v4_sock_delattr(struct sock *sk)
+static inline int cipso_v4_sock_delattr(struct sock *sk)
 {
+	return 0;
 }
 
 static inline int cipso_v4_sock_getattr(struct sock *sk,
@@ -238,9 +239,9 @@ static inline int cipso_v4_req_setattr(struct request_sock *req,
 	return -ENOSYS;
 }
 
-static inline void cipso_v4_req_delattr(struct request_sock *req)
+static inline int cipso_v4_req_delattr(struct request_sock *req)
 {
-	return;
+	return 0;
 }
 
 static inline int cipso_v4_skbuff_setattr(struct sk_buff *skb,
diff --git a/include/net/netlabel.h b/include/net/netlabel.h
index 02914b1df38b..f896b9d52ed1 100644
--- a/include/net/netlabel.h
+++ b/include/net/netlabel.h
@@ -477,7 +477,7 @@ int netlbl_sock_setattr(struct sock *sk,
 			u16 family,
 			const struct netlbl_lsm_secattr *secattr,
 			bool sk_locked);
-void netlbl_sock_delattr(struct sock *sk);
+int netlbl_sock_delattr(struct sock *sk);
 int netlbl_sock_getattr(struct sock *sk,
 			struct netlbl_lsm_secattr *secattr);
 int netlbl_conn_setattr(struct sock *sk,
@@ -485,7 +485,7 @@ int netlbl_conn_setattr(struct sock *sk,
 			const struct netlbl_lsm_secattr *secattr);
 int netlbl_req_setattr(struct request_sock *req,
 		       const struct netlbl_lsm_secattr *secattr);
-void netlbl_req_delattr(struct request_sock *req);
+int netlbl_req_delattr(struct request_sock *req);
 int netlbl_skbuff_setattr(struct sk_buff *skb,
 			  u16 family,
 			  const struct netlbl_lsm_secattr *secattr);
@@ -626,8 +626,9 @@ static inline int netlbl_sock_setattr(struct sock *sk,
 {
 	return -ENOSYS;
 }
-static inline void netlbl_sock_delattr(struct sock *sk)
+static inline int netlbl_sock_delattr(struct sock *sk)
 {
+	return 0;
 }
 static inline int netlbl_sock_getattr(struct sock *sk,
 				      struct netlbl_lsm_secattr *secattr)
@@ -645,9 +646,9 @@ static inline int netlbl_req_setattr(struct request_sock *req,
 {
 	return -ENOSYS;
 }
-static inline void netlbl_req_delattr(struct request_sock *req)
+static inline int netlbl_req_delattr(struct request_sock *req)
 {
-	return;
+	return 0;
 }
 static inline int netlbl_skbuff_setattr(struct sk_buff *skb,
 				      u16 family,
diff --git a/net/ipv4/cipso_ipv4.c b/net/ipv4/cipso_ipv4.c
index e65a3f29d2ca..ee0c72fbc877 100644
--- a/net/ipv4/cipso_ipv4.c
+++ b/net/ipv4/cipso_ipv4.c
@@ -2004,11 +2004,16 @@ static int cipso_v4_delopt(struct ip_options_rcu __rcu **opt_ptr)
 	if (!opt || opt->opt.cipso == 0)
 		return 0;
 	if (opt->opt.srr || opt->opt.rr || opt->opt.ts || opt->opt.router_alert) {
+		struct ip_options_rcu *old_opt = opt;
 		u8 cipso_len;
 		u8 cipso_off;
 		unsigned char *cipso_ptr;
 		int optlen_new;
 
+		/* ip_options_rcu must not be modified in place, use a copy */
+		opt = kmemdup(opt, sizeof(*opt) + opt->opt.optlen, GFP_ATOMIC);
+		if (!opt)
+			return -ENOMEM;
 		cipso_off = opt->opt.cipso - sizeof(struct iphdr);
 		cipso_ptr = &opt->opt.__data[cipso_off];
 		cipso_len = cipso_ptr[1];
@@ -2036,6 +2041,8 @@ static int cipso_v4_delopt(struct ip_options_rcu __rcu **opt_ptr)
 
 		memset(&opt->opt.__data[optlen_new], IPOPT_END,
 		       opt->opt.optlen - optlen_new);
+		rcu_assign_pointer(*opt_ptr, opt);
+		kfree_rcu(old_opt, rcu);
 	} else {
 		/* only the cipso option was present on the socket so we can
 		 * remove the entire option struct */
@@ -2055,7 +2062,7 @@ static int cipso_v4_delopt(struct ip_options_rcu __rcu **opt_ptr)
  * Removes the CIPSO option from a socket, if present.
  *
  */
-void cipso_v4_sock_delattr(struct sock *sk)
+int cipso_v4_sock_delattr(struct sock *sk)
 {
 	struct inet_sock *sk_inet;
 	int hdr_delta;
@@ -2068,6 +2075,7 @@ void cipso_v4_sock_delattr(struct sock *sk)
 		sk_conn->icsk_ext_hdr_len -= hdr_delta;
 		sk_conn->icsk_sync_mss(sk, sk_conn->icsk_pmtu_cookie);
 	}
+	return (hdr_delta < 0) ? hdr_delta : 0;
 }
 
 /**
@@ -2078,9 +2086,11 @@ void cipso_v4_sock_delattr(struct sock *sk)
  * Removes the CIPSO option from a request socket, if present.
  *
  */
-void cipso_v4_req_delattr(struct request_sock *req)
+int cipso_v4_req_delattr(struct request_sock *req)
 {
-	cipso_v4_delopt(&inet_rsk(req)->ireq_opt);
+	int hdr_delta = cipso_v4_delopt(&inet_rsk(req)->ireq_opt);
+
+	return (hdr_delta < 0) ? hdr_delta : 0;
 }
 
 /**
diff --git a/net/netlabel/netlabel_kapi.c b/net/netlabel/netlabel_kapi.c
index 3583fa63dd01..c0eddb6afef3 100644
--- a/net/netlabel/netlabel_kapi.c
+++ b/net/netlabel/netlabel_kapi.c
@@ -1045,18 +1045,18 @@ int netlbl_sock_setattr(struct sock *sk,
  * ensuring that @sk is locked.
  *
  */
-void netlbl_sock_delattr(struct sock *sk)
+int netlbl_sock_delattr(struct sock *sk)
 {
 	switch (sk->sk_family) {
 	case AF_INET:
-		cipso_v4_sock_delattr(sk);
-		break;
+		return cipso_v4_sock_delattr(sk);
 #if IS_ENABLED(CONFIG_IPV6)
 	case AF_INET6:
 		calipso_sock_delattr(sk);
 		break;
 #endif /* IPv6 */
 	}
+	return 0;
 }
 
 /**
@@ -1156,8 +1156,7 @@ int netlbl_conn_setattr(struct sock *sk,
 		case NETLBL_NLTYPE_UNLABELED:
 			/* just delete the protocols we support for right now
 			 * but we could remove other protocols if needed */
-			netlbl_sock_delattr(sk);
-			ret_val = 0;
+			ret_val = netlbl_sock_delattr(sk);
 			break;
 		default:
 			ret_val = -ENOENT;
@@ -1185,8 +1184,7 @@ int netlbl_conn_setattr(struct sock *sk,
 		case NETLBL_NLTYPE_UNLABELED:
 			/* just delete the protocols we support for right now
 			 * but we could remove other protocols if needed */
-			netlbl_sock_delattr(sk);
-			ret_val = 0;
+			ret_val = netlbl_sock_delattr(sk);
 			break;
 		default:
 			ret_val = -ENOENT;
@@ -1234,8 +1232,7 @@ int netlbl_req_setattr(struct request_sock *req,
 						       entry->cipso, secattr);
 			break;
 		case NETLBL_NLTYPE_UNLABELED:
-			netlbl_req_delattr(req);
-			ret_val = 0;
+			ret_val = netlbl_req_delattr(req);
 			break;
 		default:
 			ret_val = -ENOENT;
@@ -1255,8 +1252,7 @@ int netlbl_req_setattr(struct request_sock *req,
 						      entry->calipso, secattr);
 			break;
 		case NETLBL_NLTYPE_UNLABELED:
-			netlbl_req_delattr(req);
-			ret_val = 0;
+			ret_val = netlbl_req_delattr(req);
 			break;
 		default:
 			ret_val = -ENOENT;
@@ -1280,18 +1276,18 @@ int netlbl_req_setattr(struct request_sock *req,
 * Remove all the NetLabel labeling from @req.
 *
 */
-void netlbl_req_delattr(struct request_sock *req)
+int netlbl_req_delattr(struct request_sock *req)
 {
 	switch (req->rsk_ops->family) {
 	case AF_INET:
-		cipso_v4_req_delattr(req);
-		break;
+		return cipso_v4_req_delattr(req);
 #if IS_ENABLED(CONFIG_IPV6)
 	case AF_INET6:
 		calipso_req_delattr(req);
 		break;
 #endif /* IPv6 */
 	}
+	return 0;
 }
 
 /**
diff --git a/security/selinux/netlabel.c b/security/selinux/netlabel.c
index d51dfe892312..24b11d1891d8 100644
--- a/security/selinux/netlabel.c
+++ b/security/selinux/netlabel.c
@@ -557,9 +557,10 @@ static int selinux_netlbl_socket_connect_helper(struct sock *sk,
 	 * is set to AF_UNSPEC, if that is what is happening we want to reset
 	 * the socket */
 	if (addr->sa_family == AF_UNSPEC) {
-		netlbl_sock_delattr(sk);
+		rc = netlbl_sock_delattr(sk);
+		if (rc)
+			return rc;
 		sksec->nlbl_state = NLBL_REQSKB;
-		rc = 0;
 		return rc;
 	}
 	secattr = selinux_netlbl_sock_genattr(sk);
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 28a8e12603ce..3a4a33568297 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -2656,22 +2656,26 @@ static int smack_netlbl_add(struct sock *sk)
  *
  * Remove the outbound smack value from a socket
  */
-static void smack_netlbl_delete(struct sock *sk)
+static int smack_netlbl_delete(struct sock *sk)
 {
 	struct socket_smack *ssp = smack_sock(sk);
+	int rc;
 
 	/*
 	 * Take the label off the socket if one is set.
 	 */
 	if (ssp->smk_state != SMK_NETLBL_LABELED)
-		return;
+		return 0;
 
 	local_bh_disable();
 	bh_lock_sock_nested(sk);
-	netlbl_sock_delattr(sk);
+	rc = netlbl_sock_delattr(sk);
 	bh_unlock_sock(sk);
 	local_bh_enable();
+	if (rc)
+		return rc;
 	ssp->smk_state = SMK_NETLBL_UNLABELED;
+	return 0;
 }
 
 /**
@@ -2711,7 +2715,7 @@ static int smk_ipv4_check(struct sock *sk, struct sockaddr_in *sap)
 		 * Clear the socket netlabel if it's set.
 		 */
 		if (!rc)
-			smack_netlbl_delete(sk);
+			rc = smack_netlbl_delete(sk);
 	}
 	rcu_read_unlock();
 
@@ -4548,7 +4552,7 @@ static int smack_inet_conn_request(const struct sock *sk, struct sk_buff *skb,
 	if (hskp == NULL)
 		rc = netlbl_req_setattr(req, &ssp->smk_out->smk_netlabel);
 	else
-		netlbl_req_delattr(req);
+		rc = netlbl_req_delattr(req);
 
 	return rc;
 }

---
base-commit: a91337fb5e8e0127c8b5be87be9f3027f95b48bc
change-id: 20261106-cipso-rcuopts-e26ffa45b392

Best regards,
--  
Jann Horn <jannh@google.com>

Re: [PATCH net] cipso: stop modifying live ip_options_rcu

[Eric Dumazet]

Le ven. 6 nov. 2026 à 22:40, Jann Horn <jannh@google.com> a écrit :
>
> cipso_v4_delopt() operates on the ip_options_rcu of a socket or a request
> socket, and tries to remove CIPSO options from the middle by moving all
> subsequent options.
> That's not allowed - ip_options_rcu can be read locklessly and must always
> be consistent. For example, ip_options_undo() reads trusted offsets and
> length values from the options buffer.
> Fix it by doing the modification in a copy.
>
> The core change is in cipso_v4_delopt(), the rest is ugly plumbing to
> propagate memory allocation failures.
>
> Triggering this bug requires that the system is configured to use CIPSO.
> (Setting up CIPSO requires privileges in init_user_ns.)
>
> Fixes: f6d8bd051c39 ("inet: add RCU protection to inet->opt")
> Cc: stable@vger.kernel.org
> Signed-off-by: Jann Horn <jannh@google.com>
> ---
> I've sent a separate mail with a reproducer to Paul Moore.

Reviewed-by: Eric Dumazet <edumazet@kernel.org>

Side note, not for this patch: Smack calls netlbl_sock_delattr()
and netlbl_sock_setattr() under bh_lock_sock_nested() only, which does
not exclude a concurrent lock_sock() owner such as
setsockopt(IP_OPTIONS). Both sides can then kfree_rcu() the same
old inet_opt. This already exists in the "cipso only" branch of
cipso_v4_delopt() and in cipso_v4_sock_setattr(), and with this patch
the copy branch has the same issue.

The rcu_dereference_protected(*opt_ptr, 1) in cipso_v4_delopt() hides
this from lockdep. Smack should probably use lock_sock() like
SELinux does in selinux_netlbl_socket_connect().

Thanks !

Re: [PATCH net] cipso: stop modifying live ip_options_rcu

[Paul Moore]

On Fri, Nov 6, 2026 at 6:06 PM Eric Dumazet <edumazet@kernel.org> wrote:
> Le ven. 6 nov. 2026 à 22:40, Jann Horn <jannh@google.com> a écrit :
> >
> > cipso_v4_delopt() operates on the ip_options_rcu of a socket or a request
> > socket, and tries to remove CIPSO options from the middle by moving all
> > subsequent options.
> > That's not allowed - ip_options_rcu can be read locklessly and must always
> > be consistent. For example, ip_options_undo() reads trusted offsets and
> > length values from the options buffer.
> > Fix it by doing the modification in a copy.
> >
> > The core change is in cipso_v4_delopt(), the rest is ugly plumbing to
> > propagate memory allocation failures.
> >
> > Triggering this bug requires that the system is configured to use CIPSO.
> > (Setting up CIPSO requires privileges in init_user_ns.)
> >
> > Fixes: f6d8bd051c39 ("inet: add RCU protection to inet->opt")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Jann Horn <jannh@google.com>
> > ---
> > I've sent a separate mail with a reproducer to Paul Moore.
>
> Reviewed-by: Eric Dumazet <edumazet@kernel.org>
>
> Side note, not for this patch: Smack calls netlbl_sock_delattr()
> and netlbl_sock_setattr() under bh_lock_sock_nested() only, which does
> not exclude a concurrent lock_sock() owner such as
> setsockopt(IP_OPTIONS). Both sides can then kfree_rcu() the same
> old inet_opt. This already exists in the "cipso only" branch of
> cipso_v4_delopt() and in cipso_v4_sock_setattr(), and with this patch
> the copy branch has the same issue.
>
> The rcu_dereference_protected(*opt_ptr, 1) in cipso_v4_delopt() hides
> this from lockdep. Smack should probably use lock_sock() like
> SELinux does in selinux_netlbl_socket_connect().

Casey?

-- 
paul-moore.com

Re: [PATCH net] cipso: stop modifying live ip_options_rcu

[Casey Schaufler]

On 11/6/2026 3:06 PM, Eric Dumazet wrote:
> Le ven. 6 nov. 2026 à 22:40, Jann Horn <jannh@google.com> a écrit :
>> cipso_v4_delopt() operates on the ip_options_rcu of a socket or a request
>> socket, and tries to remove CIPSO options from the middle by moving all
>> subsequent options.
>> That's not allowed - ip_options_rcu can be read locklessly and must always
>> be consistent. For example, ip_options_undo() reads trusted offsets and
>> length values from the options buffer.
>> Fix it by doing the modification in a copy.
>>
>> The core change is in cipso_v4_delopt(), the rest is ugly plumbing to
>> propagate memory allocation failures.
>>
>> Triggering this bug requires that the system is configured to use CIPSO.
>> (Setting up CIPSO requires privileges in init_user_ns.)
>>
>> Fixes: f6d8bd051c39 ("inet: add RCU protection to inet->opt")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Jann Horn <jannh@google.com>
>> ---
>> I've sent a separate mail with a reproducer to Paul Moore.
> Reviewed-by: Eric Dumazet <edumazet@kernel.org>
>
> Side note, not for this patch: Smack calls netlbl_sock_delattr()
> and netlbl_sock_setattr() under bh_lock_sock_nested() only, which does
> not exclude a concurrent lock_sock() owner such as
> setsockopt(IP_OPTIONS). Both sides can then kfree_rcu() the same
> old inet_opt. This already exists in the "cipso only" branch of
> cipso_v4_delopt() and in cipso_v4_sock_setattr(), and with this patch
> the copy branch has the same issue.
>
> The rcu_dereference_protected(*opt_ptr, 1) in cipso_v4_delopt() hides
> this from lockdep. Smack should probably use lock_sock() like
> SELinux does in selinux_netlbl_socket_connect().

Thank you. I will give this a try. I would be happy to accept a viable
patch from any of the numerous people who know locking better than I.

>
> Thanks !

Re: [PATCH net] cipso: stop modifying live ip_options_rcu

[Paul Moore]

On Fri, Nov 6, 2026 at 4:40 PM Jann Horn <jannh@google.com> wrote:
>
> cipso_v4_delopt() operates on the ip_options_rcu of a socket or a request
> socket, and tries to remove CIPSO options from the middle by moving all
> subsequent options.
> That's not allowed - ip_options_rcu can be read locklessly and must always
> be consistent. For example, ip_options_undo() reads trusted offsets and
> length values from the options buffer.
> Fix it by doing the modification in a copy.
>
> The core change is in cipso_v4_delopt(), the rest is ugly plumbing to
> propagate memory allocation failures.
>
> Triggering this bug requires that the system is configured to use CIPSO.
> (Setting up CIPSO requires privileges in init_user_ns.)
>
> Fixes: f6d8bd051c39 ("inet: add RCU protection to inet->opt")
> Cc: stable@vger.kernel.org
> Signed-off-by: Jann Horn <jannh@google.com>
> ---
> I've sent a separate mail with a reproducer to Paul Moore.
> ---
>  include/net/cipso_ipv4.h     | 11 ++++++-----
>  include/net/netlabel.h       | 11 ++++++-----
>  net/ipv4/cipso_ipv4.c        | 16 +++++++++++++---
>  net/netlabel/netlabel_kapi.c | 24 ++++++++++--------------
>  security/selinux/netlabel.c  |  5 +++--
>  security/smack/smack_lsm.c   | 14 +++++++++-----
>  6 files changed, 47 insertions(+), 34 deletions(-)

Thanks Jann, this looks good to me.  In a perfect world it would be
nice to get Casey's ACK on the Smack changes, but since they are very
minor and basically just pass the return code through I think it's
okay to merge this in another day or so if we don't hear from him.

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

-- 
paul-moore.com

Re: [PATCH net] cipso: stop modifying live ip_options_rcu

[Casey Schaufler]

On 11/9/2026 12:13 PM, Paul Moore wrote:
> On Fri, Nov 6, 2026 at 4:40 PM Jann Horn <jannh@google.com> wrote:
>> cipso_v4_delopt() operates on the ip_options_rcu of a socket or a request
>> socket, and tries to remove CIPSO options from the middle by moving all
>> subsequent options.
>> That's not allowed - ip_options_rcu can be read locklessly and must always
>> be consistent. For example, ip_options_undo() reads trusted offsets and
>> length values from the options buffer.
>> Fix it by doing the modification in a copy.
>>
>> The core change is in cipso_v4_delopt(), the rest is ugly plumbing to
>> propagate memory allocation failures.
>>
>> Triggering this bug requires that the system is configured to use CIPSO.
>> (Setting up CIPSO requires privileges in init_user_ns.)
>>
>> Fixes: f6d8bd051c39 ("inet: add RCU protection to inet->opt")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Jann Horn <jannh@google.com>
>> ---
>> I've sent a separate mail with a reproducer to Paul Moore.
>> ---
>>  include/net/cipso_ipv4.h     | 11 ++++++-----
>>  include/net/netlabel.h       | 11 ++++++-----
>>  net/ipv4/cipso_ipv4.c        | 16 +++++++++++++---
>>  net/netlabel/netlabel_kapi.c | 24 ++++++++++--------------
>>  security/selinux/netlabel.c  |  5 +++--
>>  security/smack/smack_lsm.c   | 14 +++++++++-----
>>  6 files changed, 47 insertions(+), 34 deletions(-)
> Thanks Jann, this looks good to me.  In a perfect world it would be
> nice to get Casey's ACK on the Smack changes, but since they are very
> minor and basically just pass the return code through I think it's
> okay to merge this in another day or so if we don't hear from him.

I don't see any reason to wait for my testing to complete. I'll shout
if anything fails.


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

Re: [PATCH net] cipso: stop modifying live ip_options_rcu

[Casey Schaufler]

On 11/6/2026 1:40 PM, Jann Horn wrote:
> cipso_v4_delopt() operates on the ip_options_rcu of a socket or a request
> socket, and tries to remove CIPSO options from the middle by moving all
> subsequent options.
> That's not allowed - ip_options_rcu can be read locklessly and must always
> be consistent. For example, ip_options_undo() reads trusted offsets and
> length values from the options buffer.
> Fix it by doing the modification in a copy.
>
> The core change is in cipso_v4_delopt(), the rest is ugly plumbing to
> propagate memory allocation failures.

I haven't had the opportunity to test this, but the Smack changes
appear innocuous. I will try them today, weather permitting.

>
> Triggering this bug requires that the system is configured to use CIPSO.
> (Setting up CIPSO requires privileges in init_user_ns.)
>
> Fixes: f6d8bd051c39 ("inet: add RCU protection to inet->opt")
> Cc: stable@vger.kernel.org
> Signed-off-by: Jann Horn <jannh@google.com>
> ---
> I've sent a separate mail with a reproducer to Paul Moore.
> ---
>  include/net/cipso_ipv4.h     | 11 ++++++-----
>  include/net/netlabel.h       | 11 ++++++-----
>  net/ipv4/cipso_ipv4.c        | 16 +++++++++++++---
>  net/netlabel/netlabel_kapi.c | 24 ++++++++++--------------
>  security/selinux/netlabel.c  |  5 +++--
>  security/smack/smack_lsm.c   | 14 +++++++++-----
>  6 files changed, 47 insertions(+), 34 deletions(-)
>
> diff --git a/include/net/cipso_ipv4.h b/include/net/cipso_ipv4.h
> index d6780d7903f4..b41672221797 100644
> --- a/include/net/cipso_ipv4.h
> +++ b/include/net/cipso_ipv4.h
> @@ -185,12 +185,12 @@ int cipso_v4_sock_setattr(struct sock *sk,
>  			  const struct cipso_v4_doi *doi_def,
>  			  const struct netlbl_lsm_secattr *secattr,
>  			  bool sk_locked);
> -void cipso_v4_sock_delattr(struct sock *sk);
> +int cipso_v4_sock_delattr(struct sock *sk);
>  int cipso_v4_sock_getattr(struct sock *sk, struct netlbl_lsm_secattr *secattr);
>  int cipso_v4_req_setattr(struct request_sock *req,
>  			 const struct cipso_v4_doi *doi_def,
>  			 const struct netlbl_lsm_secattr *secattr);
> -void cipso_v4_req_delattr(struct request_sock *req);
> +int cipso_v4_req_delattr(struct request_sock *req);
>  int cipso_v4_skbuff_setattr(struct sk_buff *skb,
>  			    const struct cipso_v4_doi *doi_def,
>  			    const struct netlbl_lsm_secattr *secattr);
> @@ -221,8 +221,9 @@ static inline int cipso_v4_sock_setattr(struct sock *sk,
>  	return -ENOSYS;
>  }
>  
> -static inline void cipso_v4_sock_delattr(struct sock *sk)
> +static inline int cipso_v4_sock_delattr(struct sock *sk)
>  {
> +	return 0;
>  }
>  
>  static inline int cipso_v4_sock_getattr(struct sock *sk,
> @@ -238,9 +239,9 @@ static inline int cipso_v4_req_setattr(struct request_sock *req,
>  	return -ENOSYS;
>  }
>  
> -static inline void cipso_v4_req_delattr(struct request_sock *req)
> +static inline int cipso_v4_req_delattr(struct request_sock *req)
>  {
> -	return;
> +	return 0;
>  }
>  
>  static inline int cipso_v4_skbuff_setattr(struct sk_buff *skb,
> diff --git a/include/net/netlabel.h b/include/net/netlabel.h
> index 02914b1df38b..f896b9d52ed1 100644
> --- a/include/net/netlabel.h
> +++ b/include/net/netlabel.h
> @@ -477,7 +477,7 @@ int netlbl_sock_setattr(struct sock *sk,
>  			u16 family,
>  			const struct netlbl_lsm_secattr *secattr,
>  			bool sk_locked);
> -void netlbl_sock_delattr(struct sock *sk);
> +int netlbl_sock_delattr(struct sock *sk);
>  int netlbl_sock_getattr(struct sock *sk,
>  			struct netlbl_lsm_secattr *secattr);
>  int netlbl_conn_setattr(struct sock *sk,
> @@ -485,7 +485,7 @@ int netlbl_conn_setattr(struct sock *sk,
>  			const struct netlbl_lsm_secattr *secattr);
>  int netlbl_req_setattr(struct request_sock *req,
>  		       const struct netlbl_lsm_secattr *secattr);
> -void netlbl_req_delattr(struct request_sock *req);
> +int netlbl_req_delattr(struct request_sock *req);
>  int netlbl_skbuff_setattr(struct sk_buff *skb,
>  			  u16 family,
>  			  const struct netlbl_lsm_secattr *secattr);
> @@ -626,8 +626,9 @@ static inline int netlbl_sock_setattr(struct sock *sk,
>  {
>  	return -ENOSYS;
>  }
> -static inline void netlbl_sock_delattr(struct sock *sk)
> +static inline int netlbl_sock_delattr(struct sock *sk)
>  {
> +	return 0;
>  }
>  static inline int netlbl_sock_getattr(struct sock *sk,
>  				      struct netlbl_lsm_secattr *secattr)
> @@ -645,9 +646,9 @@ static inline int netlbl_req_setattr(struct request_sock *req,
>  {
>  	return -ENOSYS;
>  }
> -static inline void netlbl_req_delattr(struct request_sock *req)
> +static inline int netlbl_req_delattr(struct request_sock *req)
>  {
> -	return;
> +	return 0;
>  }
>  static inline int netlbl_skbuff_setattr(struct sk_buff *skb,
>  				      u16 family,
> diff --git a/net/ipv4/cipso_ipv4.c b/net/ipv4/cipso_ipv4.c
> index e65a3f29d2ca..ee0c72fbc877 100644
> --- a/net/ipv4/cipso_ipv4.c
> +++ b/net/ipv4/cipso_ipv4.c
> @@ -2004,11 +2004,16 @@ static int cipso_v4_delopt(struct ip_options_rcu __rcu **opt_ptr)
>  	if (!opt || opt->opt.cipso == 0)
>  		return 0;
>  	if (opt->opt.srr || opt->opt.rr || opt->opt.ts || opt->opt.router_alert) {
> +		struct ip_options_rcu *old_opt = opt;
>  		u8 cipso_len;
>  		u8 cipso_off;
>  		unsigned char *cipso_ptr;
>  		int optlen_new;
>  
> +		/* ip_options_rcu must not be modified in place, use a copy */
> +		opt = kmemdup(opt, sizeof(*opt) + opt->opt.optlen, GFP_ATOMIC);
> +		if (!opt)
> +			return -ENOMEM;
>  		cipso_off = opt->opt.cipso - sizeof(struct iphdr);
>  		cipso_ptr = &opt->opt.__data[cipso_off];
>  		cipso_len = cipso_ptr[1];
> @@ -2036,6 +2041,8 @@ static int cipso_v4_delopt(struct ip_options_rcu __rcu **opt_ptr)
>  
>  		memset(&opt->opt.__data[optlen_new], IPOPT_END,
>  		       opt->opt.optlen - optlen_new);
> +		rcu_assign_pointer(*opt_ptr, opt);
> +		kfree_rcu(old_opt, rcu);
>  	} else {
>  		/* only the cipso option was present on the socket so we can
>  		 * remove the entire option struct */
> @@ -2055,7 +2062,7 @@ static int cipso_v4_delopt(struct ip_options_rcu __rcu **opt_ptr)
>   * Removes the CIPSO option from a socket, if present.
>   *
>   */
> -void cipso_v4_sock_delattr(struct sock *sk)
> +int cipso_v4_sock_delattr(struct sock *sk)
>  {
>  	struct inet_sock *sk_inet;
>  	int hdr_delta;
> @@ -2068,6 +2075,7 @@ void cipso_v4_sock_delattr(struct sock *sk)
>  		sk_conn->icsk_ext_hdr_len -= hdr_delta;
>  		sk_conn->icsk_sync_mss(sk, sk_conn->icsk_pmtu_cookie);
>  	}
> +	return (hdr_delta < 0) ? hdr_delta : 0;
>  }
>  
>  /**
> @@ -2078,9 +2086,11 @@ void cipso_v4_sock_delattr(struct sock *sk)
>   * Removes the CIPSO option from a request socket, if present.
>   *
>   */
> -void cipso_v4_req_delattr(struct request_sock *req)
> +int cipso_v4_req_delattr(struct request_sock *req)
>  {
> -	cipso_v4_delopt(&inet_rsk(req)->ireq_opt);
> +	int hdr_delta = cipso_v4_delopt(&inet_rsk(req)->ireq_opt);
> +
> +	return (hdr_delta < 0) ? hdr_delta : 0;
>  }
>  
>  /**
> diff --git a/net/netlabel/netlabel_kapi.c b/net/netlabel/netlabel_kapi.c
> index 3583fa63dd01..c0eddb6afef3 100644
> --- a/net/netlabel/netlabel_kapi.c
> +++ b/net/netlabel/netlabel_kapi.c
> @@ -1045,18 +1045,18 @@ int netlbl_sock_setattr(struct sock *sk,
>   * ensuring that @sk is locked.
>   *
>   */
> -void netlbl_sock_delattr(struct sock *sk)
> +int netlbl_sock_delattr(struct sock *sk)
>  {
>  	switch (sk->sk_family) {
>  	case AF_INET:
> -		cipso_v4_sock_delattr(sk);
> -		break;
> +		return cipso_v4_sock_delattr(sk);
>  #if IS_ENABLED(CONFIG_IPV6)
>  	case AF_INET6:
>  		calipso_sock_delattr(sk);
>  		break;
>  #endif /* IPv6 */
>  	}
> +	return 0;
>  }
>  
>  /**
> @@ -1156,8 +1156,7 @@ int netlbl_conn_setattr(struct sock *sk,
>  		case NETLBL_NLTYPE_UNLABELED:
>  			/* just delete the protocols we support for right now
>  			 * but we could remove other protocols if needed */
> -			netlbl_sock_delattr(sk);
> -			ret_val = 0;
> +			ret_val = netlbl_sock_delattr(sk);
>  			break;
>  		default:
>  			ret_val = -ENOENT;
> @@ -1185,8 +1184,7 @@ int netlbl_conn_setattr(struct sock *sk,
>  		case NETLBL_NLTYPE_UNLABELED:
>  			/* just delete the protocols we support for right now
>  			 * but we could remove other protocols if needed */
> -			netlbl_sock_delattr(sk);
> -			ret_val = 0;
> +			ret_val = netlbl_sock_delattr(sk);
>  			break;
>  		default:
>  			ret_val = -ENOENT;
> @@ -1234,8 +1232,7 @@ int netlbl_req_setattr(struct request_sock *req,
>  						       entry->cipso, secattr);
>  			break;
>  		case NETLBL_NLTYPE_UNLABELED:
> -			netlbl_req_delattr(req);
> -			ret_val = 0;
> +			ret_val = netlbl_req_delattr(req);
>  			break;
>  		default:
>  			ret_val = -ENOENT;
> @@ -1255,8 +1252,7 @@ int netlbl_req_setattr(struct request_sock *req,
>  						      entry->calipso, secattr);
>  			break;
>  		case NETLBL_NLTYPE_UNLABELED:
> -			netlbl_req_delattr(req);
> -			ret_val = 0;
> +			ret_val = netlbl_req_delattr(req);
>  			break;
>  		default:
>  			ret_val = -ENOENT;
> @@ -1280,18 +1276,18 @@ int netlbl_req_setattr(struct request_sock *req,
>  * Remove all the NetLabel labeling from @req.
>  *
>  */
> -void netlbl_req_delattr(struct request_sock *req)
> +int netlbl_req_delattr(struct request_sock *req)
>  {
>  	switch (req->rsk_ops->family) {
>  	case AF_INET:
> -		cipso_v4_req_delattr(req);
> -		break;
> +		return cipso_v4_req_delattr(req);
>  #if IS_ENABLED(CONFIG_IPV6)
>  	case AF_INET6:
>  		calipso_req_delattr(req);
>  		break;
>  #endif /* IPv6 */
>  	}
> +	return 0;
>  }
>  
>  /**
> diff --git a/security/selinux/netlabel.c b/security/selinux/netlabel.c
> index d51dfe892312..24b11d1891d8 100644
> --- a/security/selinux/netlabel.c
> +++ b/security/selinux/netlabel.c
> @@ -557,9 +557,10 @@ static int selinux_netlbl_socket_connect_helper(struct sock *sk,
>  	 * is set to AF_UNSPEC, if that is what is happening we want to reset
>  	 * the socket */
>  	if (addr->sa_family == AF_UNSPEC) {
> -		netlbl_sock_delattr(sk);
> +		rc = netlbl_sock_delattr(sk);
> +		if (rc)
> +			return rc;
>  		sksec->nlbl_state = NLBL_REQSKB;
> -		rc = 0;
>  		return rc;
>  	}
>  	secattr = selinux_netlbl_sock_genattr(sk);
> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> index 28a8e12603ce..3a4a33568297 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -2656,22 +2656,26 @@ static int smack_netlbl_add(struct sock *sk)
>   *
>   * Remove the outbound smack value from a socket
>   */
> -static void smack_netlbl_delete(struct sock *sk)
> +static int smack_netlbl_delete(struct sock *sk)
>  {
>  	struct socket_smack *ssp = smack_sock(sk);
> +	int rc;
>  
>  	/*
>  	 * Take the label off the socket if one is set.
>  	 */
>  	if (ssp->smk_state != SMK_NETLBL_LABELED)
> -		return;
> +		return 0;
>  
>  	local_bh_disable();
>  	bh_lock_sock_nested(sk);
> -	netlbl_sock_delattr(sk);
> +	rc = netlbl_sock_delattr(sk);
>  	bh_unlock_sock(sk);
>  	local_bh_enable();
> +	if (rc)
> +		return rc;
>  	ssp->smk_state = SMK_NETLBL_UNLABELED;
> +	return 0;
>  }
>  
>  /**
> @@ -2711,7 +2715,7 @@ static int smk_ipv4_check(struct sock *sk, struct sockaddr_in *sap)
>  		 * Clear the socket netlabel if it's set.
>  		 */
>  		if (!rc)
> -			smack_netlbl_delete(sk);
> +			rc = smack_netlbl_delete(sk);
>  	}
>  	rcu_read_unlock();
>  
> @@ -4548,7 +4552,7 @@ static int smack_inet_conn_request(const struct sock *sk, struct sk_buff *skb,
>  	if (hskp == NULL)
>  		rc = netlbl_req_setattr(req, &ssp->smk_out->smk_netlabel);
>  	else
> -		netlbl_req_delattr(req);
> +		rc = netlbl_req_delattr(req);
>  
>  	return rc;
>  }
>
> ---
> base-commit: a91337fb5e8e0127c8b5be87be9f3027f95b48bc
> change-id: 20261106-cipso-rcuopts-e26ffa45b392
>
> Best regards,
> --  
> Jann Horn <jannh@google.com>
>