* [PATCH] ipvs: Fix signed integer overflow when setsockopt timeout
@ 2019-01-10 8:39 ZhangXiaoxu
2019-01-14 7:34 ` zhangxiaoxu (A)
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: ZhangXiaoxu @ 2019-01-10 8:39 UTC (permalink / raw)
To: zhangxiaoxu5, wensong, horms, ja, netdev, lvs-devel
There is a UBSAN bug report as below:
UBSAN: Undefined behaviour in net/netfilter/ipvs/ip_vs_ctl.c:2227:21
signed integer overflow:
-2147483647 * 1000 cannot be represented in type 'int'
Reproduce program:
#include <stdio.h>
#include <sys/types.h>
#include <sys/socket.h>
#define IPPROTO_IP 0
#define IPPROTO_RAW 255
#define IP_VS_BASE_CTL (64+1024+64)
#define IP_VS_SO_SET_TIMEOUT (IP_VS_BASE_CTL+10)
/* The argument to IP_VS_SO_GET_TIMEOUT */
struct ipvs_timeout_t {
int tcp_timeout;
int tcp_fin_timeout;
int udp_timeout;
};
int main() {
int ret = -1;
int sockfd = -1;
struct ipvs_timeout_t to;
sockfd = socket(AF_INET, SOCK_RAW, IPPROTO_RAW);
if (sockfd == -1) {
printf("socket init error\n");
return -1;
}
to.tcp_timeout = -2147483647;
to.tcp_fin_timeout = -2147483647;
to.udp_timeout = -2147483647;
ret = setsockopt(sockfd,
IPPROTO_IP,
IP_VS_SO_SET_TIMEOUT,
(char *)(&to),
sizeof(to));
printf("setsockopt return %d\n", ret);
return ret;
}
Return -EINVAL if the timeout value is negative or max than 'INT_MAX / HZ'.
Signed-off-by: ZhangXiaoxu <zhangxiaoxu5@huawei.com>
---
net/netfilter/ipvs/ip_vs_ctl.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
index 432141f..444aaca 100644
--- a/net/netfilter/ipvs/ip_vs_ctl.c
+++ b/net/netfilter/ipvs/ip_vs_ctl.c
@@ -2221,6 +2221,19 @@ static int ip_vs_set_timeout(struct netns_ipvs *ipvs, struct ip_vs_timeout_user
u->udp_timeout);
#ifdef CONFIG_IP_VS_PROTO_TCP
+ if (u->tcp_timeout < 0 || u->tcp_timeout > (INT_MAX / HZ) ||
+ u->tcp_fin_timeout < 0 || u->tcp_fin_timeout > (INT_MAX / HZ)) {
+ return -EINVAL;
+ }
+#endif
+
+#ifdef CONFIG_IP_VS_PROTO_UDP
+ if (u->udp_timeout < 0 || u->udp_timeout > (INT_MAX / HZ)) {
+ return -EINVAL;
+ }
+#endif
+
+#ifdef CONFIG_IP_VS_PROTO_TCP
if (u->tcp_timeout) {
pd = ip_vs_proto_data_get(ipvs, IPPROTO_TCP);
pd->timeout_table[IP_VS_TCP_S_ESTABLISHED]
--
2.7.4
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] ipvs: Fix signed integer overflow when setsockopt timeout
2019-01-10 8:39 [PATCH] ipvs: Fix signed integer overflow when setsockopt timeout ZhangXiaoxu
@ 2019-01-14 7:34 ` zhangxiaoxu (A)
2019-01-14 12:15 ` Simon Horman
2019-01-24 12:32 ` Pablo Neira Ayuso
2 siblings, 0 replies; 5+ messages in thread
From: zhangxiaoxu (A) @ 2019-01-14 7:34 UTC (permalink / raw)
To: wensong, horms, ja, netdev, lvs-devel
ping?
On 1/10/2019 4:39 PM, ZhangXiaoxu wrote:
> There is a UBSAN bug report as below:
> UBSAN: Undefined behaviour in net/netfilter/ipvs/ip_vs_ctl.c:2227:21
> signed integer overflow:
> -2147483647 * 1000 cannot be represented in type 'int'
>
> Reproduce program:
> #include <stdio.h>
> #include <sys/types.h>
> #include <sys/socket.h>
>
> #define IPPROTO_IP 0
> #define IPPROTO_RAW 255
>
> #define IP_VS_BASE_CTL (64+1024+64)
> #define IP_VS_SO_SET_TIMEOUT (IP_VS_BASE_CTL+10)
>
> /* The argument to IP_VS_SO_GET_TIMEOUT */
> struct ipvs_timeout_t {
> int tcp_timeout;
> int tcp_fin_timeout;
> int udp_timeout;
> };
>
> int main() {
> int ret = -1;
> int sockfd = -1;
> struct ipvs_timeout_t to;
>
> sockfd = socket(AF_INET, SOCK_RAW, IPPROTO_RAW);
> if (sockfd == -1) {
> printf("socket init error\n");
> return -1;
> }
>
> to.tcp_timeout = -2147483647;
> to.tcp_fin_timeout = -2147483647;
> to.udp_timeout = -2147483647;
>
> ret = setsockopt(sockfd,
> IPPROTO_IP,
> IP_VS_SO_SET_TIMEOUT,
> (char *)(&to),
> sizeof(to));
>
> printf("setsockopt return %d\n", ret);
> return ret;
> }
>
> Return -EINVAL if the timeout value is negative or max than 'INT_MAX / HZ'.
>
> Signed-off-by: ZhangXiaoxu <zhangxiaoxu5@huawei.com>
> ---
> net/netfilter/ipvs/ip_vs_ctl.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
> index 432141f..444aaca 100644
> --- a/net/netfilter/ipvs/ip_vs_ctl.c
> +++ b/net/netfilter/ipvs/ip_vs_ctl.c
> @@ -2221,6 +2221,19 @@ static int ip_vs_set_timeout(struct netns_ipvs *ipvs, struct ip_vs_timeout_user
> u->udp_timeout);
>
> #ifdef CONFIG_IP_VS_PROTO_TCP
> + if (u->tcp_timeout < 0 || u->tcp_timeout > (INT_MAX / HZ) ||
> + u->tcp_fin_timeout < 0 || u->tcp_fin_timeout > (INT_MAX / HZ)) {
> + return -EINVAL;
> + }
> +#endif
> +
> +#ifdef CONFIG_IP_VS_PROTO_UDP
> + if (u->udp_timeout < 0 || u->udp_timeout > (INT_MAX / HZ)) {
> + return -EINVAL;
> + }
> +#endif
> +
> +#ifdef CONFIG_IP_VS_PROTO_TCP
> if (u->tcp_timeout) {
> pd = ip_vs_proto_data_get(ipvs, IPPROTO_TCP);
> pd->timeout_table[IP_VS_TCP_S_ESTABLISHED]
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] ipvs: Fix signed integer overflow when setsockopt timeout
2019-01-10 8:39 [PATCH] ipvs: Fix signed integer overflow when setsockopt timeout ZhangXiaoxu
2019-01-14 7:34 ` zhangxiaoxu (A)
@ 2019-01-14 12:15 ` Simon Horman
2019-01-24 12:22 ` zhangxiaoxu (A)
2019-01-24 12:32 ` Pablo Neira Ayuso
2 siblings, 1 reply; 5+ messages in thread
From: Simon Horman @ 2019-01-14 12:15 UTC (permalink / raw)
To: ZhangXiaoxu; +Cc: wensong, ja, netdev, lvs-devel, Pablo Neira Ayuso
On Thu, Jan 10, 2019 at 04:39:06PM +0800, ZhangXiaoxu wrote:
> There is a UBSAN bug report as below:
> UBSAN: Undefined behaviour in net/netfilter/ipvs/ip_vs_ctl.c:2227:21
> signed integer overflow:
> -2147483647 * 1000 cannot be represented in type 'int'
>
> Reproduce program:
> #include <stdio.h>
> #include <sys/types.h>
> #include <sys/socket.h>
>
> #define IPPROTO_IP 0
> #define IPPROTO_RAW 255
>
> #define IP_VS_BASE_CTL (64+1024+64)
> #define IP_VS_SO_SET_TIMEOUT (IP_VS_BASE_CTL+10)
>
> /* The argument to IP_VS_SO_GET_TIMEOUT */
> struct ipvs_timeout_t {
> int tcp_timeout;
> int tcp_fin_timeout;
> int udp_timeout;
> };
>
> int main() {
> int ret = -1;
> int sockfd = -1;
> struct ipvs_timeout_t to;
>
> sockfd = socket(AF_INET, SOCK_RAW, IPPROTO_RAW);
> if (sockfd == -1) {
> printf("socket init error\n");
> return -1;
> }
>
> to.tcp_timeout = -2147483647;
> to.tcp_fin_timeout = -2147483647;
> to.udp_timeout = -2147483647;
>
> ret = setsockopt(sockfd,
> IPPROTO_IP,
> IP_VS_SO_SET_TIMEOUT,
> (char *)(&to),
> sizeof(to));
>
> printf("setsockopt return %d\n", ret);
> return ret;
> }
>
> Return -EINVAL if the timeout value is negative or max than 'INT_MAX / HZ'.
>
> Signed-off-by: ZhangXiaoxu <zhangxiaoxu5@huawei.com>
Thanks, this looks good to me.
Pablo, could you consider applying this to nf?
Acked-by: Simon Horman <horms@verge.net.au>
> ---
> net/netfilter/ipvs/ip_vs_ctl.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
> index 432141f..444aaca 100644
> --- a/net/netfilter/ipvs/ip_vs_ctl.c
> +++ b/net/netfilter/ipvs/ip_vs_ctl.c
> @@ -2221,6 +2221,19 @@ static int ip_vs_set_timeout(struct netns_ipvs *ipvs, struct ip_vs_timeout_user
> u->udp_timeout);
>
> #ifdef CONFIG_IP_VS_PROTO_TCP
> + if (u->tcp_timeout < 0 || u->tcp_timeout > (INT_MAX / HZ) ||
> + u->tcp_fin_timeout < 0 || u->tcp_fin_timeout > (INT_MAX / HZ)) {
> + return -EINVAL;
> + }
> +#endif
> +
> +#ifdef CONFIG_IP_VS_PROTO_UDP
> + if (u->udp_timeout < 0 || u->udp_timeout > (INT_MAX / HZ)) {
> + return -EINVAL;
> + }
> +#endif
> +
> +#ifdef CONFIG_IP_VS_PROTO_TCP
> if (u->tcp_timeout) {
> pd = ip_vs_proto_data_get(ipvs, IPPROTO_TCP);
> pd->timeout_table[IP_VS_TCP_S_ESTABLISHED]
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] ipvs: Fix signed integer overflow when setsockopt timeout
2019-01-14 12:15 ` Simon Horman
@ 2019-01-24 12:22 ` zhangxiaoxu (A)
0 siblings, 0 replies; 5+ messages in thread
From: zhangxiaoxu (A) @ 2019-01-24 12:22 UTC (permalink / raw)
To: Simon Horman; +Cc: wensong, ja, netdev, lvs-devel, Pablo Neira Ayuso
ping.
On 1/14/2019 8:15 PM, Simon Horman wrote:
> On Thu, Jan 10, 2019 at 04:39:06PM +0800, ZhangXiaoxu wrote:
>> There is a UBSAN bug report as below:
>> UBSAN: Undefined behaviour in net/netfilter/ipvs/ip_vs_ctl.c:2227:21
>> signed integer overflow:
>> -2147483647 * 1000 cannot be represented in type 'int'
>>
>> Reproduce program:
>> #include <stdio.h>
>> #include <sys/types.h>
>> #include <sys/socket.h>
>>
>> #define IPPROTO_IP 0
>> #define IPPROTO_RAW 255
>>
>> #define IP_VS_BASE_CTL (64+1024+64)
>> #define IP_VS_SO_SET_TIMEOUT (IP_VS_BASE_CTL+10)
>>
>> /* The argument to IP_VS_SO_GET_TIMEOUT */
>> struct ipvs_timeout_t {
>> int tcp_timeout;
>> int tcp_fin_timeout;
>> int udp_timeout;
>> };
>>
>> int main() {
>> int ret = -1;
>> int sockfd = -1;
>> struct ipvs_timeout_t to;
>>
>> sockfd = socket(AF_INET, SOCK_RAW, IPPROTO_RAW);
>> if (sockfd == -1) {
>> printf("socket init error\n");
>> return -1;
>> }
>>
>> to.tcp_timeout = -2147483647;
>> to.tcp_fin_timeout = -2147483647;
>> to.udp_timeout = -2147483647;
>>
>> ret = setsockopt(sockfd,
>> IPPROTO_IP,
>> IP_VS_SO_SET_TIMEOUT,
>> (char *)(&to),
>> sizeof(to));
>>
>> printf("setsockopt return %d\n", ret);
>> return ret;
>> }
>>
>> Return -EINVAL if the timeout value is negative or max than 'INT_MAX / HZ'.
>>
>> Signed-off-by: ZhangXiaoxu <zhangxiaoxu5@huawei.com>
>
> Thanks, this looks good to me.
>
> Pablo, could you consider applying this to nf?
>
> Acked-by: Simon Horman <horms@verge.net.au>
>
>> ---
>> net/netfilter/ipvs/ip_vs_ctl.c | 13 +++++++++++++
>> 1 file changed, 13 insertions(+)
>>
>> diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
>> index 432141f..444aaca 100644
>> --- a/net/netfilter/ipvs/ip_vs_ctl.c
>> +++ b/net/netfilter/ipvs/ip_vs_ctl.c
>> @@ -2221,6 +2221,19 @@ static int ip_vs_set_timeout(struct netns_ipvs *ipvs, struct ip_vs_timeout_user
>> u->udp_timeout);
>>
>> #ifdef CONFIG_IP_VS_PROTO_TCP
>> + if (u->tcp_timeout < 0 || u->tcp_timeout > (INT_MAX / HZ) ||
>> + u->tcp_fin_timeout < 0 || u->tcp_fin_timeout > (INT_MAX / HZ)) {
>> + return -EINVAL;
>> + }
>> +#endif
>> +
>> +#ifdef CONFIG_IP_VS_PROTO_UDP
>> + if (u->udp_timeout < 0 || u->udp_timeout > (INT_MAX / HZ)) {
>> + return -EINVAL;
>> + }
>> +#endif
>> +
>> +#ifdef CONFIG_IP_VS_PROTO_TCP
>> if (u->tcp_timeout) {
>> pd = ip_vs_proto_data_get(ipvs, IPPROTO_TCP);
>> pd->timeout_table[IP_VS_TCP_S_ESTABLISHED]
>> --
>> 2.7.4
>>
>
> .
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ipvs: Fix signed integer overflow when setsockopt timeout
2019-01-10 8:39 [PATCH] ipvs: Fix signed integer overflow when setsockopt timeout ZhangXiaoxu
2019-01-14 7:34 ` zhangxiaoxu (A)
2019-01-14 12:15 ` Simon Horman
@ 2019-01-24 12:32 ` Pablo Neira Ayuso
2 siblings, 0 replies; 5+ messages in thread
From: Pablo Neira Ayuso @ 2019-01-24 12:32 UTC (permalink / raw)
To: ZhangXiaoxu; +Cc: wensong, horms, ja, netdev, lvs-devel, netfilter-devel
On Thu, Jan 10, 2019 at 04:39:06PM +0800, ZhangXiaoxu wrote:
> There is a UBSAN bug report as below:
> UBSAN: Undefined behaviour in net/netfilter/ipvs/ip_vs_ctl.c:2227:21
> signed integer overflow:
> -2147483647 * 1000 cannot be represented in type 'int'
Applied, thanks.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-01-24 12:32 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-01-10 8:39 [PATCH] ipvs: Fix signed integer overflow when setsockopt timeout ZhangXiaoxu
2019-01-14 7:34 ` zhangxiaoxu (A)
2019-01-14 12:15 ` Simon Horman
2019-01-24 12:22 ` zhangxiaoxu (A)
2019-01-24 12:32 ` Pablo Neira Ayuso
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox