mbox series

[0/2] nvme: avoid dynamic stack allocations

Message ID 20230811174751.784620-1-peter.maydell@linaro.org
Headers show
Series nvme: avoid dynamic stack allocations | expand

Message

Peter Maydell Aug. 11, 2023, 5:47 p.m. UTC
The QEMU codebase has very few C variable length arrays, and if we can
get rid of them all we can make the compiler error on new additions.
This is a defensive measure against security bugs where an on-stack
dynamic allocation isn't correctly size-checked (e.g.  CVE-2021-3527).

We last had a go at this a few years ago, when Philippe wrote
patches for this:
https://patchew.org/QEMU/20210505211047.1496765-1-philmd@redhat.com/
Some of the fixes made it into the tree, but some didn't (either
because of lack of review or because review found some changes
that needed to be made). I'm going through the remainder as a
non-urgent Friday afternoon task...

This patchset deals with two VLAs in the NVME code.

thanks
-- PMM

Peter Maydell (1):
  hw/nvme: Avoid dynamic stack allocation

Philippe Mathieu-Daudé (1):
  hw/nvme: Use #define to avoid variable length array

 hw/nvme/ctrl.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Klaus Jensen Aug. 14, 2023, 7:09 a.m. UTC | #1
On Aug 11 18:47, Peter Maydell wrote:
> The QEMU codebase has very few C variable length arrays, and if we can
> get rid of them all we can make the compiler error on new additions.
> This is a defensive measure against security bugs where an on-stack
> dynamic allocation isn't correctly size-checked (e.g.  CVE-2021-3527).
> 
> We last had a go at this a few years ago, when Philippe wrote
> patches for this:
> https://patchew.org/QEMU/20210505211047.1496765-1-philmd@redhat.com/
> Some of the fixes made it into the tree, but some didn't (either
> because of lack of review or because review found some changes
> that needed to be made). I'm going through the remainder as a
> non-urgent Friday afternoon task...
> 
> This patchset deals with two VLAs in the NVME code.
> 
> thanks
> -- PMM
> 
> Peter Maydell (1):
>   hw/nvme: Avoid dynamic stack allocation
> 
> Philippe Mathieu-Daudé (1):
>   hw/nvme: Use #define to avoid variable length array
> 
>  hw/nvme/ctrl.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> -- 
> 2.34.1
> 

Thanks Peter,

Applied to nvme-next!
Philippe Mathieu-Daudé Aug. 16, 2023, 9:47 a.m. UTC | #2
On 11/8/23 19:47, Peter Maydell wrote:
> The QEMU codebase has very few C variable length arrays, and if we can
> get rid of them all we can make the compiler error on new additions.
> This is a defensive measure against security bugs where an on-stack
> dynamic allocation isn't correctly size-checked (e.g.  CVE-2021-3527).
> 
> We last had a go at this a few years ago, when Philippe wrote
> patches for this:
> https://patchew.org/QEMU/20210505211047.1496765-1-philmd@redhat.com/
> Some of the fixes made it into the tree, but some didn't (either
> because of lack of review or because review found some changes
> that needed to be made). I'm going through the remainder as a
> non-urgent Friday afternoon task...

Thanks for refreshing this, I totally forgot about it :/

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

> This patchset deals with two VLAs in the NVME code.
> 
> thanks
> -- PMM
> 
> Peter Maydell (1):
>    hw/nvme: Avoid dynamic stack allocation
> 
> Philippe Mathieu-Daudé (1):
>    hw/nvme: Use #define to avoid variable length array
> 
>   hw/nvme/ctrl.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
Peter Maydell Sept. 12, 2023, 2:15 p.m. UTC | #3
On Mon, 14 Aug 2023 at 08:09, Klaus Jensen <its@irrelevant.dk> wrote:
>
> On Aug 11 18:47, Peter Maydell wrote:
> > The QEMU codebase has very few C variable length arrays, and if we can
> > get rid of them all we can make the compiler error on new additions.
> > This is a defensive measure against security bugs where an on-stack
> > dynamic allocation isn't correctly size-checked (e.g.  CVE-2021-3527).
> >
> > We last had a go at this a few years ago, when Philippe wrote
> > patches for this:
> > https://patchew.org/QEMU/20210505211047.1496765-1-philmd@redhat.com/
> > Some of the fixes made it into the tree, but some didn't (either
> > because of lack of review or because review found some changes
> > that needed to be made). I'm going through the remainder as a
> > non-urgent Friday afternoon task...
> >
> > This patchset deals with two VLAs in the NVME code.
> >
> > thanks
> > -- PMM
> >
> > Peter Maydell (1):
> >   hw/nvme: Avoid dynamic stack allocation
> >
> > Philippe Mathieu-Daudé (1):
> >   hw/nvme: Use #define to avoid variable length array
> >
> >  hw/nvme/ctrl.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > --
> > 2.34.1
> >
>
> Thanks Peter,
>
> Applied to nvme-next!


Hi Klaus -- did these patches get lost? They don't seem to
have appeared in master yet.

thanks
-- PMM
Klaus Jensen Sept. 12, 2023, 2:19 p.m. UTC | #4
On Sep 12 15:15, Peter Maydell wrote:
> On Mon, 14 Aug 2023 at 08:09, Klaus Jensen <its@irrelevant.dk> wrote:
> >
> > On Aug 11 18:47, Peter Maydell wrote:
> > > The QEMU codebase has very few C variable length arrays, and if we can
> > > get rid of them all we can make the compiler error on new additions.
> > > This is a defensive measure against security bugs where an on-stack
> > > dynamic allocation isn't correctly size-checked (e.g.  CVE-2021-3527).
> > >
> > > We last had a go at this a few years ago, when Philippe wrote
> > > patches for this:
> > > https://patchew.org/QEMU/20210505211047.1496765-1-philmd@redhat.com/
> > > Some of the fixes made it into the tree, but some didn't (either
> > > because of lack of review or because review found some changes
> > > that needed to be made). I'm going through the remainder as a
> > > non-urgent Friday afternoon task...
> > >
> > > This patchset deals with two VLAs in the NVME code.
> > >
> > > thanks
> > > -- PMM
> > >
> > > Peter Maydell (1):
> > >   hw/nvme: Avoid dynamic stack allocation
> > >
> > > Philippe Mathieu-Daudé (1):
> > >   hw/nvme: Use #define to avoid variable length array
> > >
> > >  hw/nvme/ctrl.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > --
> > > 2.34.1
> > >
> >
> > Thanks Peter,
> >
> > Applied to nvme-next!
> 
> 
> Hi Klaus -- did these patches get lost? They don't seem to
> have appeared in master yet.
> 
> thanks
> -- PMM

Oh. I never sent the pull - I'll do that right away! Thanks for the
reminder!