| Message ID | 20220628152429.286-1-schspa@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] vfio: Clear the caps->buf to NULL after free | expand |
On Tue, 28 Jun 2022 23:24:29 +0800 Schspa Shi <schspa@gmail.com> wrote: > API vfio_info_cap_add will free caps->buf, clear it to NULL after > free. Should this be something like: On buffer resize failure, vfio_info_cap_add() will free the buffer, report zero for the size, and return -ENOMEM. As additional hardening, also clear the buffer pointer to prevent any chance of a double free. Thanks, Alex > Signed-off-by: Schspa Shi <schspa@gmail.com> > --- > drivers/vfio/vfio.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c > index 61e71c1154be..a0fb93866f61 100644 > --- a/drivers/vfio/vfio.c > +++ b/drivers/vfio/vfio.c > @@ -1812,6 +1812,7 @@ struct vfio_info_cap_header *vfio_info_cap_add(struct vfio_info_cap *caps, > buf = krealloc(caps->buf, caps->size + size, GFP_KERNEL); > if (!buf) { > kfree(caps->buf); > + caps->buf = NULL; > caps->size = 0; > return ERR_PTR(-ENOMEM); > }
On Tue, Jun 28 2022, Alex Williamson <alex.williamson@redhat.com> wrote: > On Tue, 28 Jun 2022 23:24:29 +0800 > Schspa Shi <schspa@gmail.com> wrote: > >> API vfio_info_cap_add will free caps->buf, clear it to NULL after >> free. > > Should this be something like: > > On buffer resize failure, vfio_info_cap_add() will free the buffer, > report zero for the size, and return -ENOMEM. As additional > hardening, also clear the buffer pointer to prevent any chance of a > double free. I like that better. With that, Reviewed-by: Cornelia Huck <cohuck@redhat.com> > > Thanks, > Alex > >> Signed-off-by: Schspa Shi <schspa@gmail.com> >> --- >> drivers/vfio/vfio.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c >> index 61e71c1154be..a0fb93866f61 100644 >> --- a/drivers/vfio/vfio.c >> +++ b/drivers/vfio/vfio.c >> @@ -1812,6 +1812,7 @@ struct vfio_info_cap_header *vfio_info_cap_add(struct vfio_info_cap *caps, >> buf = krealloc(caps->buf, caps->size + size, GFP_KERNEL); >> if (!buf) { >> kfree(caps->buf); >> + caps->buf = NULL; >> caps->size = 0; >> return ERR_PTR(-ENOMEM); >> }
Cornelia Huck <cohuck@redhat.com> writes: > On Tue, Jun 28 2022, Alex Williamson > <alex.williamson@redhat.com> wrote: > >> On Tue, 28 Jun 2022 23:24:29 +0800 >> Schspa Shi <schspa@gmail.com> wrote: >> >>> API vfio_info_cap_add will free caps->buf, clear it to NULL >>> after >>> free. >> >> Should this be something like: >> >> On buffer resize failure, vfio_info_cap_add() will free the >> buffer, >> report zero for the size, and return -ENOMEM. As >> additional >> hardening, also clear the buffer pointer to prevent any >> chance of a >> double free. > > I like that better. With that, > > Reviewed-by: Cornelia Huck <cohuck@redhat.com> > I have send a V3 patch for this comment change, please review it. >> >> Thanks, >> Alex >> >>> Signed-off-by: Schspa Shi <schspa@gmail.com> >>> --- >>> drivers/vfio/vfio.c | 1 + >>> 1 file changed, 1 insertion(+) >>> >>> diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c >>> index 61e71c1154be..a0fb93866f61 100644 >>> --- a/drivers/vfio/vfio.c >>> +++ b/drivers/vfio/vfio.c >>> @@ -1812,6 +1812,7 @@ struct vfio_info_cap_header >>> *vfio_info_cap_add(struct vfio_info_cap *caps, >>> buf = krealloc(caps->buf, caps->size + size, GFP_KERNEL); >>> if (!buf) { >>> kfree(caps->buf); >>> + caps->buf = NULL; >>> caps->size = 0; >>> return ERR_PTR(-ENOMEM); >>> }
diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c index 61e71c1154be..a0fb93866f61 100644 --- a/drivers/vfio/vfio.c +++ b/drivers/vfio/vfio.c @@ -1812,6 +1812,7 @@ struct vfio_info_cap_header *vfio_info_cap_add(struct vfio_info_cap *caps, buf = krealloc(caps->buf, caps->size + size, GFP_KERNEL); if (!buf) { kfree(caps->buf); + caps->buf = NULL; caps->size = 0; return ERR_PTR(-ENOMEM); }
API vfio_info_cap_add will free caps->buf, clear it to NULL after free. Signed-off-by: Schspa Shi <schspa@gmail.com> --- drivers/vfio/vfio.c | 1 + 1 file changed, 1 insertion(+)