misc/170832: jail v2 loses a binding of ip which sets after ips with /"network prefix"

[Jamie Gritton]

On 08/21/12 12:49, Oleg Ginzburg wrote:
>> Number:         170832
>> Category:       misc
>> Synopsis:       jail v2 loses a binding of ip which sets after ips with /"network prefix"
>> Confidential:   no
>> Severity:       non-critical
>> Priority:       low
>> Responsible:    freebsd-bugs
>> State:          open
>> Quarter:
>> Keywords:
>> Date-Required:
>> Class:          sw-bug
>> Submitter-Id:   current-users
>> Arrival-Date:   Tue Aug 21 18:50:10 UTC 2012
>> Closed-Date:
>> Last-Modified:
>> Originator:     Oleg Ginzburg
>> Release:        9.1-PRERELEASE, 10-CURRENT
>> Organization:
>> Environment:
> FreeBSD cbuilder64.my.domain 10.0-CURRENT FreeBSD 10.0-CURRENT #4 r239330: Thu Aug 16 22:08:12 MSK 2012     root at cbuilder64.my.domain:/usr/obj/usr/src/sys/GENERIC  amd64
>> Description:
> when the jail is created by config file with multiple ips, jail loses all ip assignment which sets after ips with /"network prefix"
> Nevertheless, all IP are established on the interface with the correct mask
>> How-To-Repeat:
> Have jail config with multiple ips with prefix in the list. Config sample (we mean that in /usr/jails/jail1 we have a complete freebsd base environment):
>
> % cat jail1.conf
> jail1 {
> exec.start = "/bin/sh /etc/rc";
> exec.stop = "/bin/sh /etc/rc.shutdown";
> exec.clean;
> #mount.devfs;
> host.hostname = "jail1.my.domain";
> path = "/usr/jails/jail1";
> allow.raw_sockets;
> allow.socket_af;
> allow.chflags;
> allow.sysvipc;
> ip4.addr = 10.0.0.1,10.0.0.2,172.17.0.0/16,10.0.0.3;
> interface="em0";
> mount.devfs;
> devfs_ruleset="4";
> }
>
> // Before jail creation, interfaces em0 have:
> % ifconfig em0 | grep "inet "
>          inet 192.168.1.2 netmask 0xffffff00 broadcast 192.168.1.255
>
> // Jail start:
> % jail -f jail1.conf -c jail1
> jail1: created
> ..
>
> //done. Check for IPs on interface. 172.17.0.0 have correct mask:
> ifconfig em0 | grep "inet "
>          inet 192.168.1.2 netmask 0xffffff00 broadcast 192.168.1.255
>          inet 10.0.0.1 netmask 0xffffffff broadcast 10.0.0.1
>          inet 10.0.0.2 netmask 0xffffffff broadcast 10.0.0.2
>          inet 172.17.0.0 netmask 0xffff0000 broadcast 172.17.255.255
>          inet 10.0.0.3 netmask 0xffffffff broadcast 10.0.0.3
>
> //Check for IP in jls:
> % jls -v
>     JID  Hostname                      Path
>          Name                          State
>          CPUSetID
>          IP Address(es)
>       1  jail1.my.domain               /usr/jails/jail1
>          jail1                         ACTIVE
>          2
>          10.0.0.1
>          10.0.0.2
>          172.17.0.0
>
> We have 10.0.0.1/32,10.0.0.2/32 and 172.17.0.0/16 but loose 10.0.0.3

I confused myself on the difference between null-terminated and
length-defined strings, and stuck a '\0' were it didn't belong.
I've committed the fix to HEAD, and I'll have it in 9.1 next week.
I'm also including it here for easy consumption :-).

- Jamie
-------------- next part --------------
Index: usr.sbin/jail/config.c
===================================================================
--- usr.sbin/jail/config.c	(revision 239600)
+++ usr.sbin/jail/config.c	(revision 239601)
@@ -597,8 +597,7 @@
 					    "ip4.addr: bad netmask \"%s\"", cs);
 					error = -1;	
 				}
-				*cs = '\0';
-				s->len = cs - s->s + 1;
+				s->len = cs - s->s;
 			}
 		}
 	}
@@ -621,8 +620,7 @@
 					    cs);
 					error = -1;	
 				}
-				*cs = '\0';
-				s->len = cs - s->s + 1;
+				s->len = cs - s->s;
 			}
 		}
 	}
@@ -714,7 +712,7 @@
 			value = alloca(vallen);
 			cs = value;
 			TAILQ_FOREACH_SAFE(s, &p->val, tq, ts) {
-				strcpy(cs, s->s);
+				memcpy(cs, s->s, s->len);
 				if (ts != NULL) {
 					cs += s->len + 1;
 					cs[-1] = ',';