Skip to content

Commit 5e29dc3

Browse files
Jozsef Kadlecsikummakynes
authored andcommitted
netfilter: ipset: Rework long task execution when adding/deleting entries
When adding/deleting large number of elements in one step in ipset, it can take a reasonable amount of time and can result in soft lockup errors. The patch 5f7b51b ("netfilter: ipset: Limit the maximal range of consecutive elements to add/delete") tried to fix it by limiting the max elements to process at all. However it was not enough, it is still possible that we get hung tasks. Lowering the limit is not reasonable, so the approach in this patch is as follows: rely on the method used at resizing sets and save the state when we reach a smaller internal batch limit, unlock/lock and proceed from the saved state. Thus we can avoid long continuous tasks and at the same time removed the limit to add/delete large number of elements in one step. The nfnl mutex is held during the whole operation which prevents one to issue other ipset commands in parallel. Fixes: 5f7b51b ("netfilter: ipset: Limit the maximal range of consecutive elements to add/delete") Reported-by: syzbot+9204e7399656300bf271@syzkaller.appspotmail.com Signed-off-by: Jozsef Kadlecsik <kadlec@netfilter.org> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
1 parent a31d47b commit 5e29dc3

File tree

11 files changed

+68
-81
lines changed

11 files changed

+68
-81
lines changed

include/linux/netfilter/ipset/ip_set.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,7 @@ struct ip_set_region {
197197
};
198198

199199
/* Max range where every element is added/deleted in one step */
200-
#define IPSET_MAX_RANGE (1<<20)
200+
#define IPSET_MAX_RANGE (1<<14)
201201

202202
/* The max revision number supported by any set type + 1 */
203203
#define IPSET_REVISION_MAX 9

net/netfilter/ipset/ip_set_core.c

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1698,9 +1698,10 @@ call_ad(struct net *net, struct sock *ctnl, struct sk_buff *skb,
16981698
ret = set->variant->uadt(set, tb, adt, &lineno, flags, retried);
16991699
ip_set_unlock(set);
17001700
retried = true;
1701-
} while (ret == -EAGAIN &&
1702-
set->variant->resize &&
1703-
(ret = set->variant->resize(set, retried)) == 0);
1701+
} while (ret == -ERANGE ||
1702+
(ret == -EAGAIN &&
1703+
set->variant->resize &&
1704+
(ret = set->variant->resize(set, retried)) == 0));
17041705

17051706
if (!ret || (ret == -IPSET_ERR_EXIST && eexist))
17061707
return 0;

