[CRIU] [PATCH 1/2] unix: a unix name can be a non-null terminated string

[Andrei Vagin]

On Fri, Mar 31, 2017 at 12:33:54PM +0300, Pavel Emelyanov wrote:
> On 03/30/2017 10:00 PM, Andrei Vagin wrote:
> > From: Andrei Vagin <avagin at virtuozzo.com>
> > 
> > In this patch, we replace all zero characters to '@'.
> 
> Do we save thus modified string into image?

no, we don't

> 
> > ==30==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60300000e3ca at pc 0x7f34144b6be1 bp 0x7ffee7b6bb20 sp 0x7ffee7b6b298
> > READ of size 26 at 0x60300000e3ca thread T0
> >     #0 0x7f34144b6be0  (/lib64/libasan.so.3+0x8dbe0)
> >     #1 0x7f34144b8e4d in __interceptor_vsnprintf (/lib64/libasan.so.3+0x8fe4d)
> >     #2 0x4966cb in vprint_on_level criu/log.c:228
> >     #3 0x496b64 in print_on_level criu/log.c:249
> >     #4 0x505c94 in collect_one_unixsk criu/sk-unix.c:1401
> >     #5 0x4e7ae3 in collect_image criu/protobuf.c:213
> >     #6 0x462c5c in root_prepare_shared criu/cr-restore.c:247
> >     #7 0x462c5c in restore_task_with_children criu/cr-restore.c:1420
> >     #8 0x7f34132d70ec in __clone (/lib64/libc.so.6+0x1030ec)
> > 
> > 0x60300000e3ca is located 0 bytes to the right of 26-byte region [0x60300000e3b0,0x60300000e3ca)
> > allocated by thread T0 here:
> >     #0 0x7f34144efe70 in malloc (/lib64/libasan.so.3+0xc6e70)
> >     #1 0x7f3413bdb021  (/lib64/libprotobuf-c.so.1+0x6021)
> > 
> > Signed-off-by: Andrei Vagin <avagin at virtuozzo.com>
> > ---
> >  criu/sk-unix.c | 31 +++++++++++++++++++++++++++++--
> >  1 file changed, 29 insertions(+), 2 deletions(-)
> > 
> > diff --git a/criu/sk-unix.c b/criu/sk-unix.c
> > index 2291fde..f8b884c 100644
> > --- a/criu/sk-unix.c
> > +++ b/criu/sk-unix.c
> > @@ -977,6 +977,9 @@ static int bind_unix_sk(int sk, struct unix_sk_info *ui)
> >  	int cwd_fd = -1, root_fd = -1;
> >  	int ret = -1;
> >  
> > +	if (ui->ue->name.len == 0)
> > +		return 0;
> > +
> >  	if ((ui->ue->type == SOCK_STREAM) && (ui->ue->state == TCP_ESTABLISHED)) {
> >  		/*
> >  		 * FIXME this can be done, but for doing this properly we
> > @@ -1369,6 +1372,8 @@ static int collect_one_unixsk(void *o, ProtobufCMessage *base, struct cr_img *i)
> >  {
> >  	struct unix_sk_info *ui = o;
> >  	static bool post_queued = false;
> > +	char *uname, *prefix = "";
> > +	int ulen;
> >  
> >  	ui->ue = pb_msg(base, UnixSkEntry);
> >  	ui->name_dir = (void *)ui->ue->name_dir;
> > @@ -1398,10 +1403,32 @@ static int collect_one_unixsk(void *o, ProtobufCMessage *base, struct cr_img *i)
> >  	INIT_LIST_HEAD(&ui->connected);
> >  	INIT_LIST_HEAD(&ui->node);
> >  	ui->flags = 0;
> > -	pr_info(" `- Got %#x peer %#x (name %s dir %s)\n",
> > +
> > +	uname = ui->name;
> > +	ulen = ui->ue->name.len;
> > +	if (ulen > 0 && uname[0] == 0) {
> > +		prefix = "@";
> > +		uname++;
> > +		ulen--;
> > +		if (strlen(uname) != ulen) {
> > +			/* replace zero characters */
> > +			char *s = alloca(ulen + 1);
> > +			int i;
> > +
> > +			for (i = 0; i < ulen; i++)
> > +				s[i] = uname[i] ? : '@';
> > +			uname = s;
> > +		}
> > +	} else if (ulen == 0) {
> > +		ulen = 1;
> > +		uname = "-";
> > +	}
> > +
> > +	pr_info(" `- Got %#x peer %#x (name %s%.*s dir %s)\n",
> >  		ui->ue->ino, ui->ue->peer,
> > -		ui->name ? (ui->name[0] ? ui->name : &ui->name[1]) : "-",
> > +		prefix, ulen, uname,
> >  		ui->name_dir ? ui->name_dir : "-");
> > +
> >  	list_add_tail(&ui->list, &unix_sockets);
> >  	return file_desc_add(&ui->d, ui->ue->id, &unix_desc_ops);
> >  }
> > 
>