[rlc-9/5.14.0-687.46.1.el9_8] mpls: add seqcount to protect the platform_label{,s} pair - #1611
Conversation
|
🤖 Validation Checks In Progress Workflow run: https://github.com/ctrliq/kernel-src-tree/actions/runs/35038815144 |
🔍 Interdiff Analysis
================================================================================
* DELTA DIFFERENCES - code changes that differ between the patches *
================================================================================
--- b/net/mpls/af_mpls.c
+++ b/net/mpls/af_mpls.c
@@ -38,8 +38,6 @@
#define MPLS_NEIGH_TABLE_UNSPEC (NEIGH_LINK_TABLE + 1)
-static seqcount_t mpls_platform_label_seq = SEQCNT_ZERO(mpls_platform_label_seq);
-
static int label_limit = (1 << 20) - 1;
static int ttl_max = 255;
@@ -77,32 +75,16 @@
struct nlmsghdr *nlh, struct net *net, u32 portid,
unsigned int nlm_flags);
-static struct mpls_route __rcu **mpls_platform_label_rcu(struct net *net,
- size_t *platform_labels)
-{
- struct mpls_route __rcu **platform_label;
- unsigned int sequence;
-
- do {
- sequence = read_seqcount_begin(&mpls_platform_label_seq);
- platform_label = rcu_dereference_rtnl(net->mpls.platform_label);
- *platform_labels = net->mpls.platform_labels;
- } while (read_seqcount_retry(&mpls_platform_label_seq, sequence));
-
- return platform_label;
-}
-
static struct mpls_route *mpls_route_input_rcu(struct net *net, unsigned index)
{
- struct mpls_route __rcu **platform_label;
- size_t platform_labels;
-
- platform_label = mpls_platform_label_rcu(net, &platform_labels);
+ struct mpls_route *rt = NULL;
- if (index < platform_labels)
- return rcu_dereference_rtnl(platform_label[index]);
-
- return NULL;
+ if (index < net->mpls.platform_labels) {
+ struct mpls_route __rcu **platform_label =
+ rcu_dereference_rtnl(net->mpls.platform_label);
+ rt = rcu_dereference_rtnl(platform_label[index]);
+ }
+ return rt;
}
bool mpls_output_possible(const struct net_device *dev)
@@ -2235,7 +2217,8 @@
if (index < MPLS_LABEL_FIRST_UNRESERVED)
index = MPLS_LABEL_FIRST_UNRESERVED;
- platform_label = mpls_platform_label_rcu(net, &platform_labels);
+ platform_label = rtnl_dereference(net->mpls.platform_label);
+ platform_labels = net->mpls.platform_labels;
if (filter.filter_set)
flags |= NLM_F_DUMP_FILTERED;
@@ -2622,10 +2605,10 @@
/* Update the global pointers */
local_bh_disable();
- write_seqcount_begin(&mpls_platform_label_seq);
+ write_seqcount_begin(&net->mpls.platform_label_seq);
net->mpls.platform_labels = limit;
rcu_assign_pointer(net->mpls.platform_label, labels);
- write_seqcount_end(&mpls_platform_label_seq);
+ write_seqcount_end(&net->mpls.platform_label_seq);
local_bh_enable();
rtnl_unlock();
################################################################################
! REJECTED PATCH2 HUNKS - could not be compared; manual review needed !
################################################################################
--- b/net/mpls/af_mpls.c
+++ b/net/mpls/af_mpls.c
@@ -83,6 +83,20 @@
return mpls_dereference(net, platform_label[index]);
}
+static struct mpls_route __rcu **mpls_platform_label_rcu(struct net *net, size_t *platform_labels)
+{
+ struct mpls_route __rcu **platform_label;
+ unsigned int sequence;
+
+ do {
+ sequence = read_seqcount_begin(&net->mpls.platform_label_seq);
+ platform_label = rcu_dereference(net->mpls.platform_label);
+ *platform_labels = net->mpls.platform_labels;
+ } while (read_seqcount_retry(&net->mpls.platform_label_seq, sequence));
+
+ return platform_label;
+}
+
static struct mpls_route *mpls_route_input_rcu(struct net *net, unsigned int index)
{
struct mpls_route __rcu **platform_label;
@@ -86,6 +100,9 @@
static struct mpls_route *mpls_route_input_rcu(struct net *net, unsigned int index)
{
struct mpls_route __rcu **platform_label;
+ size_t platform_labels;
+
+ platform_label = mpls_platform_label_rcu(net, &platform_labels);
if (index >= net->mpls.platform_labels)
return NULL;
@@ -87,7 +104,7 @@
{
struct mpls_route __rcu **platform_label;
- if (index >= net->mpls.platform_labels)
+ if (index >= platform_labels)
return NULL;
platform_label = rcu_dereference(net->mpls.platform_label);
@@ -90,7 +107,6 @@
if (index >= net->mpls.platform_labels)
return NULL;
- platform_label = rcu_dereference(net->mpls.platform_label);
return rcu_dereference(platform_label[index]);
}
@@ -2240,8 +2256,7 @@
if (index < MPLS_LABEL_FIRST_UNRESERVED)
index = MPLS_LABEL_FIRST_UNRESERVED;
- platform_label = rcu_dereference(net->mpls.platform_label);
- platform_labels = net->mpls.platform_labels;
+ platform_label = mpls_platform_label_rcu(net, &platform_labels);
if (filter.filter_set)
flags |= NLM_F_DUMP_FILTERED;
@@ -2732,6 +2751,8 @@
int i;
mutex_init(&net->mpls.platform_mutex);
+ seqcount_mutex_init(&net->mpls.platform_label_seq, &net->mpls.platform_mutex);
+
net->mpls.platform_labels = 0;
net->mpls.platform_label = NULL;
net->mpls.ip_ttl_propagate = 1;
================================================================================
* CONTEXT DIFFERENCES - surrounding code differences between the patches *
================================================================================
--- b/net/mpls/af_mpls.c
+++ b/net/mpls/af_mpls.c
@@ -72,13 +68,12 @@
-static struct mpls_route *mpls_route_input_rcu(struct net *net, unsigned index)
+static struct mpls_route *mpls_route_input_rcu(struct net *net, unsigned int index)
{
- struct mpls_route *rt = NULL;
+ struct mpls_route __rcu **platform_label;
- if (index < net->mpls.platform_labels) {
- struct mpls_route __rcu **platform_label =
- rcu_dereference_rtnl(net->mpls.platform_label);
- rt = rcu_dereference_rtnl(platform_label[index]);
- }
- return rt;
+ if (index >= net->mpls.platform_labels)
+ return NULL;
+
+ platform_label = rcu_dereference(net->mpls.platform_label);
+ return rcu_dereference(platform_label[index]);
}
@@ -2217,7 +2240,7 @@
if (index < MPLS_LABEL_FIRST_UNRESERVED)
index = MPLS_LABEL_FIRST_UNRESERVED;
- platform_label = rtnl_dereference(net->mpls.platform_label);
+ platform_label = rcu_dereference(net->mpls.platform_label);
platform_labels = net->mpls.platform_labels;
if (filter.filter_set)
@@ -2601,7 +2624,8 @@
}
/* Update the global pointers */
net->mpls.platform_labels = limit;
rcu_assign_pointer(net->mpls.platform_label, labels);
- rtnl_unlock();
+ mutex_unlock(&net->mpls.platform_mutex);
+
================================================================================
* ONLY IN PATCH2 - files not modified by patch1 *
================================================================================
--- a/include/net/netns/mpls.h
+++ b/include/net/netns/mpls.h
@@ -17,6 +17,7 @@ struct netns_mpls {
size_t platform_labels;
struct mpls_route __rcu * __rcu *platform_label;
struct mutex platform_mutex;
+ seqcount_mutex_t platform_label_seq;
struct ctl_table_header *ctl;
};This is an automated interdiff check for backported commits. |
|
✅ Validation checks completed successfully View full results: https://github.com/ctrliq/kernel-src-tree/actions/runs/35038815144 |
3a4843d to
e993a09
Compare
|
🤖 Validation Checks In Progress Workflow run: https://github.com/ctrliq/kernel-src-tree/actions/runs/35119525304 |
🔍 Interdiff Analysis
================================================================================
* DELTA DIFFERENCES - code changes that differ between the patches *
================================================================================
--- b/net/mpls/af_mpls.c
+++ b/net/mpls/af_mpls.c
@@ -38,10 +38,6 @@
#define MPLS_NEIGH_TABLE_UNSPEC (NEIGH_LINK_TABLE + 1)
-static DEFINE_SPINLOCK(mpls_platform_label_lock);
-static seqcount_spinlock_t mpls_platform_label_seq =
- SEQCNT_SPINLOCK_ZERO(mpls_platform_label_seq, &mpls_platform_label_lock);
-
static int label_limit = (1 << 20) - 1;
static int ttl_max = 255;
@@ -79,32 +75,16 @@
struct nlmsghdr *nlh, struct net *net, u32 portid,
unsigned int nlm_flags);
-static struct mpls_route __rcu **mpls_platform_label_rcu(struct net *net,
- size_t *platform_labels)
-{
- struct mpls_route __rcu **platform_label;
- unsigned int sequence;
-
- do {
- sequence = read_seqcount_begin(&mpls_platform_label_seq);
- platform_label = rcu_dereference_rtnl(net->mpls.platform_label);
- *platform_labels = net->mpls.platform_labels;
- } while (read_seqcount_retry(&mpls_platform_label_seq, sequence));
-
- return platform_label;
-}
-
static struct mpls_route *mpls_route_input_rcu(struct net *net, unsigned index)
{
- struct mpls_route __rcu **platform_label;
- size_t platform_labels;
-
- platform_label = mpls_platform_label_rcu(net, &platform_labels);
+ struct mpls_route *rt = NULL;
- if (index < platform_labels)
- return rcu_dereference_rtnl(platform_label[index]);
-
- return NULL;
+ if (index < net->mpls.platform_labels) {
+ struct mpls_route __rcu **platform_label =
+ rcu_dereference_rtnl(net->mpls.platform_label);
+ rt = rcu_dereference_rtnl(platform_label[index]);
+ }
+ return rt;
}
bool mpls_output_possible(const struct net_device *dev)
@@ -2237,7 +2217,8 @@
if (index < MPLS_LABEL_FIRST_UNRESERVED)
index = MPLS_LABEL_FIRST_UNRESERVED;
- platform_label = mpls_platform_label_rcu(net, &platform_labels);
+ platform_label = rtnl_dereference(net->mpls.platform_label);
+ platform_labels = net->mpls.platform_labels;
if (filter.filter_set)
flags |= NLM_F_DUMP_FILTERED;
@@ -2623,12 +2604,12 @@
}
/* Update the global pointers */
- spin_lock_bh(&mpls_platform_label_lock);
- write_seqcount_begin(&mpls_platform_label_seq);
+ local_bh_disable();
+ write_seqcount_begin(&net->mpls.platform_label_seq);
net->mpls.platform_labels = limit;
rcu_assign_pointer(net->mpls.platform_label, labels);
- write_seqcount_end(&mpls_platform_label_seq);
- spin_unlock_bh(&mpls_platform_label_lock);
+ write_seqcount_end(&net->mpls.platform_label_seq);
+ local_bh_enable();
rtnl_unlock();
################################################################################
! REJECTED PATCH2 HUNKS - could not be compared; manual review needed !
################################################################################
--- b/net/mpls/af_mpls.c
+++ b/net/mpls/af_mpls.c
@@ -83,6 +83,20 @@
return mpls_dereference(net, platform_label[index]);
}
+static struct mpls_route __rcu **mpls_platform_label_rcu(struct net *net, size_t *platform_labels)
+{
+ struct mpls_route __rcu **platform_label;
+ unsigned int sequence;
+
+ do {
+ sequence = read_seqcount_begin(&net->mpls.platform_label_seq);
+ platform_label = rcu_dereference(net->mpls.platform_label);
+ *platform_labels = net->mpls.platform_labels;
+ } while (read_seqcount_retry(&net->mpls.platform_label_seq, sequence));
+
+ return platform_label;
+}
+
static struct mpls_route *mpls_route_input_rcu(struct net *net, unsigned int index)
{
struct mpls_route __rcu **platform_label;
@@ -86,6 +100,9 @@
static struct mpls_route *mpls_route_input_rcu(struct net *net, unsigned int index)
{
struct mpls_route __rcu **platform_label;
+ size_t platform_labels;
+
+ platform_label = mpls_platform_label_rcu(net, &platform_labels);
if (index >= net->mpls.platform_labels)
return NULL;
@@ -87,7 +104,7 @@
{
struct mpls_route __rcu **platform_label;
- if (index >= net->mpls.platform_labels)
+ if (index >= platform_labels)
return NULL;
platform_label = rcu_dereference(net->mpls.platform_label);
@@ -90,7 +107,6 @@
if (index >= net->mpls.platform_labels)
return NULL;
- platform_label = rcu_dereference(net->mpls.platform_label);
return rcu_dereference(platform_label[index]);
}
@@ -2240,8 +2256,7 @@
if (index < MPLS_LABEL_FIRST_UNRESERVED)
index = MPLS_LABEL_FIRST_UNRESERVED;
- platform_label = rcu_dereference(net->mpls.platform_label);
- platform_labels = net->mpls.platform_labels;
+ platform_label = mpls_platform_label_rcu(net, &platform_labels);
if (filter.filter_set)
flags |= NLM_F_DUMP_FILTERED;
@@ -2732,6 +2751,8 @@
int i;
mutex_init(&net->mpls.platform_mutex);
+ seqcount_mutex_init(&net->mpls.platform_label_seq, &net->mpls.platform_mutex);
+
net->mpls.platform_labels = 0;
net->mpls.platform_label = NULL;
net->mpls.ip_ttl_propagate = 1;
================================================================================
* CONTEXT DIFFERENCES - surrounding code differences between the patches *
================================================================================
--- b/net/mpls/af_mpls.c
+++ b/net/mpls/af_mpls.c
@@ -72,13 +68,12 @@
-static struct mpls_route *mpls_route_input_rcu(struct net *net, unsigned index)
+static struct mpls_route *mpls_route_input_rcu(struct net *net, unsigned int index)
{
- struct mpls_route *rt = NULL;
+ struct mpls_route __rcu **platform_label;
- if (index < net->mpls.platform_labels) {
- struct mpls_route __rcu **platform_label =
- rcu_dereference_rtnl(net->mpls.platform_label);
- rt = rcu_dereference_rtnl(platform_label[index]);
- }
- return rt;
+ if (index >= net->mpls.platform_labels)
+ return NULL;
+
+ platform_label = rcu_dereference(net->mpls.platform_label);
+ return rcu_dereference(platform_label[index]);
}
@@ -2217,7 +2240,7 @@
if (index < MPLS_LABEL_FIRST_UNRESERVED)
index = MPLS_LABEL_FIRST_UNRESERVED;
- platform_label = rtnl_dereference(net->mpls.platform_label);
+ platform_label = rcu_dereference(net->mpls.platform_label);
platform_labels = net->mpls.platform_labels;
if (filter.filter_set)
@@ -2601,7 +2624,8 @@
}
/* Update the global pointers */
net->mpls.platform_labels = limit;
rcu_assign_pointer(net->mpls.platform_label, labels);
- rtnl_unlock();
+ mutex_unlock(&net->mpls.platform_mutex);
+
================================================================================
* ONLY IN PATCH2 - files not modified by patch1 *
================================================================================
--- a/include/net/netns/mpls.h
+++ b/include/net/netns/mpls.h
@@ -17,6 +17,7 @@ struct netns_mpls {
size_t platform_labels;
struct mpls_route __rcu * __rcu *platform_label;
struct mutex platform_mutex;
+ seqcount_mutex_t platform_label_seq;
struct ctl_table_header *ctl;
};This is an automated interdiff check for backported commits. |
|
✅ Validation checks completed successfully View full results: https://github.com/ctrliq/kernel-src-tree/actions/runs/35119525304 |
kerneltoast
left a comment
There was a problem hiding this comment.
Introducing mpls_platform_label_lock is excessive when it's just for preventing a livelock on PREEMPT_RT.
You can remove the lock and use a plain seqcount, and then use preempt_disable_nested() to prevent a possible livelock on PREEMPT_RT.
Apply the following diff:
--- a/net/mpls/af_mpls.c
+++ b/net/mpls/af_mpls.c
@@ -38,9 +38,8 @@
#define MPLS_NEIGH_TABLE_UNSPEC (NEIGH_LINK_TABLE + 1)
-static DEFINE_SPINLOCK(mpls_platform_label_lock);
-static seqcount_spinlock_t mpls_platform_label_seq =
- SEQCNT_SPINLOCK_ZERO(mpls_platform_label_seq, &mpls_platform_label_lock);
+static seqcount_t mpls_platform_label_seq =
+ SEQCNT_ZERO(mpls_platform_label_seq);
static int label_limit = (1 << 20) - 1;
static int ttl_max = 255;
@@ -2623,12 +2622,14 @@ static int resize_platform_label_table(struct net *net, size_t limit)
}
/* Update the global pointers */
- spin_lock_bh(&mpls_platform_label_lock);
+ local_bh_disable();
+ preempt_disable_nested();
write_seqcount_begin(&mpls_platform_label_seq);
net->mpls.platform_labels = limit;
rcu_assign_pointer(net->mpls.platform_label, labels);
write_seqcount_end(&mpls_platform_label_seq);
- spin_unlock_bh(&mpls_platform_label_lock);
+ preempt_enable_nested();
+ local_bh_enable();
rtnl_unlock();
Some side comments:
- A global seqcount lock means the reader side in
mpls_platform_label_rcu()will retry when an unrelatednetis updated. Since the critical section is so small, it probably doesn't matter in the grand scheme of performace (especially since a global mutex lock (rtnl_lock()) is used on the writer side). - Upstream sure chose a convoluted solution when they could've just stored the size of the platform_label array when they allocate that array. Then the single platform_label pointer wouldn't need an outside variable to track the size of the array. Le sigh.
e993a09 to
517632b
Compare
Good call. I've updated the branch with your suggestion. Thanks! |
|
🤖 Validation Checks In Progress Workflow run: https://github.com/ctrliq/kernel-src-tree/actions/runs/35245376972 |
🔍 Interdiff Analysis
================================================================================
* DELTA DIFFERENCES - code changes that differ between the patches *
================================================================================
--- b/net/mpls/af_mpls.c
+++ b/net/mpls/af_mpls.c
@@ -38,8 +38,6 @@
#define MPLS_NEIGH_TABLE_UNSPEC (NEIGH_LINK_TABLE + 1)
-static seqcount_t mpls_platform_label_seq = SEQCNT_ZERO(mpls_platform_label_seq);
-
static int label_limit = (1 << 20) - 1;
static int ttl_max = 255;
@@ -77,32 +75,16 @@
struct nlmsghdr *nlh, struct net *net, u32 portid,
unsigned int nlm_flags);
-static struct mpls_route __rcu **mpls_platform_label_rcu(struct net *net,
- size_t *platform_labels)
-{
- struct mpls_route __rcu **platform_label;
- unsigned int sequence;
-
- do {
- sequence = read_seqcount_begin(&mpls_platform_label_seq);
- platform_label = rcu_dereference_rtnl(net->mpls.platform_label);
- *platform_labels = net->mpls.platform_labels;
- } while (read_seqcount_retry(&mpls_platform_label_seq, sequence));
-
- return platform_label;
-}
-
static struct mpls_route *mpls_route_input_rcu(struct net *net, unsigned index)
{
- struct mpls_route __rcu **platform_label;
- size_t platform_labels;
-
- platform_label = mpls_platform_label_rcu(net, &platform_labels);
+ struct mpls_route *rt = NULL;
- if (index < platform_labels)
- return rcu_dereference_rtnl(platform_label[index]);
-
- return NULL;
+ if (index < net->mpls.platform_labels) {
+ struct mpls_route __rcu **platform_label =
+ rcu_dereference_rtnl(net->mpls.platform_label);
+ rt = rcu_dereference_rtnl(platform_label[index]);
+ }
+ return rt;
}
bool mpls_output_possible(const struct net_device *dev)
@@ -2235,7 +2217,8 @@
if (index < MPLS_LABEL_FIRST_UNRESERVED)
index = MPLS_LABEL_FIRST_UNRESERVED;
- platform_label = mpls_platform_label_rcu(net, &platform_labels);
+ platform_label = rtnl_dereference(net->mpls.platform_label);
+ platform_labels = net->mpls.platform_labels;
if (filter.filter_set)
flags |= NLM_F_DUMP_FILTERED;
@@ -2622,12 +2605,10 @@
/* Update the global pointers */
local_bh_disable();
- preempt_disable_nested();
- write_seqcount_begin(&mpls_platform_label_seq);
+ write_seqcount_begin(&net->mpls.platform_label_seq);
net->mpls.platform_labels = limit;
rcu_assign_pointer(net->mpls.platform_label, labels);
- write_seqcount_end(&mpls_platform_label_seq);
- preempt_enable_nested();
+ write_seqcount_end(&net->mpls.platform_label_seq);
local_bh_enable();
rtnl_unlock();
################################################################################
! REJECTED PATCH2 HUNKS - could not be compared; manual review needed !
################################################################################
--- b/net/mpls/af_mpls.c
+++ b/net/mpls/af_mpls.c
@@ -83,6 +83,20 @@
return mpls_dereference(net, platform_label[index]);
}
+static struct mpls_route __rcu **mpls_platform_label_rcu(struct net *net, size_t *platform_labels)
+{
+ struct mpls_route __rcu **platform_label;
+ unsigned int sequence;
+
+ do {
+ sequence = read_seqcount_begin(&net->mpls.platform_label_seq);
+ platform_label = rcu_dereference(net->mpls.platform_label);
+ *platform_labels = net->mpls.platform_labels;
+ } while (read_seqcount_retry(&net->mpls.platform_label_seq, sequence));
+
+ return platform_label;
+}
+
static struct mpls_route *mpls_route_input_rcu(struct net *net, unsigned int index)
{
struct mpls_route __rcu **platform_label;
@@ -86,6 +100,9 @@
static struct mpls_route *mpls_route_input_rcu(struct net *net, unsigned int index)
{
struct mpls_route __rcu **platform_label;
+ size_t platform_labels;
+
+ platform_label = mpls_platform_label_rcu(net, &platform_labels);
if (index >= net->mpls.platform_labels)
return NULL;
@@ -87,7 +104,7 @@
{
struct mpls_route __rcu **platform_label;
- if (index >= net->mpls.platform_labels)
+ if (index >= platform_labels)
return NULL;
platform_label = rcu_dereference(net->mpls.platform_label);
@@ -90,7 +107,6 @@
if (index >= net->mpls.platform_labels)
return NULL;
- platform_label = rcu_dereference(net->mpls.platform_label);
return rcu_dereference(platform_label[index]);
}
@@ -2240,8 +2256,7 @@
if (index < MPLS_LABEL_FIRST_UNRESERVED)
index = MPLS_LABEL_FIRST_UNRESERVED;
- platform_label = rcu_dereference(net->mpls.platform_label);
- platform_labels = net->mpls.platform_labels;
+ platform_label = mpls_platform_label_rcu(net, &platform_labels);
if (filter.filter_set)
flags |= NLM_F_DUMP_FILTERED;
@@ -2732,6 +2751,8 @@
int i;
mutex_init(&net->mpls.platform_mutex);
+ seqcount_mutex_init(&net->mpls.platform_label_seq, &net->mpls.platform_mutex);
+
net->mpls.platform_labels = 0;
net->mpls.platform_label = NULL;
net->mpls.ip_ttl_propagate = 1;
================================================================================
* CONTEXT DIFFERENCES - surrounding code differences between the patches *
================================================================================
--- b/net/mpls/af_mpls.c
+++ b/net/mpls/af_mpls.c
@@ -72,13 +68,12 @@
-static struct mpls_route *mpls_route_input_rcu(struct net *net, unsigned index)
+static struct mpls_route *mpls_route_input_rcu(struct net *net, unsigned int index)
{
- struct mpls_route *rt = NULL;
+ struct mpls_route __rcu **platform_label;
- if (index < net->mpls.platform_labels) {
- struct mpls_route __rcu **platform_label =
- rcu_dereference_rtnl(net->mpls.platform_label);
- rt = rcu_dereference_rtnl(platform_label[index]);
- }
- return rt;
+ if (index >= net->mpls.platform_labels)
+ return NULL;
+
+ platform_label = rcu_dereference(net->mpls.platform_label);
+ return rcu_dereference(platform_label[index]);
}
@@ -2217,7 +2240,7 @@
if (index < MPLS_LABEL_FIRST_UNRESERVED)
index = MPLS_LABEL_FIRST_UNRESERVED;
- platform_label = rtnl_dereference(net->mpls.platform_label);
+ platform_label = rcu_dereference(net->mpls.platform_label);
platform_labels = net->mpls.platform_labels;
if (filter.filter_set)
@@ -2601,7 +2624,8 @@
}
/* Update the global pointers */
net->mpls.platform_labels = limit;
rcu_assign_pointer(net->mpls.platform_label, labels);
- rtnl_unlock();
+ mutex_unlock(&net->mpls.platform_mutex);
+
================================================================================
* ONLY IN PATCH2 - files not modified by patch1 *
================================================================================
--- a/include/net/netns/mpls.h
+++ b/include/net/netns/mpls.h
@@ -17,6 +17,7 @@ struct netns_mpls {
size_t platform_labels;
struct mpls_route __rcu * __rcu *platform_label;
struct mutex platform_mutex;
+ seqcount_mutex_t platform_label_seq;
struct ctl_table_header *ctl;
};This is an automated interdiff check for backported commits. |
|
✅ Validation checks completed successfully View full results: https://github.com/ctrliq/kernel-src-tree/actions/runs/35245376972 |
|
PoC results: |
cve CVE-2026-43042 commit-author Sabrina Dubroca <sd@queasysnail.net> commit 629ec78 upstream-diff Uses a file-scope seqcount_t instead of upstream's per-netns seqcount_mutex_t to avoid a kABI-breaking struct change. Write side uses local_bh_disable() with preempt_disable_nested() for RT safety. Uses rcu_dereference_rtnl() instead of upstream's plain rcu_dereference() to stay lockdep-clean under RTNL. mpls_dump_routes() still runs under RTNL on this tree; the seqcount there is extra hardening. The RCU-protected codepaths (mpls_forward, mpls_dump_routes) can have an inconsistent view of platform_labels vs platform_label in case of a concurrent resize (resize_platform_label_table, under platform_mutex). This can lead to OOB accesses. This patch adds a seqcount, so that we get a consistent snapshot. Note that mpls_label_ok is also susceptible to this, so the check against RTA_DST in rtm_to_route_config, done outside platform_mutex, is not sufficient. This value gets passed to mpls_label_ok once more in both mpls_route_add and mpls_route_del, so there is no issue, but that additional check must not be removed. Reported-by: Yuan Tan <tanyuan98@outlook.com> Reported-by: Yifan Wu <yifanwucs@gmail.com> Reported-by: Juefei Pu <tomapufckgml@gmail.com> Reported-by: Xin Liu <bird@lzu.edu.cn> Fixes: 7720c01 ("mpls: Add a sysctl to control the size of the mpls label table") Fixes: dde1b38 ("mpls: Convert mpls_dump_routes() to RCU.") Signed-off-by: Sabrina Dubroca <sd@queasysnail.net> Link: https://patch.msgid.link/cd8fca15e3eb7e212b094064cd83652e20fd9d31.1774284088.git.sd@queasysnail.net Signed-off-by: Jakub Kicinski <kuba@kernel.org> (cherry picked from commit 629ec78) Signed-off-by: Brett Mastbergen <bmastbergen@ciq.com>
021f402
517632b to
021f402
Compare
|
🤖 Validation Checks In Progress Workflow run: https://github.com/ctrliq/kernel-src-tree/actions/runs/35350542499 |
🔍 Interdiff Analysis
================================================================================
* DELTA DIFFERENCES - code changes that differ between the patches *
================================================================================
--- b/net/mpls/af_mpls.c
+++ b/net/mpls/af_mpls.c
@@ -38,8 +38,6 @@
#define MPLS_NEIGH_TABLE_UNSPEC (NEIGH_LINK_TABLE + 1)
-static seqcount_t mpls_platform_label_seq = SEQCNT_ZERO(mpls_platform_label_seq);
-
static int label_limit = (1 << 20) - 1;
static int ttl_max = 255;
@@ -77,31 +75,16 @@
struct nlmsghdr *nlh, struct net *net, u32 portid,
unsigned int nlm_flags);
-static struct mpls_route __rcu **mpls_platform_label_rcu(struct net *net, size_t *platform_labels)
-{
- struct mpls_route __rcu **platform_label;
- unsigned int sequence;
-
- do {
- sequence = read_seqcount_begin(&mpls_platform_label_seq);
- platform_label = rcu_dereference_rtnl(net->mpls.platform_label);
- *platform_labels = net->mpls.platform_labels;
- } while (read_seqcount_retry(&mpls_platform_label_seq, sequence));
-
- return platform_label;
-}
-
static struct mpls_route *mpls_route_input_rcu(struct net *net, unsigned index)
{
- struct mpls_route __rcu **platform_label;
- size_t platform_labels;
-
- platform_label = mpls_platform_label_rcu(net, &platform_labels);
+ struct mpls_route *rt = NULL;
- if (index < platform_labels)
- return rcu_dereference_rtnl(platform_label[index]);
-
- return NULL;
+ if (index < net->mpls.platform_labels) {
+ struct mpls_route __rcu **platform_label =
+ rcu_dereference_rtnl(net->mpls.platform_label);
+ rt = rcu_dereference_rtnl(platform_label[index]);
+ }
+ return rt;
}
bool mpls_output_possible(const struct net_device *dev)
@@ -2234,7 +2217,8 @@
if (index < MPLS_LABEL_FIRST_UNRESERVED)
index = MPLS_LABEL_FIRST_UNRESERVED;
- platform_label = mpls_platform_label_rcu(net, &platform_labels);
+ platform_label = rtnl_dereference(net->mpls.platform_label);
+ platform_labels = net->mpls.platform_labels;
if (filter.filter_set)
flags |= NLM_F_DUMP_FILTERED;
@@ -2621,12 +2605,10 @@
/* Update the global pointers */
local_bh_disable();
- preempt_disable_nested();
- write_seqcount_begin(&mpls_platform_label_seq);
+ write_seqcount_begin(&net->mpls.platform_label_seq);
net->mpls.platform_labels = limit;
rcu_assign_pointer(net->mpls.platform_label, labels);
- write_seqcount_end(&mpls_platform_label_seq);
- preempt_enable_nested();
+ write_seqcount_end(&net->mpls.platform_label_seq);
local_bh_enable();
rtnl_unlock();
################################################################################
! REJECTED PATCH2 HUNKS - could not be compared; manual review needed !
################################################################################
--- b/net/mpls/af_mpls.c
+++ b/net/mpls/af_mpls.c
@@ -83,6 +83,20 @@
return mpls_dereference(net, platform_label[index]);
}
+static struct mpls_route __rcu **mpls_platform_label_rcu(struct net *net, size_t *platform_labels)
+{
+ struct mpls_route __rcu **platform_label;
+ unsigned int sequence;
+
+ do {
+ sequence = read_seqcount_begin(&net->mpls.platform_label_seq);
+ platform_label = rcu_dereference(net->mpls.platform_label);
+ *platform_labels = net->mpls.platform_labels;
+ } while (read_seqcount_retry(&net->mpls.platform_label_seq, sequence));
+
+ return platform_label;
+}
+
static struct mpls_route *mpls_route_input_rcu(struct net *net, unsigned int index)
{
struct mpls_route __rcu **platform_label;
@@ -86,6 +100,9 @@
static struct mpls_route *mpls_route_input_rcu(struct net *net, unsigned int index)
{
struct mpls_route __rcu **platform_label;
+ size_t platform_labels;
+
+ platform_label = mpls_platform_label_rcu(net, &platform_labels);
if (index >= net->mpls.platform_labels)
return NULL;
@@ -87,7 +104,7 @@
{
struct mpls_route __rcu **platform_label;
- if (index >= net->mpls.platform_labels)
+ if (index >= platform_labels)
return NULL;
platform_label = rcu_dereference(net->mpls.platform_label);
@@ -90,7 +107,6 @@
if (index >= net->mpls.platform_labels)
return NULL;
- platform_label = rcu_dereference(net->mpls.platform_label);
return rcu_dereference(platform_label[index]);
}
@@ -2240,8 +2256,7 @@
if (index < MPLS_LABEL_FIRST_UNRESERVED)
index = MPLS_LABEL_FIRST_UNRESERVED;
- platform_label = rcu_dereference(net->mpls.platform_label);
- platform_labels = net->mpls.platform_labels;
+ platform_label = mpls_platform_label_rcu(net, &platform_labels);
if (filter.filter_set)
flags |= NLM_F_DUMP_FILTERED;
@@ -2732,6 +2751,8 @@
int i;
mutex_init(&net->mpls.platform_mutex);
+ seqcount_mutex_init(&net->mpls.platform_label_seq, &net->mpls.platform_mutex);
+
net->mpls.platform_labels = 0;
net->mpls.platform_label = NULL;
net->mpls.ip_ttl_propagate = 1;
================================================================================
* CONTEXT DIFFERENCES - surrounding code differences between the patches *
================================================================================
--- b/net/mpls/af_mpls.c
+++ b/net/mpls/af_mpls.c
@@ -72,13 +68,12 @@
-static struct mpls_route *mpls_route_input_rcu(struct net *net, unsigned index)
+static struct mpls_route *mpls_route_input_rcu(struct net *net, unsigned int index)
{
- struct mpls_route *rt = NULL;
+ struct mpls_route __rcu **platform_label;
- if (index < net->mpls.platform_labels) {
- struct mpls_route __rcu **platform_label =
- rcu_dereference_rtnl(net->mpls.platform_label);
- rt = rcu_dereference_rtnl(platform_label[index]);
- }
- return rt;
+ if (index >= net->mpls.platform_labels)
+ return NULL;
+
+ platform_label = rcu_dereference(net->mpls.platform_label);
+ return rcu_dereference(platform_label[index]);
}
@@ -2217,7 +2240,7 @@
if (index < MPLS_LABEL_FIRST_UNRESERVED)
index = MPLS_LABEL_FIRST_UNRESERVED;
- platform_label = rtnl_dereference(net->mpls.platform_label);
+ platform_label = rcu_dereference(net->mpls.platform_label);
platform_labels = net->mpls.platform_labels;
if (filter.filter_set)
@@ -2601,7 +2624,8 @@
}
/* Update the global pointers */
net->mpls.platform_labels = limit;
rcu_assign_pointer(net->mpls.platform_label, labels);
- rtnl_unlock();
+ mutex_unlock(&net->mpls.platform_mutex);
+
================================================================================
* ONLY IN PATCH2 - files not modified by patch1 *
================================================================================
--- a/include/net/netns/mpls.h
+++ b/include/net/netns/mpls.h
@@ -17,6 +17,7 @@ struct netns_mpls {
size_t platform_labels;
struct mpls_route __rcu * __rcu *platform_label;
struct mutex platform_mutex;
+ seqcount_mutex_t platform_label_seq;
struct ctl_table_header *ctl;
};This is an automated interdiff check for backported commits. |
|
✅ Validation checks completed successfully View full results: https://github.com/ctrliq/kernel-src-tree/actions/runs/35350542499 |
PlaidCat
left a comment
There was a problem hiding this comment.
![]()
@kerneltoast I think your block has been addressed right?
Summary
This PR has been automatically created after successful completion of all CI stages.
Commit Message(s)
Test Results
✅ Build Stage
✅ Boot Verification
✅ Kernel Selftests
✅ LTP Results
aarch64 newly passing:
🤖 This PR was automatically generated by GitHub Actions
Run ID: 35349704838