[lxc-devel] [PATCH] Do not use strlen() on non-null terminated buffer

[Christian Brauner]

On Tue, Sep 08, 2015 at 02:36:20AM +0000, Serge Hallyn wrote:
> Quoting Christian Brauner (christianvanbrauner at gmail.com):
> > Signed-off-by: Christian Brauner <christianvanbrauner at gmail.com>
> 
> Thanks, this looks good, but I'd like to give it another
> look with a fresh pair of eyes in the morning.
> 
> (and maybe write a testcase)

Thanks, Serge! If this solution does not satisfy in the end I have another
solution available which I coded before deciding for the mmap()-based solution:
Using read() + calloc() to read in the whole file, then removing the line from
the buffer using strstr() and writing the buffer back to the file with write().
In this case we wouldn't need to fiddle with non-null terminated buffers.

Christian

> 
> > ---
> >  src/lxc/lxccontainer.c | 31 +++++++++++++++++--------------
> >  1 file changed, 17 insertions(+), 14 deletions(-)
> > 
> > diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c
> > index 932d658..fb99892 100644
> > --- a/src/lxc/lxccontainer.c
> > +++ b/src/lxc/lxccontainer.c
> > @@ -1989,7 +1989,7 @@ static bool mod_rdep(struct lxc_container *c0, struct lxc_container *c, bool inc
> >  	char newpath[MAXPATHLEN];
> >  	int fd, ret, n = 0, v = 0;
> >  	bool bret = false;
> > -	size_t len;
> > +	size_t len, difflen;
> >  
> >  	if (container_disk_lock(c0))
> >  		return false;
> > @@ -2072,19 +2072,22 @@ static bool mod_rdep(struct lxc_container *c0, struct lxc_container *c, bool inc
> >  
> >  			/* mmap()ed memory is only \0-terminated when it is not
> >  			 * a multiple of a pagesize. Hence, we'll use memmem(). */
> > -			if ((del = memmem(buf, fbuf.st_size, newpath, len))) {
> > -				/* remove container entry */
> > -				memmove(del, del + len, strlen(del) - len + 1);
> > -
> > -				munmap(buf, fbuf.st_size);
> > -
> > -				if (ftruncate(fd, fbuf.st_size - len) < 0) {
> > -					SYSERROR("Failed to truncate file %s", path);
> > -					close(fd);
> > -					goto out;
> > -				}
> > -			} else {
> > -				munmap(buf, fbuf.st_size);
> > +                        if ((del = memmem(buf, fbuf.st_size, newpath, len))) {
> > +                                /* remove container entry */
> > +                                if (del != buf + fbuf.st_size - len) {
> > +                                        difflen = fbuf.st_size - (del-buf);
> > +                                        memmove(del, del + len, strnlen(del, difflen) - len);
> > +                                }
> > +
> > +                                munmap(buf, fbuf.st_size);
> > +
> > +                                if (ftruncate(fd, fbuf.st_size - len) < 0) {
> > +                                        SYSERROR("Failed to truncate file %s", path);
> > +                                        close(fd);
> > +                                        goto out;
> > +                                }
> > +                        } else {
> > +                                munmap(buf, fbuf.st_size);
> >  			}
> >  
> >  			close(fd);
> > -- 
> > 2.5.1
> > 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20150908/0d28c5f5/attachment.sig>