[PATCH net] cipso: stop modifying live ip_options_rcu
From: Jann Horn
Date: Fri Nov 06 2026 - 16:42:57 EST
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@xxxxxxxxxxxxxxx
Signed-off-by: Jann Horn <jannh@xxxxxxxxxx>
---
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@xxxxxxxxxx>