| Message ID | 1547109546-11344-1-git-send-email-zhangxiaoxu5@huawei.com |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | David Miller |
| Headers | show |
| Series | ipvs: Fix signed integer overflow when setsockopt timeout | expand |
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] >
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 >
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 >> > > . >
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.
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]
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(+)