On Tue, Sep 11, 2018 at 11:25:52AM -0700, Ori Bernstein wrote:
> On Tue, 11 Sep 2018 15:36:49 +0800
> Michael Mikonos <m...@ii.net> wrote:
>
> > Hello,
> >
> > Sometimes vmd doesn't seem to check the result of malloc/calloc.
> > I tried to preserve the existing behavour w.r.t. return values
> > for the functions modified; some functions returned 1 on error
> > while others return -1. Does this look correct?
> >
> > - Michael
> >
> >
>
> > Index: vioqcow2.c
> > ===================================================================
> > RCS file: /cvs/src/usr.sbin/vmd/vioqcow2.c,v
> > retrieving revision 1.2
> > diff -u -p -u -r1.2 vioqcow2.c
> > --- vioqcow2.c 11 Sep 2018 04:06:32 -0000 1.2
> > +++ vioqcow2.c 11 Sep 2018 07:29:10 -0000
> > @@ -202,6 +202,9 @@ qc2_open(struct qcdisk *disk, int fd)
> > }
> >
> > disk->l1 = calloc(disk->l1sz, sizeof *disk->l1);
> > + if (disk->l1 == NULL)
> > + return -1;
> > +
> > if (pread(disk->fd, (char*)disk->l1, 8*disk->l1sz, disk->l1off)
> > != 8*disk->l1sz) {
> > free(disk->l1);
> > @@ -237,6 +240,8 @@ qc2_open(struct qcdisk *disk, int fd)
> > basepath[backingsz] = 0;
> >
> > disk->base = calloc(1, sizeof(struct qcdisk));
> > + if (disk->base == NULL)
> > + return -1;
>
> This early return leaks disk->l1. The other vioqcow2/vioraw changes
> look fine to me.
Thanks for the feedback.
The other eary returns which currently free disk->base don't free
disk->l1. I can change those error cases, and this calloc() one, to free
disk->l1 if that's what is intended.
>
> > if (qc2_openpath(disk->base, basepath, O_RDONLY) == -1) {
> > free(disk->base);
> > return -1;
> > Index: vioraw.c
> > ===================================================================
> > RCS file: /cvs/src/usr.sbin/vmd/vioraw.c,v
> > retrieving revision 1.1
> > diff -u -p -u -r1.1 vioraw.c
> > --- vioraw.c 25 Aug 2018 04:16:09 -0000 1.1
> > +++ vioraw.c 11 Sep 2018 07:29:10 -0000
> > @@ -62,6 +62,8 @@ virtio_init_raw(struct virtio_backing *f
> > return -1;
> >
> > fdp = malloc(sizeof(int));
> > + if (fdp == NULL)
> > + return -1;
> > *fdp = fd;
> > file->p = fdp;
> > file->pread = raw_pread;
> >
>
>
> --
> Ori Bernstein <o...@eigenstate.org>
>
