Quoting Dan Smith (danms@xxxxxxxxxx): > Fix the potential for breakage if our UTS changes during checkpoint > by grabbing uts_sem and copying those strings to temporary buffers. > > Cc: orenl@xxxxxxxxxxxxxxx > Signed-off-by: Dan Smith <danms@xxxxxxxxxx> Looks good. The only thing I'd add is that you are depending on __NEW_UTS_LEN+1 being something very specific, so if utsname.h gets a change, checkpoint/ckpt_task.c needs a corresponding change. So it would be robust to future code changes if you #define MAX_UTS_LEN (__NEW_UTS_LEN+1) in utsname.h, so that anyone expanding the size of hostname doesn't need to look for this usage. Still, Acked-by: Serge Hallyn <serue@xxxxxxxxxx> -serge > Changes in v2: > - Be less stupid about holding the system-wide uts_sem during > checkpoint (!) > - Don't hold it during restart > - Hold uts_sem only while copying out the strings > - Calculate the length of the saved buffers outside of the semaphore and > avoid the duplicate _len variables by cleaning up the cr_hdr_utsns > after the cr_write_string() of the two buffers > --- > checkpoint/ckpt_task.c | 25 ++++++++++++++----------- > 1 files changed, 14 insertions(+), 11 deletions(-) > > diff --git a/checkpoint/ckpt_task.c b/checkpoint/ckpt_task.c > index 4d19e31..30858d2 100644 > --- a/checkpoint/ckpt_task.c > +++ b/checkpoint/ckpt_task.c > @@ -171,8 +171,8 @@ static int cr_write_utsns(struct cr_ctx *ctx, struct uts_namespace *uts_ns) > { > struct cr_hdr h; > struct cr_hdr_utsns *hh; > - int domainname_len; > - int nodename_len; > + char nodename[__NEW_UTS_LEN + 1]; > + char domainname[__NEW_UTS_LEN + 1]; > int ret; > > h.type = CR_HDR_UTSNS; > @@ -182,22 +182,25 @@ static int cr_write_utsns(struct cr_ctx *ctx, struct uts_namespace *uts_ns) > if (!hh) > return -ENOMEM; > > - nodename_len = strlen(uts_ns->name.nodename) + 1; > - domainname_len = strlen(uts_ns->name.domainname) + 1; > + down_read(&uts_sem); > + memcpy(nodename, uts_ns->name.nodename, sizeof(nodename)); > + memcpy(domainname, uts_ns->name.domainname, sizeof(nodename)); > + up_read(&uts_sem); > > - hh->nodename_len = nodename_len; > - hh->domainname_len = domainname_len; > + hh->nodename_len = strlen(nodename) + 1; > + hh->domainname_len = strlen(domainname) + 1; > > ret = cr_write_obj(ctx, &h, hh); > - cr_hbuf_put(ctx, sizeof(*hh)); > if (ret < 0) > - return ret; > + goto out; > > - ret = cr_write_string(ctx, uts_ns->name.nodename, nodename_len); > + ret = cr_write_string(ctx, nodename, hh->nodename_len); > if (ret < 0) > - return ret; > + goto out; > > - ret = cr_write_string(ctx, uts_ns->name.domainname, domainname_len); > + ret = cr_write_string(ctx, domainname, hh->domainname_len); > + out: > + cr_hbuf_put(ctx, sizeof(*hh)); > return ret; > } > > -- > 1.5.6.3 > > _______________________________________________ > Containers mailing list > Containers@xxxxxxxxxxxxxxxxxxxxxxxxxx > https://lists.linux-foundation.org/mailman/listinfo/containers _______________________________________________ Containers mailing list Containers@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linux-foundation.org/mailman/listinfo/containers