libipvs: fix some buffer sizes

Size or length? Here is the answer:

- IP_VS_SCHEDNAME_MAXLEN and IP_VS_IFNAME_MAXLEN are sizes
because they are used in kernel structures exported to user
space for the old setsockopt interface. We can not change
these structures in the kernel.

- IP_VS_PENAME_MAXLEN and IP_VS_PEDATA_MAXLEN are max lengths
because they are not exported to the old interface.

As result:
- buffers should have space for NUL terminator
- strncpy should use sizeof(buffer) - 1 as max length

As we change the libipvs structures, their users should be
recompiled.

Maintainers notes:
This change is related/inspired by kernel commit 52f96757905b
("ipvs: fix buffer overflow with sync daemon and service") found
by syzkaller.  While this fix is found by manual review.

Signed-off-by: Julian Anastasov <ja@ssi.bg>
Acked-by: Simon Horman <horms@verge.net.au>
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>

3 files changed, 10 insertions, 9 deletions

diff --git a/ipvsadm.c b/ipvsadm.c
index 1a28d72..7695006 100644
--- a/ipvsadm.c
+++ b/ipvsadm.c
@@ -595,7 +595,7 @@ parse_options(int argc, char **argv, struct ipvs_command_entry *ce,
 		case 's':
 			set_option(options, OPT_SCHEDULER);
 			strncpy(ce->svc.sched_name,
-				optarg, IP_VS_SCHEDNAME_MAXLEN);
+				optarg, IP_VS_SCHEDNAME_MAXLEN - 1);
 			break;
 		case 'p':
 			set_option(options, OPT_PERSISTENT);
@@ -670,7 +670,7 @@ parse_options(int argc, char **argv, struct ipvs_command_entry *ce,
 		case TAG_MCAST_INTERFACE:
 			set_option(options, OPT_MCAST);
 			strncpy(ce->daemon.mcast_ifn,
-				optarg, IP_VS_IFNAME_MAXLEN);
+				optarg, IP_VS_IFNAME_MAXLEN - 1);
 			break;
 		case 'I':
 			set_option(options, OPT_SYNCID);
@@ -1415,8 +1415,8 @@ static void print_conn(char *buf, unsigned int format)
 	unsigned int    expires;
 	unsigned short  af = AF_INET;
 	unsigned short  daf = AF_INET;
-	char		pe_name[IP_VS_PENAME_MAXLEN];
-	char		pe_data[IP_VS_PEDATA_MAXLEN];
+	char		pe_name[IP_VS_PENAME_MAXLEN + 1];
+	char		pe_data[IP_VS_PEDATA_MAXLEN + 1];
 
 	int n;
 	char temp1[INET6_ADDRSTRLEN], temp2[INET6_ADDRSTRLEN], temp3[INET6_ADDRSTRLEN];
diff --git a/libipvs/ip_vs.h b/libipvs/ip_vs.h
index 774ff96..e57d55a 100644
--- a/libipvs/ip_vs.h
+++ b/libipvs/ip_vs.h
@@ -144,7 +144,7 @@ struct ip_vs_service_user {
 	__be32			netmask;	/* persistent netmask */
 	u_int16_t		af;
 	union nf_inet_addr	addr;
-	char			pe_name[IP_VS_PENAME_MAXLEN];
+	char			pe_name[IP_VS_PENAME_MAXLEN + 1];
 };
 
 struct ip_vs_dest_kern {
@@ -267,7 +267,7 @@ struct ip_vs_service_entry {
 
 	u_int16_t		af;
 	union nf_inet_addr	addr;
-	char			pe_name[IP_VS_PENAME_MAXLEN];
+	char			pe_name[IP_VS_PENAME_MAXLEN + 1];
 
 	/* statistics, 64-bit */
 	struct ip_vs_stats64	stats64;
diff --git a/libipvs/libipvs.c b/libipvs/libipvs.c
index a843243..9be7700 100644
--- a/libipvs/libipvs.c
+++ b/libipvs/libipvs.c
@@ -719,7 +719,7 @@ static int ipvs_services_parse_cb(struct nl_msg *msg, void *arg)
 
 	strncpy(get->entrytable[i].sched_name,
 		nla_get_string(svc_attrs[IPVS_SVC_ATTR_SCHED_NAME]),
-		IP_VS_SCHEDNAME_MAXLEN);
+		IP_VS_SCHEDNAME_MAXLEN - 1);
 
 	if (svc_attrs[IPVS_SVC_ATTR_PE_NAME])
 		strncpy(get->entrytable[i].pe_name,
@@ -1199,7 +1199,7 @@ static int ipvs_daemon_parse_cb(struct nl_msg *msg, void *arg)
 	u[i].state = nla_get_u32(daemon_attrs[IPVS_DAEMON_ATTR_STATE]);
 	strncpy(u[i].mcast_ifn,
 		nla_get_string(daemon_attrs[IPVS_DAEMON_ATTR_MCAST_IFN]),
-		IP_VS_IFNAME_MAXLEN);
+		IP_VS_IFNAME_MAXLEN - 1);
 	u[i].syncid = nla_get_u32(daemon_attrs[IPVS_DAEMON_ATTR_SYNC_ID]);
 
 	a = daemon_attrs[IPVS_DAEMON_ATTR_SYNC_MAXLEN];
@@ -1265,7 +1265,8 @@ ipvs_daemon_t *ipvs_get_daemon(void)
 	}
 	for (i = 0; i < 2; i++) {
 		u[i].state = dmk[i].state;
-		strncpy(u[i].mcast_ifn, dmk[i].mcast_ifn, IP_VS_IFNAME_MAXLEN);
+		strncpy(u[i].mcast_ifn, dmk[i].mcast_ifn,
+			IP_VS_IFNAME_MAXLEN - 1);
 		u[i].syncid = dmk[i].syncid;
 	}
 	return u;