Skip to content

Commit afbd961

Browse files
Julian Anastasovummakynes
authored andcommitted
ipvs: fixes for the new ip_vs_status info
Sashiko reports some problems for the recently added /proc/net/ip_vs_status: * ip_vs_status_show() as a table reader may run long after the conn_tab and svc_table table are released. While ip_vs_conn_flush() properly changes the conn_tab_changes counter when conn_tab is removed, ip_vs_del_service() and ip_vs_flush() were missing such change for the svc_table_changes counter. As result, readers like ip_vs_dst_event() and ip_vs_status_show() may continue to use a freed table after a cond_resched_rcu() call. * While counting the buckets in ip_vs_status_show() make sure we traverse only the needed number of entries in the chain. This also prevents possible overflow of the 'count' variable. * Add check for 'loops' to prevent infinite loops while restarting the traversal on table change. * While IP_VS_CONN_TAB_MAX_BITS is 20 on 32-bit platforms and there is no risk to overflow when multiplying the number of conn_tab buckets to 100, prefer the div_u64() helper to make the following dividing safer. * Use 0440 permissions for ip_vs_status to restrict the info only to root due to the exported information for hash distribution. Link: https://sashiko.dev/#/patchset/20260410112352.23599-1-fw%40strlen.de Fixes: 9a9ccef ("ipvs: add ip_vs_status info") Signed-off-by: Julian Anastasov <ja@ssi.bg> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
1 parent bd3a479 commit afbd961

1 file changed

Lines changed: 36 additions & 15 deletions

File tree

net/netfilter/ipvs/ip_vs_ctl.c

