[Devel] Re: [PATCH 1/1] cr: uts: don't pass an unsigned var as a signed int

[Nathan Lynch]

"Serge E. Hallyn" <serge at hallyn.com> writes:

> Quoting Nathan Lynch (ntl at pobox.com):
>> "Serge E. Hallyn" <serue at us.ibm.com> writes:
>> 
>> > Else my checkpoing image gets reeeaallly huge.  Just passing the
>> > result of sizeof() however does the right thing.
>> >
>> > Signed-off-by: Serge E. Hallyn <serue at us.ibm.com>
>> > ---
>> >  checkpoint/namespace.c |   12 ++++++------
>> >  1 files changed, 6 insertions(+), 6 deletions(-)
>> 
>> But right above the code you're changing we have:
>> 
>> 	h->sysname_len = sizeof(name->sysname);
>> 	h->nodename_len = sizeof(name->nodename);
>> 	h->release_len = sizeof(name->release);
>> 	h->version_len = sizeof(name->version);
>> 	h->machine_len = sizeof(name->machine);
>> 	h->domainname_len = sizeof(name->domainname);
>> 
>> Your patch shouldn't change any behavior.  What gives?
>
> "Shouldn't", perhaps, but does.


Revisiting do_checkpoint_uts_ns, I think it's a case of use after free:

	h = ckpt_hdr_get_type(ctx, sizeof(*h), CKPT_HDR_UTS_NS);
	if (!h)
		return -ENOMEM;

	h->sysname_len = sizeof(name->sysname);
	h->nodename_len = sizeof(name->nodename);
	h->release_len = sizeof(name->release);
	h->version_len = sizeof(name->version);
	h->machine_len = sizeof(name->machine);
	h->domainname_len = sizeof(name->domainname);

	ret = ckpt_write_obj(ctx, &h->h);
	ckpt_hdr_put(ctx, h);
	if (ret < 0)
		return ret;

	down_read(&uts_sem);
	ret = ckpt_write_string(ctx, name->sysname, h->sysname_len);

We're continuing to use h's memory after it has been released by
ckpt_hdr_put.  Seems plausible that the poison values written by sl*b
debug would cause the len argument to be ridiculously large.

_______________________________________________
Containers mailing list
Containers at lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers