Eric Blake
2022-Feb-10 14:50 UTC
[Libguestfs] [libnbd PATCH 3/3] api: Add new API nbd_set_pread_initialize()
On Thu, Feb 10, 2022 at 09:38:30AM +0000, Richard W.M. Jones wrote:> On Wed, Feb 09, 2022 at 04:07:26PM -0600, Eric Blake wrote: > > + "set_pread_initialize", { > > + default_call with > > + args = [Bool "request"]; ret = RErr; > > + shortdesc = "control whether libnbd pre-initializes read buffers"; > > + longdesc = "\ > > +By default, libnbd will pre-initialize the contents of a buffer > > +passed to calls such as L<nbd_pread(3)> to all zeroes prior to checking > > +for any other errors, so that even if a client application passed in an > > +uninitialized buffer but fails to check for errors, it will not result > > +in a potential security risk caused by an accidental leak of prior heap > > +contents. However, for a client application that has audited that an > > +uninitialized buffer is never dereferenced, or which performs its own > > +pre-initialization, libnbd's sanitization efforts merely pessimize > > +performance. > > + > > +Calling this function with C<request> set to false tells libnbd to > > +skip the buffer initialization step in read commands."; > > + see_also = [Link "get_pread_initialize"; > > + Link "set_strict_mode"; > > + Link "pread"; Link "pread_structured"; Link "aio_pread"; > > + Link "aio_pread_structured"]; > > + }; > > Could it be worth mentioning CVE-2022-0485 by name in the text here? > And/or linking to: > https://listman.redhat.com/archives/libguestfs/2022-February/msg00104.htmlGood idea, I went with: diff --git i/generator/API.ml w/generator/API.ml index 9b7eb545..aaba9951 100644 --- i/generator/API.ml +++ w/generator/API.ml @@ -784,14 +784,19 @@ "set_pread_initialize", { shortdesc = "control whether libnbd pre-initializes read buffers"; longdesc = "\ By default, libnbd will pre-initialize the contents of a buffer -passed to calls such as L<nbd_pread(3)> to all zeroes prior to checking -for any other errors, so that even if a client application passed in an -uninitialized buffer but fails to check for errors, it will not result -in a potential security risk caused by an accidental leak of prior heap -contents. However, for a client application that has audited that an -uninitialized buffer is never dereferenced, or which performs its own -pre-initialization, libnbd's sanitization efforts merely pessimize -performance. +passed to calls such as L<nbd_pread(3)> to all zeroes prior to +checking for any other errors, so that even if a client application +passed in an uninitialized buffer but fails to check for errors, it +will not result in a potential security risk caused by an accidental +leak of prior heap contents (see CVE-2022-0485 in +L<libnbd-security(3)> for an example of a security hole in an +application built against an earlier version of libnbd that lacked +consistent pre-initialization). However, for a client application +that has audited that an uninitialized buffer is never dereferenced, +or which performs its own pre-initialization, libnbd's sanitization +efforts merely pessimize performance (although the time spent in +pre-initialization may pale in comparison to time spent waiting on +network packets). Calling this function with C<request> set to false tells libnbd to skip the buffer initialization step in read commands.";> > Anyway the whole patch series looks good, so: > > Reviewed-by: Richard W.M. Jones <rjones at redhat.com>Series is now checked in with amendments, as ab52914b..e0953cb7 -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Richard W.M. Jones
2022-Feb-10 15:09 UTC
[Libguestfs] [libnbd PATCH 3/3] api: Add new API nbd_set_pread_initialize()
On Thu, Feb 10, 2022 at 08:50:07AM -0600, Eric Blake wrote:> On Thu, Feb 10, 2022 at 09:38:30AM +0000, Richard W.M. Jones wrote: > > On Wed, Feb 09, 2022 at 04:07:26PM -0600, Eric Blake wrote: > > > + "set_pread_initialize", { > > > + default_call with > > > + args = [Bool "request"]; ret = RErr; > > > + shortdesc = "control whether libnbd pre-initializes read buffers"; > > > + longdesc = "\ > > > +By default, libnbd will pre-initialize the contents of a buffer > > > +passed to calls such as L<nbd_pread(3)> to all zeroes prior to checking > > > +for any other errors, so that even if a client application passed in an > > > +uninitialized buffer but fails to check for errors, it will not result > > > +in a potential security risk caused by an accidental leak of prior heap > > > +contents. However, for a client application that has audited that an > > > +uninitialized buffer is never dereferenced, or which performs its own > > > +pre-initialization, libnbd's sanitization efforts merely pessimize > > > +performance. > > > + > > > +Calling this function with C<request> set to false tells libnbd to > > > +skip the buffer initialization step in read commands."; > > > + see_also = [Link "get_pread_initialize"; > > > + Link "set_strict_mode"; > > > + Link "pread"; Link "pread_structured"; Link "aio_pread"; > > > + Link "aio_pread_structured"]; > > > + }; > > > > Could it be worth mentioning CVE-2022-0485 by name in the text here? > > And/or linking to: > > https://listman.redhat.com/archives/libguestfs/2022-February/msg00104.html > > Good idea, I went with: > > diff --git i/generator/API.ml w/generator/API.ml > index 9b7eb545..aaba9951 100644 > --- i/generator/API.ml > +++ w/generator/API.ml > @@ -784,14 +784,19 @@ "set_pread_initialize", { > shortdesc = "control whether libnbd pre-initializes read buffers"; > longdesc = "\ > By default, libnbd will pre-initialize the contents of a buffer > -passed to calls such as L<nbd_pread(3)> to all zeroes prior to checking > -for any other errors, so that even if a client application passed in an > -uninitialized buffer but fails to check for errors, it will not result > -in a potential security risk caused by an accidental leak of prior heap > -contents. However, for a client application that has audited that an > -uninitialized buffer is never dereferenced, or which performs its own > -pre-initialization, libnbd's sanitization efforts merely pessimize > -performance. > +passed to calls such as L<nbd_pread(3)> to all zeroes prior to > +checking for any other errors, so that even if a client application > +passed in an uninitialized buffer but fails to check for errors, it > +will not result in a potential security risk caused by an accidental > +leak of prior heap contents (see CVE-2022-0485 in > +L<libnbd-security(3)> for an example of a security hole in an > +application built against an earlier version of libnbd that lacked > +consistent pre-initialization). However, for a client application > +that has audited that an uninitialized buffer is never dereferenced, > +or which performs its own pre-initialization, libnbd's sanitization > +efforts merely pessimize performance (although the time spent in > +pre-initialization may pale in comparison to time spent waiting on > +network packets). > > Calling this function with C<request> set to false tells libnbd to > skip the buffer initialization step in read commands."; > > > > > > Anyway the whole patch series looks good, so: > > > > Reviewed-by: Richard W.M. Jones <rjones at redhat.com> > > Series is now checked in with amendments, as ab52914b..e0953cb7Thanks. For Fedora, I won't do anything now, just let this go out in the next release. It'll need a small adjustment if we backport it to the stable-1.10 branch. For RHEL, I'm going to put this in RHEL 9.0, but won't bother with RHEL 8 (it has ancient history libnbd 1.6). Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://people.redhat.com/~rjones/virt-df/