| Message ID | 1304e396-7249-4fb3-8337-0c2f88472693@stanley.mountain (mailing list archive) |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [v2,net] ipvs: prevent integer overflow in do_ip_vs_get_ctl() | expand |
Hello, On Mon, 10 Mar 2025, Dan Carpenter wrote: > The get->num_services variable is an unsigned int which is controlled by > the user. The struct_size() function ensures that the size calculation > does not overflow an unsigned long, however, we are saving the result to > an int so the calculation can overflow. > > Both "len" and "get->num_services" come from the user. This check is > just a sanity check to help the user and ensure they are using the API > correctly. An integer overflow here is not a big deal. This has no > security impact. > > Save the result from struct_size() type size_t to fix this integer > overflow bug. > > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") > Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org> Looks good to me, thanks! Acked-by: Julian Anastasov <ja@ssi.bg> Pablo, you can apply it to the nf tree. > --- > v2: fix %lu vs %zu in the printk(). It breaks the build on 32bit > systems. > Remove the CC stable. > > net/netfilter/ipvs/ip_vs_ctl.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c > index 7d13110ce188..0633276d96bf 100644 > --- a/net/netfilter/ipvs/ip_vs_ctl.c > +++ b/net/netfilter/ipvs/ip_vs_ctl.c > @@ -3091,12 +3091,12 @@ do_ip_vs_get_ctl(struct sock *sk, int cmd, void __user *user, int *len) > case IP_VS_SO_GET_SERVICES: > { > struct ip_vs_get_services *get; > - int size; > + size_t size; > > get = (struct ip_vs_get_services *)arg; > size = struct_size(get, entrytable, get->num_services); > if (*len != size) { > - pr_err("length: %u != %u\n", *len, size); > + pr_err("length: %u != %zu\n", *len, size); > ret = -EINVAL; > goto out; > } > @@ -3132,12 +3132,12 @@ do_ip_vs_get_ctl(struct sock *sk, int cmd, void __user *user, int *len) > case IP_VS_SO_GET_DESTS: > { > struct ip_vs_get_dests *get; > - int size; > + size_t size; > > get = (struct ip_vs_get_dests *)arg; > size = struct_size(get, entrytable, get->num_dests); > if (*len != size) { > - pr_err("length: %u != %u\n", *len, size); > + pr_err("length: %u != %zu\n", *len, size); > ret = -EINVAL; > goto out; > } > -- > 2.47.2 Regards -- Julian Anastasov <ja@ssi.bg>
On Tue, Mar 11, 2025 at 07:50:44PM +0200, Julian Anastasov wrote: > > Hello, > > On Mon, 10 Mar 2025, Dan Carpenter wrote: > > > The get->num_services variable is an unsigned int which is controlled by > > the user. The struct_size() function ensures that the size calculation > > does not overflow an unsigned long, however, we are saving the result to > > an int so the calculation can overflow. > > > > Both "len" and "get->num_services" come from the user. This check is > > just a sanity check to help the user and ensure they are using the API > > correctly. An integer overflow here is not a big deal. This has no > > security impact. > > > > Save the result from struct_size() type size_t to fix this integer > > overflow bug. > > > > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") > > Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org> > > Looks good to me, thanks! > > Acked-by: Julian Anastasov <ja@ssi.bg> > > Pablo, you can apply it to the nf tree. Done, thanks Julian.
diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c index 7d13110ce188..0633276d96bf 100644 --- a/net/netfilter/ipvs/ip_vs_ctl.c +++ b/net/netfilter/ipvs/ip_vs_ctl.c @@ -3091,12 +3091,12 @@ do_ip_vs_get_ctl(struct sock *sk, int cmd, void __user *user, int *len) case IP_VS_SO_GET_SERVICES: { struct ip_vs_get_services *get; - int size; + size_t size; get = (struct ip_vs_get_services *)arg; size = struct_size(get, entrytable, get->num_services); if (*len != size) { - pr_err("length: %u != %u\n", *len, size); + pr_err("length: %u != %zu\n", *len, size); ret = -EINVAL; goto out; } @@ -3132,12 +3132,12 @@ do_ip_vs_get_ctl(struct sock *sk, int cmd, void __user *user, int *len) case IP_VS_SO_GET_DESTS: { struct ip_vs_get_dests *get; - int size; + size_t size; get = (struct ip_vs_get_dests *)arg; size = struct_size(get, entrytable, get->num_dests); if (*len != size) { - pr_err("length: %u != %u\n", *len, size); + pr_err("length: %u != %zu\n", *len, size); ret = -EINVAL; goto out; }
The get->num_services variable is an unsigned int which is controlled by the user. The struct_size() function ensures that the size calculation does not overflow an unsigned long, however, we are saving the result to an int so the calculation can overflow. Both "len" and "get->num_services" come from the user. This check is just a sanity check to help the user and ensure they are using the API correctly. An integer overflow here is not a big deal. This has no security impact. Save the result from struct_size() type size_t to fix this integer overflow bug. Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org> --- v2: fix %lu vs %zu in the printk(). It breaks the build on 32bit systems. Remove the CC stable. net/netfilter/ipvs/ip_vs_ctl.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)