Lines changed: 36 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2032,6 +2032,9 @@ static int ip_vs_del_service(struct ip_vs_service *svc)
20322032
cancel_delayed_work_sync(&ipvs->svc_resize_work);
20332033
if (t) {
20342034
rcu_assign_pointer(ipvs->svc_table, NULL);
2035+
/* Inform readers that table is removed */
2036+
smp_mb__before_atomic();
2037+
atomic_inc(&ipvs->svc_table_changes);
20352038
while (1) {
20362039
p = rcu_dereference_protected(t->new_tbl, 1);
20372040
call_rcu(&t->rcu_head, ip_vs_rht_rcu_free);
@@ -2078,6 +2081,9 @@ static int ip_vs_flush(struct netns_ipvs *ipvs, bool cleanup)
20782081
t = rcu_dereference_protected(ipvs->svc_table, 1);
20792082
if (t) {
20802083
rcu_assign_pointer(ipvs->svc_table, NULL);
2084+
/* Inform readers that table is removed */
2085+
smp_mb__before_atomic();
2086+
atomic_inc(&ipvs->svc_table_changes);
20812087
while (1) {
20822088
p = rcu_dereference_protected(t->new_tbl, 1);
20832089
call_rcu(&t->rcu_head, ip_vs_rht_rcu_free);
@@ -3004,7 +3010,8 @@ static int ip_vs_status_show(struct seq_file *seq, void *v)
30043010
int old_gen, new_gen;
30053011
u32 counts[8];
30063012
u32 bucket;
3007-
int count;
3013+
u32 count;
3014+
int loops;
30083015
u32 sum1;
30093016
u32 sum;
30103017
int i;
@@ -3020,6 +3027,7 @@ static int ip_vs_status_show(struct seq_file *seq, void *v)
30203027
if (!atomic_read(&ipvs->conn_count))
30213028
goto after_conns;
30223029
old_gen = atomic_read(&ipvs->conn_tab_changes);
3030+
loops = 0;
30233031

30243032
repeat_conn:
30253033
smp_rmb(); /* ipvs->conn_tab and conn_tab_changes */
@@ -3032,8 +3040,11 @@ static int ip_vs_status_show(struct seq_file *seq, void *v)
30323040
resched_score++;
30333041
ip_vs_rht_walk_bucket_rcu(t, bucket, head) {
30343042
count = 0;
3035-
hlist_bl_for_each_entry_rcu(hn, e, head, node)
3043+
hlist_bl_for_each_entry_rcu(hn, e, head, node) {
30363044
count++;
3045+
if (count >= ARRAY_SIZE(counts) - 1)
3046+
break;
3047+
}
30373048
}
30383049
resched_score += count;
30393050
if (resched_score >= 100) {
@@ -3042,37 +3053,41 @@ static int ip_vs_status_show(struct seq_file *seq, void *v)
30423053
new_gen = atomic_read(&ipvs->conn_tab_changes);
30433054
/* New table installed ? */
30443055
if (old_gen != new_gen) {
3056+
/* Too many changes? */
3057+
if (++loops >= 5)
3058+
goto after_conns;
30453059
old_gen = new_gen;
30463060
goto repeat_conn;
30473061
}
30483062
}
3049-
counts[min(count, (int)ARRAY_SIZE(counts) - 1)]++;
3063+
counts[count]++;
30503064
}
30513065
}
30523066
for (sum = 0, i = 0; i < ARRAY_SIZE(counts); i++)
30533067
sum += counts[i];
30543068
sum1 = sum - counts[0];
3055-
seq_printf(seq, "Conn buckets empty:\t%u (%lu%%)\n",
3056-
counts[0], (unsigned long)counts[0] * 100 / max(sum, 1U));
3069+
seq_printf(seq, "Conn buckets empty:\t%u (%llu%%)\n",
3070+
counts[0], div_u64((u64)counts[0] * 100U, max(sum, 1U)));
30573071
for (i = 1; i < ARRAY_SIZE(counts); i++) {
30583072
if (!counts[i])
30593073
continue;
3060-
seq_printf(seq, "Conn buckets len-%d:\t%u (%lu%%)\n",
3074+
seq_printf(seq, "Conn buckets len-%d:\t%u (%llu%%)\n",
30613075
i, counts[i],
3062-
(unsigned long)counts[i] * 100 / max(sum1, 1U));
3076+
div_u64((u64)counts[i] * 100U, max(sum1, 1U)));
30633077
}
30643078

30653079
after_conns:
30663080
t = rcu_dereference(ipvs->svc_table);
30673081

30683082
count = ip_vs_get_num_services(ipvs);
3069-
seq_printf(seq, "Services:\t%d\n", count);
3083+
seq_printf(seq, "Services:\t%u\n", count);
30703084
seq_printf(seq, "Service buckets:\t%d (%d bits, lfactor %d)\n",
30713085
t ? t->size : 0, t ? t->bits : 0, t ? t->lfactor : 0);
30723086

30733087
if (!count)
30743088
goto after_svc;
30753089
old_gen = atomic_read(&ipvs->svc_table_changes);
3090+
loops = 0;
30763091

30773092
repeat_svc:
30783093
smp_rmb(); /* ipvs->svc_table and svc_table_changes */
@@ -3086,8 +3101,11 @@ static int ip_vs_status_show(struct seq_file *seq, void *v)
30863101
ip_vs_rht_walk_bucket_rcu(t, bucket, head) {
30873102
count = 0;
30883103
hlist_bl_for_each_entry_rcu(svc, e, head,
3089-
s_list)
3104+
s_list) {
30903105
count++;
3106+
if (count >= ARRAY_SIZE(counts) - 1)
3107+
break;
3108+
}
30913109
}
30923110
resched_score += count;
30933111
if (resched_score >= 100) {
@@ -3096,24 +3114,27 @@ static int ip_vs_status_show(struct seq_file *seq, void *v)
30963114
new_gen = atomic_read(&ipvs->svc_table_changes);
30973115
/* New table installed ? */
30983116
if (old_gen != new_gen) {
3117+
/* Too many changes? */
3118+
if (++loops >= 5)
3119+
goto after_svc;
30993120
old_gen = new_gen;
31003121
goto repeat_svc;
31013122
}
31023123
}
3103-
counts[min(count, (int)ARRAY_SIZE(counts) - 1)]++;
3124+
counts[count]++;
31043125
}
31053126
}
31063127
for (sum = 0, i = 0; i < ARRAY_SIZE(counts); i++)
31073128
sum += counts[i];
31083129
sum1 = sum - counts[0];
3109-
seq_printf(seq, "Service buckets empty:\t%u (%lu%%)\n",
3110-
counts[0], (unsigned long)counts[0] * 100 / max(sum, 1U));
3130+
seq_printf(seq, "Service buckets empty:\t%u (%llu%%)\n",
3131+
counts[0], div_u64((u64)counts[0] * 100U, max(sum, 1U)));
31113132
for (i = 1; i < ARRAY_SIZE(counts); i++) {
31123133
if (!counts[i])
31133134
continue;
3114-
seq_printf(seq, "Service buckets len-%d:\t%u (%lu%%)\n",
3135+
seq_printf(seq, "Service buckets len-%d:\t%u (%llu%%)\n",
31153136
i, counts[i],
3116-
(unsigned long)counts[i] * 100 / max(sum1, 1U));
3137+
div_u64((u64)counts[i] * 100U, max(sum1, 1U)));
31173138
}
31183139

31193140
after_svc:
@@ -5039,7 +5060,7 @@ int __net_init ip_vs_control_net_init(struct netns_ipvs *ipvs)
50395060
ipvs->net->proc_net,
50405061
ip_vs_stats_percpu_show, NULL))
50415062
goto err_percpu;
5042-
if (!proc_create_net_single("ip_vs_status", 0, ipvs->net->proc_net,
5063+
if (!proc_create_net_single("ip_vs_status", 0440, ipvs->net->proc_net,
50435064
ip_vs_status_show, NULL))
50445065
goto err_status;
50455066
#endif

0 commit comments

Comments
 (0)