net/netfilter/ipset/ip_set_hash_ip.c

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -100,11 +100,11 @@ static int
100100
hash_ip4_uadt(struct ip_set *set, struct nlattr *tb[],
101101
enum ipset_adt adt, u32 *lineno, u32 flags, bool retried)
102102
{
103-
const struct hash_ip4 *h = set->data;
103+
struct hash_ip4 *h = set->data;
104104
ipset_adtfn adtfn = set->variant->adt[adt];
105105
struct hash_ip4_elem e = { 0 };
106106
struct ip_set_ext ext = IP_SET_INIT_UEXT(set);
107-
u32 ip = 0, ip_to = 0, hosts;
107+
u32 ip = 0, ip_to = 0, hosts, i = 0;
108108
int ret = 0;
109109

110110
if (tb[IPSET_ATTR_LINENO])
@@ -149,14 +149,14 @@ hash_ip4_uadt(struct ip_set *set, struct nlattr *tb[],
149149

150150
hosts = h->netmask == 32 ? 1 : 2 << (32 - h->netmask - 1);
151151

152-
/* 64bit division is not allowed on 32bit */
153-
if (((u64)ip_to - ip + 1) >> (32 - h->netmask) > IPSET_MAX_RANGE)
154-
return -ERANGE;
155-
156152
if (retried)
157153
ip = ntohl(h->next.ip);
158-
for (; ip <= ip_to;) {
154+
for (; ip <= ip_to; i++) {
159155
e.ip = htonl(ip);
156+
if (i > IPSET_MAX_RANGE) {
157+
hash_ip4_data_next(&h->next, &e);
158+
return -ERANGE;
159+
}
160160
ret = adtfn(set, &e, &ext, &ext, flags);
161161
if (ret && !ip_set_eexist(ret, flags))
162162
return ret;

net/netfilter/ipset/ip_set_hash_ipmark.c

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -97,11 +97,11 @@ static int
9797
hash_ipmark4_uadt(struct ip_set *set, struct nlattr *tb[],
9898
enum ipset_adt adt, u32 *lineno, u32 flags, bool retried)
9999
{
100-
const struct hash_ipmark4 *h = set->data;
100+
struct hash_ipmark4 *h = set->data;
101101
ipset_adtfn adtfn = set->variant->adt[adt];
102102
struct hash_ipmark4_elem e = { };
103103
struct ip_set_ext ext = IP_SET_INIT_UEXT(set);
104-
u32 ip, ip_to = 0;
104+
u32 ip, ip_to = 0, i = 0;
105105
int ret;
106106

107107
if (tb[IPSET_ATTR_LINENO])
@@ -148,13 +148,14 @@ hash_ipmark4_uadt(struct ip_set *set, struct nlattr *tb[],
148148
ip_set_mask_from_to(ip, ip_to, cidr);
149149
}
150150

151-
if (((u64)ip_to - ip + 1) > IPSET_MAX_RANGE)
152-
return -ERANGE;
153-
154151
if (retried)
155152
ip = ntohl(h->next.ip);
156-
for (; ip <= ip_to; ip++) {
153+
for (; ip <= ip_to; ip++, i++) {
157154
e.ip = htonl(ip);
155+
if (i > IPSET_MAX_RANGE) {
156+
hash_ipmark4_data_next(&h->next, &e);
157+
return -ERANGE;
158+
}
158159
ret = adtfn(set, &e, &ext, &ext, flags);
159160

160161
if (ret && !ip_set_eexist(ret, flags))

net/netfilter/ipset/ip_set_hash_ipport.c

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -112,11 +112,11 @@ static int
112112
hash_ipport4_uadt(struct ip_set *set, struct nlattr *tb[],
113113
enum ipset_adt adt, u32 *lineno, u32 flags, bool retried)
114114
{
115-
const struct hash_ipport4 *h = set->data;
115+
struct hash_ipport4 *h = set->data;
116116
ipset_adtfn adtfn = set->variant->adt[adt];
117117
struct hash_ipport4_elem e = { .ip = 0 };
118118
struct ip_set_ext ext = IP_SET_INIT_UEXT(set);
119-
u32 ip, ip_to = 0, p = 0, port, port_to;
119+
u32 ip, ip_to = 0, p = 0, port, port_to, i = 0;
120120
bool with_ports = false;
121121
int ret;
122122

@@ -184,17 +184,18 @@ hash_ipport4_uadt(struct ip_set *set, struct nlattr *tb[],
184184
swap(port, port_to);
185185
}
186186

187-
if (((u64)ip_to - ip + 1)*(port_to - port + 1) > IPSET_MAX_RANGE)
188-
return -ERANGE;
189-
190187
if (retried)
191188
ip = ntohl(h->next.ip);
192189
for (; ip <= ip_to; ip++) {
193190
p = retried && ip == ntohl(h->next.ip) ? ntohs(h->next.port)
194191
: port;
195-
for (; p <= port_to; p++) {
192+
for (; p <= port_to; p++, i++) {
196193
e.ip = htonl(ip);
197194
e.port = htons(p);
195+
if (i > IPSET_MAX_RANGE) {
196+
hash_ipport4_data_next(&h->next, &e);
197+
return -ERANGE;
198+
}
198199
ret = adtfn(set, &e, &ext, &ext, flags);
199200

200201
if (ret && !ip_set_eexist(ret, flags))

net/netfilter/ipset/ip_set_hash_ipportip.c

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -108,11 +108,11 @@ static int
108108
hash_ipportip4_uadt(struct ip_set *set, struct nlattr *tb[],
109109
enum ipset_adt adt, u32 *lineno, u32 flags, bool retried)
110110
{
111-
const struct hash_ipportip4 *h = set->data;
111+
struct hash_ipportip4 *h = set->data;
112112
ipset_adtfn adtfn = set->variant->adt[adt];
113113
struct hash_ipportip4_elem e = { .ip = 0 };
114114
struct ip_set_ext ext = IP_SET_INIT_UEXT(set);
115-
u32 ip, ip_to = 0, p = 0, port, port_to;
115+
u32 ip, ip_to = 0, p = 0, port, port_to, i = 0;
116116
bool with_ports = false;
117117
int ret;
118118

@@ -180,17 +180,18 @@ hash_ipportip4_uadt(struct ip_set *set, struct nlattr *tb[],
180180
swap(port, port_to);
181181
}
182182

183-
if (((u64)ip_to - ip + 1)*(port_to - port + 1) > IPSET_MAX_RANGE)
184-
return -ERANGE;
185-
186183
if (retried)
187184
ip = ntohl(h->next.ip);
188185
for (; ip <= ip_to; ip++) {
189186
p = retried && ip == ntohl(h->next.ip) ? ntohs(h->next.port)
190187
: port;
191-
for (; p <= port_to; p++) {
188+
for (; p <= port_to; p++, i++) {
192189
e.ip = htonl(ip);
193190
e.port = htons(p);
191+
if (i > IPSET_MAX_RANGE) {
192+
hash_ipportip4_data_next(&h->next, &e);
193+
return -ERANGE;
194+
}
194195
ret = adtfn(set, &e, &ext, &ext, flags);
195196

196197
if (ret && !ip_set_eexist(ret, flags))

net/netfilter/ipset/ip_set_hash_ipportnet.c

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -160,12 +160,12 @@ static int
160160
hash_ipportnet4_uadt(struct ip_set *set, struct nlattr *tb[],
161161
enum ipset_adt adt, u32 *lineno, u32 flags, bool retried)
162162
{
163-
const struct hash_ipportnet4 *h = set->data;
163+
struct hash_ipportnet4 *h = set->data;
164164
ipset_adtfn adtfn = set->variant->adt[adt];
165165
struct hash_ipportnet4_elem e = { .cidr = HOST_MASK - 1 };
166166
struct ip_set_ext ext = IP_SET_INIT_UEXT(set);
167167
u32 ip = 0, ip_to = 0, p = 0, port, port_to;
168-
u32 ip2_from = 0, ip2_to = 0, ip2;
168+
u32 ip2_from = 0, ip2_to = 0, ip2, i = 0;
169169
bool with_ports = false;
170170
u8 cidr;
171171
int ret;
@@ -253,9 +253,6 @@ hash_ipportnet4_uadt(struct ip_set *set, struct nlattr *tb[],
253253
swap(port, port_to);
254254
}
255255

256-
if (((u64)ip_to - ip + 1)*(port_to - port + 1) > IPSET_MAX_RANGE)
257-
return -ERANGE;
258-
259256
ip2_to = ip2_from;
260257
if (tb[IPSET_ATTR_IP2_TO]) {
261258
ret = ip_set_get_hostipaddr4(tb[IPSET_ATTR_IP2_TO], &ip2_to);
@@ -282,9 +279,15 @@ hash_ipportnet4_uadt(struct ip_set *set, struct nlattr *tb[],
282279
for (; p <= port_to; p++) {
283280
e.port = htons(p);
284281
do {
282+
i++;
285283
e.ip2 = htonl(ip2);
286284
ip2 = ip_set_range_to_cidr(ip2, ip2_to, &cidr);
287285
e.cidr = cidr - 1;
286+
if (i > IPSET_MAX_RANGE) {
287+
hash_ipportnet4_data_next(&h->next,
288+
&e);
289+
return -ERANGE;
290+
}
288291
ret = adtfn(set, &e, &ext, &ext, flags);
289292

290293
if (ret && !ip_set_eexist(ret, flags))

net/netfilter/ipset/ip_set_hash_net.c

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -136,11 +136,11 @@ static int
136136
hash_net4_uadt(struct ip_set *set, struct nlattr *tb[],
137137
enum ipset_adt adt, u32 *lineno, u32 flags, bool retried)
138138
{
139-
const struct hash_net4 *h = set->data;
139+
struct hash_net4 *h = set->data;
140140
ipset_adtfn adtfn = set->variant->adt[adt];
141141
struct hash_net4_elem e = { .cidr = HOST_MASK };
142142
struct ip_set_ext ext = IP_SET_INIT_UEXT(set);
143-
u32 ip = 0, ip_to = 0, ipn, n = 0;
143+
u32 ip = 0, ip_to = 0, i = 0;
144144
int ret;
145145

146146
if (tb[IPSET_ATTR_LINENO])
@@ -188,19 +188,16 @@ hash_net4_uadt(struct ip_set *set, struct nlattr *tb[],
188188
if (ip + UINT_MAX == ip_to)
189189
return -IPSET_ERR_HASH_RANGE;
190190
}
191-
ipn = ip;
192-
do {
193-
ipn = ip_set_range_to_cidr(ipn, ip_to, &e.cidr);
194-
n++;
195-
} while (ipn++ < ip_to);
196-
197-
if (n > IPSET_MAX_RANGE)
198-
return -ERANGE;
199191

200192
if (retried)
201193
ip = ntohl(h->next.ip);
202194
do {
195+
i++;
203196
e.ip = htonl(ip);
197+
if (i > IPSET_MAX_RANGE) {
198+
hash_net4_data_next(&h->next, &e);
199+
return -ERANGE;
200+
}
204201
ip = ip_set_range_to_cidr(ip, ip_to, &e.cidr);
205202
ret = adtfn(set, &e, &ext, &ext, flags);
206203
if (ret && !ip_set_eexist(ret, flags))

net/netfilter/ipset/ip_set_hash_netiface.c

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,7 @@ hash_netiface4_uadt(struct ip_set *set, struct nlattr *tb[],
202202
ipset_adtfn adtfn = set->variant->adt[adt];
203203
struct hash_netiface4_elem e = { .cidr = HOST_MASK, .elem = 1 };
204204
struct ip_set_ext ext = IP_SET_INIT_UEXT(set);
205-
u32 ip = 0, ip_to = 0, ipn, n = 0;
205+
u32 ip = 0, ip_to = 0, i = 0;
206206
int ret;
207207

208208
if (tb[IPSET_ATTR_LINENO])
@@ -256,19 +256,16 @@ hash_netiface4_uadt(struct ip_set *set, struct nlattr *tb[],
256256
} else {
257257
ip_set_mask_from_to(ip, ip_to, e.cidr);
258258
}
259-
ipn = ip;
260-
do {
261-
ipn = ip_set_range_to_cidr(ipn, ip_to, &e.cidr);
262-
n++;
263-
} while (ipn++ < ip_to);
264-
265-
if (n > IPSET_MAX_RANGE)
266-
return -ERANGE;
267259

268260
if (retried)
269261
ip = ntohl(h->next.ip);
270262
do {
263+
i++;
271264
e.ip = htonl(ip);
265+
if (i > IPSET_MAX_RANGE) {
266+
hash_netiface4_data_next(&h->next, &e);
267+
return -ERANGE;
268+
}
272269
ip = ip_set_range_to_cidr(ip, ip_to, &e.cidr);
273270
ret = adtfn(set, &e, &ext, &ext, flags);
274271

net/netfilter/ipset/ip_set_hash_netnet.c

Lines changed: 7 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -166,13 +166,12 @@ static int
166166
hash_netnet4_uadt(struct ip_set *set, struct nlattr *tb[],
167167
enum ipset_adt adt, u32 *lineno, u32 flags, bool retried)
168168
{
169-
const struct hash_netnet4 *h = set->data;
169+
struct hash_netnet4 *h = set->data;
170170
ipset_adtfn adtfn = set->variant->adt[adt];
171171
struct hash_netnet4_elem e = { };
172172
struct ip_set_ext ext = IP_SET_INIT_UEXT(set);
173173
u32 ip = 0, ip_to = 0;
174-
u32 ip2 = 0, ip2_from = 0, ip2_to = 0, ipn;
175-
u64 n = 0, m = 0;
174+
u32 ip2 = 0, ip2_from = 0, ip2_to = 0, i = 0;
176175
int ret;
177176

178177
if (tb[IPSET_ATTR_LINENO])
@@ -248,19 +247,6 @@ hash_netnet4_uadt(struct ip_set *set, struct nlattr *tb[],
248247
} else {
249248
ip_set_mask_from_to(ip2_from, ip2_to, e.cidr[1]);
250249
}
251-
ipn = ip;
252-
do {
253-
ipn = ip_set_range_to_cidr(ipn, ip_to, &e.cidr[0]);
254-
n++;
255-
} while (ipn++ < ip_to);
256-
ipn = ip2_from;
257-
do {
258-
ipn = ip_set_range_to_cidr(ipn, ip2_to, &e.cidr[1]);
259-
m++;
260-
} while (ipn++ < ip2_to);
261-
262-
if (n*m > IPSET_MAX_RANGE)
263-
return -ERANGE;
264250

265251
if (retried) {
266252
ip = ntohl(h->next.ip[0]);
@@ -273,7 +259,12 @@ hash_netnet4_uadt(struct ip_set *set, struct nlattr *tb[],
273259
e.ip[0] = htonl(ip);
274260
ip = ip_set_range_to_cidr(ip, ip_to, &e.cidr[0]);
275261
do {
262+
i++;
276263
e.ip[1] = htonl(ip2);
264+
if (i > IPSET_MAX_RANGE) {
265+
hash_netnet4_data_next(&h->next, &e);
266+
return -ERANGE;
267+
}
277268
ip2 = ip_set_range_to_cidr(ip2, ip2_to, &e.cidr[1]);
278269
ret = adtfn(set, &e, &ext, &ext, flags);
279270
if (ret && !ip_set_eexist(ret, flags))

0 commit comments

Comments
 (0)