[PATCH 0/4] net: avoid variable length arrays

[PATCH 0/4] net: avoid variable length arrays

[Peter Maydell]

This patchset removes the use of variable length arrays in a couple
of network devices and the net/ core code.  In one case we can switch
to a fixed-sized array on the stack; in the other three we have to
use a heap allocation.

The codebase has very few VLAs, 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).

Philippe had a go at these in  a patch in 2021:
https://patchew.org/QEMU/20210505211047.1496765-1-philmd@redhat.com/20210505211047.1496765-16-philmd@redhat.com/
but these are re-implementations, mostly.

Usual disclaimer: I have tested these patches only with
"make check" and "make check-avocado".

thanks
-- PMM

Peter Maydell (4):
  hw/net/fsl_etsec/rings.c: Avoid variable length array
  hw/net/rocker: Avoid variable length array
  net/dump: Avoid variable length array
  net/tap: Avoid variable-length array

 hw/net/fsl_etsec/rings.c      | 12 ++++++++++--
 hw/net/rocker/rocker_of_dpa.c |  2 +-
 net/dump.c                    |  2 +-
 net/tap.c                     |  3 ++-
 4 files changed, 14 insertions(+), 5 deletions(-)

-- 
2.34.1

[PATCH 1/4] hw/net/fsl_etsec/rings.c: Avoid variable length array

[Peter Maydell]

In fill_rx_bd() we create a variable length array of size
etsec->rx_padding. In fact we know that this will never be
larger than 64 bytes, because rx_padding is set in rx_init_frame()
in a way that ensures it is only that large. Use a fixed sized
array and assert that it is big enough.

Since padd[] is now potentially rather larger than the actual
padding required, adjust the memset() we do on it to match the
size that we write with cpu_physical_memory_write(), rather than
clearing the entire array.

The codebase has very few VLAs, 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).

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/net/fsl_etsec/rings.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/hw/net/fsl_etsec/rings.c b/hw/net/fsl_etsec/rings.c
index 788463f1b62..2f2f359f7a5 100644
--- a/hw/net/fsl_etsec/rings.c
+++ b/hw/net/fsl_etsec/rings.c
@@ -372,6 +372,12 @@ void etsec_walk_tx_ring(eTSEC *etsec, int ring_nbr)
     etsec->regs[TSTAT].value |= 1 << (31 - ring_nbr);
 }
 
+/*
+ * rx_init_frame() ensures we never do more padding than this
+ * (checksum plus minimum data packet size)
+ */
+#define MAX_RX_PADDING 64
+
 static void fill_rx_bd(eTSEC          *etsec,
                        eTSEC_rxtx_bd  *bd,
                        const uint8_t **buf,
@@ -380,9 +386,11 @@ static void fill_rx_bd(eTSEC          *etsec,
     uint16_t to_write;
     hwaddr   bufptr = bd->bufptr +
         ((hwaddr)(etsec->regs[TBDBPH].value & 0xF) << 32);
-    uint8_t  padd[etsec->rx_padding];
+    uint8_t  padd[MAX_RX_PADDING];
     uint8_t  rem;
 
+    assert(etsec->rx_padding <= MAX_RX_PADDING);
+
     RING_DEBUG("eTSEC fill Rx buffer @ 0x%016" HWADDR_PRIx
                " size:%zu(padding + crc:%u) + fcb:%u\n",
                bufptr, *size, etsec->rx_padding, etsec->rx_fcb_size);
@@ -426,7 +434,7 @@ static void fill_rx_bd(eTSEC          *etsec,
         rem = MIN(etsec->regs[MRBLR].value - bd->length, etsec->rx_padding);
 
         if (rem > 0) {
-            memset(padd, 0x0, sizeof(padd));
+            memset(padd, 0x0, rem);
             etsec->rx_padding -= rem;
             *size             -= rem;
             bd->length        += rem;
-- 
2.34.1

Re: [PATCH 1/4] hw/net/fsl_etsec/rings.c: Avoid variable length array

[Philippe Mathieu-Daudé]

On 24/8/23 17:32, Peter Maydell wrote:
> In fill_rx_bd() we create a variable length array of size
> etsec->rx_padding. In fact we know that this will never be
> larger than 64 bytes, because rx_padding is set in rx_init_frame()
> in a way that ensures it is only that large. Use a fixed sized
> array and assert that it is big enough.
> 
> Since padd[] is now potentially rather larger than the actual
> padding required, adjust the memset() we do on it to match the
> size that we write with cpu_physical_memory_write(), rather than
> clearing the entire array.
> 
> The codebase has very few VLAs, 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).
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   hw/net/fsl_etsec/rings.c | 12 ++++++++++--
>   1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/net/fsl_etsec/rings.c b/hw/net/fsl_etsec/rings.c
> index 788463f1b62..2f2f359f7a5 100644
> --- a/hw/net/fsl_etsec/rings.c
> +++ b/hw/net/fsl_etsec/rings.c
> @@ -372,6 +372,12 @@ void etsec_walk_tx_ring(eTSEC *etsec, int ring_nbr)
>       etsec->regs[TSTAT].value |= 1 << (31 - ring_nbr);
>   }
>   
> +/*
> + * rx_init_frame() ensures we never do more padding than this
> + * (checksum plus minimum data packet size)
> + */
> +#define MAX_RX_PADDING 64
> +
>   static void fill_rx_bd(eTSEC          *etsec,
>                          eTSEC_rxtx_bd  *bd,
>                          const uint8_t **buf,
> @@ -380,9 +386,11 @@ static void fill_rx_bd(eTSEC          *etsec,
>       uint16_t to_write;
>       hwaddr   bufptr = bd->bufptr +
>           ((hwaddr)(etsec->regs[TBDBPH].value & 0xF) << 32);
> -    uint8_t  padd[etsec->rx_padding];
> +    uint8_t  padd[MAX_RX_PADDING];
>       uint8_t  rem;
>   
> +    assert(etsec->rx_padding <= MAX_RX_PADDING);
> +
>       RING_DEBUG("eTSEC fill Rx buffer @ 0x%016" HWADDR_PRIx
>                  " size:%zu(padding + crc:%u) + fcb:%u\n",
>                  bufptr, *size, etsec->rx_padding, etsec->rx_fcb_size);
> @@ -426,7 +434,7 @@ static void fill_rx_bd(eTSEC          *etsec,
>           rem = MIN(etsec->regs[MRBLR].value - bd->length, etsec->rx_padding);
>   
>           if (rem > 0) {
> -            memset(padd, 0x0, sizeof(padd));
> +            memset(padd, 0x0, rem);
>               etsec->rx_padding -= rem;
>               *size             -= rem;
>               bd->length        += rem;

Maybe we can add this for clarity:

@@ -468,6 +468,6 @@ static void rx_init_frame(eTSEC *etsec, const 
uint8_t *buf, size_t size)
       * minimum MTU size bytes long (64)
       */
-    if (etsec->rx_buffer_len < 60) {
-        etsec->rx_padding += 60 - etsec->rx_buffer_len;
+    if (etsec->rx_padding + etsec->rx_buffer_len < MAX_RX_PADDING) {
+        etsec->rx_padding = MAX_RX_PADDING - etsec->rx_buffer_len;
      }

Re: [PATCH 1/4] hw/net/fsl_etsec/rings.c: Avoid variable length array

[Peter Maydell]

On Thu, 24 Aug 2023 at 16:47, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> On 24/8/23 17:32, Peter Maydell wrote:
> > In fill_rx_bd() we create a variable length array of size
> > etsec->rx_padding. In fact we know that this will never be
> > larger than 64 bytes, because rx_padding is set in rx_init_frame()
> > in a way that ensures it is only that large. Use a fixed sized
> > array and assert that it is big enough.
> >
> > Since padd[] is now potentially rather larger than the actual
> > padding required, adjust the memset() we do on it to match the
> > size that we write with cpu_physical_memory_write(), rather than
> > clearing the entire array.
> >
> > The codebase has very few VLAs, 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).
> >
> > Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> > ---
> >   hw/net/fsl_etsec/rings.c | 12 ++++++++++--
> >   1 file changed, 10 insertions(+), 2 deletions(-)
> >
> > diff --git a/hw/net/fsl_etsec/rings.c b/hw/net/fsl_etsec/rings.c
> > index 788463f1b62..2f2f359f7a5 100644
> > --- a/hw/net/fsl_etsec/rings.c
> > +++ b/hw/net/fsl_etsec/rings.c
> > @@ -372,6 +372,12 @@ void etsec_walk_tx_ring(eTSEC *etsec, int ring_nbr)
> >       etsec->regs[TSTAT].value |= 1 << (31 - ring_nbr);
> >   }
> >
> > +/*
> > + * rx_init_frame() ensures we never do more padding than this
> > + * (checksum plus minimum data packet size)
> > + */
> > +#define MAX_RX_PADDING 64
> > +
> >   static void fill_rx_bd(eTSEC          *etsec,
> >                          eTSEC_rxtx_bd  *bd,
> >                          const uint8_t **buf,
> > @@ -380,9 +386,11 @@ static void fill_rx_bd(eTSEC          *etsec,
> >       uint16_t to_write;
> >       hwaddr   bufptr = bd->bufptr +
> >           ((hwaddr)(etsec->regs[TBDBPH].value & 0xF) << 32);
> > -    uint8_t  padd[etsec->rx_padding];
> > +    uint8_t  padd[MAX_RX_PADDING];
> >       uint8_t  rem;
> >
> > +    assert(etsec->rx_padding <= MAX_RX_PADDING);
> > +
> >       RING_DEBUG("eTSEC fill Rx buffer @ 0x%016" HWADDR_PRIx
> >                  " size:%zu(padding + crc:%u) + fcb:%u\n",
> >                  bufptr, *size, etsec->rx_padding, etsec->rx_fcb_size);
> > @@ -426,7 +434,7 @@ static void fill_rx_bd(eTSEC          *etsec,
> >           rem = MIN(etsec->regs[MRBLR].value - bd->length, etsec->rx_padding);
> >
> >           if (rem > 0) {
> > -            memset(padd, 0x0, sizeof(padd));
> > +            memset(padd, 0x0, rem);
> >               etsec->rx_padding -= rem;
> >               *size             -= rem;
> >               bd->length        += rem;
>
> Maybe we can add this for clarity:
>
> @@ -468,6 +468,6 @@ static void rx_init_frame(eTSEC *etsec, const
> uint8_t *buf, size_t size)
>        * minimum MTU size bytes long (64)
>        */
> -    if (etsec->rx_buffer_len < 60) {
> -        etsec->rx_padding += 60 - etsec->rx_buffer_len;
> +    if (etsec->rx_padding + etsec->rx_buffer_len < MAX_RX_PADDING) {
> +        etsec->rx_padding = MAX_RX_PADDING - etsec->rx_buffer_len;
>       }

I think that's a more confusing way of putting it. What the
code is doing is "if the packet is too short, pad it to
the minimum-packet-length", and the clear way to express
that is "if (packet_len < max) add_more_padding;".

There is potential to use the constants ETH_ZLEN (60) and
ETH_FCS_LEN (4) instead of the hard-coded 60 and 4 currently
in the code, but I felt that was starting to wander a bit
out of scope of just getting rid of the VLA.

thanks
-- PMM

Re: [PATCH 1/4] hw/net/fsl_etsec/rings.c: Avoid variable length array

[Philippe Mathieu-Daudé]

On 24/8/23 18:01, Peter Maydell wrote:
> On Thu, 24 Aug 2023 at 16:47, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>>
>> On 24/8/23 17:32, Peter Maydell wrote:
>>> In fill_rx_bd() we create a variable length array of size
>>> etsec->rx_padding. In fact we know that this will never be
>>> larger than 64 bytes, because rx_padding is set in rx_init_frame()
>>> in a way that ensures it is only that large. Use a fixed sized
>>> array and assert that it is big enough.
>>>
>>> Since padd[] is now potentially rather larger than the actual
>>> padding required, adjust the memset() we do on it to match the
>>> size that we write with cpu_physical_memory_write(), rather than
>>> clearing the entire array.
>>>
>>> The codebase has very few VLAs, 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).
>>>
>>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>>> ---
>>>    hw/net/fsl_etsec/rings.c | 12 ++++++++++--
>>>    1 file changed, 10 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/hw/net/fsl_etsec/rings.c b/hw/net/fsl_etsec/rings.c
>>> index 788463f1b62..2f2f359f7a5 100644
>>> --- a/hw/net/fsl_etsec/rings.c
>>> +++ b/hw/net/fsl_etsec/rings.c
>>> @@ -372,6 +372,12 @@ void etsec_walk_tx_ring(eTSEC *etsec, int ring_nbr)
>>>        etsec->regs[TSTAT].value |= 1 << (31 - ring_nbr);
>>>    }
>>>
>>> +/*
>>> + * rx_init_frame() ensures we never do more padding than this
>>> + * (checksum plus minimum data packet size)
>>> + */
>>> +#define MAX_RX_PADDING 64


>> Maybe we can add this for clarity:
>>
>> @@ -468,6 +468,6 @@ static void rx_init_frame(eTSEC *etsec, const
>> uint8_t *buf, size_t size)
>>         * minimum MTU size bytes long (64)
>>         */
>> -    if (etsec->rx_buffer_len < 60) {
>> -        etsec->rx_padding += 60 - etsec->rx_buffer_len;
>> +    if (etsec->rx_padding + etsec->rx_buffer_len < MAX_RX_PADDING) {
>> +        etsec->rx_padding = MAX_RX_PADDING - etsec->rx_buffer_len;
>>        }
> 
> I think that's a more confusing way of putting it. What the
> code is doing is "if the packet is too short, pad it to
> the minimum-packet-length", and the clear way to express
> that is "if (packet_len < max) add_more_padding;".
> 
> There is potential to use the constants ETH_ZLEN (60) and
> ETH_FCS_LEN (4) instead of the hard-coded 60 and 4 currently
> in the code, but I felt that was starting to wander a bit
> out of scope of just getting rid of the VLA.

Right. So possibly:

#define MAX_RX_PADDING (ETH_ZLEN + ETH_FCS_LEN)

but 64 is clear enough.

Thanks for the feedback.

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

[PATCH 2/4] hw/net/rocker: Avoid variable length array

[Peter Maydell]

Replace an on-stack variable length array in of_dpa_ig() with
a g_autofree heap allocation.

The codebase has very few VLAs, 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).

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/net/rocker/rocker_of_dpa.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/net/rocker/rocker_of_dpa.c b/hw/net/rocker/rocker_of_dpa.c
index dfe47544694..5e16056be66 100644
--- a/hw/net/rocker/rocker_of_dpa.c
+++ b/hw/net/rocker/rocker_of_dpa.c
@@ -1043,7 +1043,7 @@ static void of_dpa_flow_ig_tbl(OfDpaFlowContext *fc, uint32_t tbl_id)
 static ssize_t of_dpa_ig(World *world, uint32_t pport,
                          const struct iovec *iov, int iovcnt)
 {
-    struct iovec iov_copy[iovcnt + 2];
+    g_autofree struct iovec *iov_copy = g_new(struct iovec, iovcnt + 2);
     OfDpaFlowContext fc = {
         .of_dpa = world_private(world),
         .in_pport = pport,
-- 
2.34.1

Re: [PATCH 2/4] hw/net/rocker: Avoid variable length array

[Francisco Iglesias]

On [2023 Aug 24] Thu 16:32:22, Peter Maydell wrote:
> Replace an on-stack variable length array in of_dpa_ig() with
> a g_autofree heap allocation.
> 
> The codebase has very few VLAs, 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).
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Francisco Iglesias <frasse.iglesias@gmail.com>

> ---
>  hw/net/rocker/rocker_of_dpa.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/net/rocker/rocker_of_dpa.c b/hw/net/rocker/rocker_of_dpa.c
> index dfe47544694..5e16056be66 100644
> --- a/hw/net/rocker/rocker_of_dpa.c
> +++ b/hw/net/rocker/rocker_of_dpa.c
> @@ -1043,7 +1043,7 @@ static void of_dpa_flow_ig_tbl(OfDpaFlowContext *fc, uint32_t tbl_id)
>  static ssize_t of_dpa_ig(World *world, uint32_t pport,
>                           const struct iovec *iov, int iovcnt)
>  {
> -    struct iovec iov_copy[iovcnt + 2];
> +    g_autofree struct iovec *iov_copy = g_new(struct iovec, iovcnt + 2);
>      OfDpaFlowContext fc = {
>          .of_dpa = world_private(world),
>          .in_pport = pport,
> -- 
> 2.34.1
> 
> 

[PATCH 3/4] net/dump: Avoid variable length array

[Peter Maydell]

Use a g_autofree heap allocation instead of a variable length
array in dump_receive_iov().

The codebase has very few VLAs, 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).

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 net/dump.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/dump.c b/net/dump.c
index 7d05f16ca7a..16073f24582 100644
--- a/net/dump.c
+++ b/net/dump.c
@@ -68,7 +68,7 @@ static ssize_t dump_receive_iov(DumpState *s, const struct iovec *iov, int cnt,
     int64_t ts;
     int caplen;
     size_t size = iov_size(iov, cnt) - offset;
-    struct iovec dumpiov[cnt + 1];
+    g_autofree struct iovec *dumpiov = g_new(struct iovec, cnt + 1);
 
     /* Early return in case of previous error. */
     if (s->fd < 0) {
-- 
2.34.1

Re: [PATCH 3/4] net/dump: Avoid variable length array

[Francisco Iglesias]

On [2023 Aug 24] Thu 16:32:23, Peter Maydell wrote:
> Use a g_autofree heap allocation instead of a variable length
> array in dump_receive_iov().
> 
> The codebase has very few VLAs, 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).
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Francisco Iglesias <frasse.iglesias@gmail.com>

> ---
>  net/dump.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/dump.c b/net/dump.c
> index 7d05f16ca7a..16073f24582 100644
> --- a/net/dump.c
> +++ b/net/dump.c
> @@ -68,7 +68,7 @@ static ssize_t dump_receive_iov(DumpState *s, const struct iovec *iov, int cnt,
>      int64_t ts;
>      int caplen;
>      size_t size = iov_size(iov, cnt) - offset;
> -    struct iovec dumpiov[cnt + 1];
> +    g_autofree struct iovec *dumpiov = g_new(struct iovec, cnt + 1);
>  
>      /* Early return in case of previous error. */
>      if (s->fd < 0) {
> -- 
> 2.34.1
> 
> 

[PATCH 4/4] net/tap: Avoid variable-length array

[Peter Maydell]

Use a heap allocation instead of a variable length array in
tap_receive_iov().

The codebase has very few VLAs, 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).

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 net/tap.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/tap.c b/net/tap.c
index 1bf085d4228..34b1e3f0918 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -117,10 +117,11 @@ static ssize_t tap_receive_iov(NetClientState *nc, const struct iovec *iov,
 {
     TAPState *s = DO_UPCAST(TAPState, nc, nc);
     const struct iovec *iovp = iov;
-    struct iovec iov_copy[iovcnt + 1];
+    g_autofree struct iovec *iov_copy = NULL;
     struct virtio_net_hdr_mrg_rxbuf hdr = { };
 
     if (s->host_vnet_hdr_len && !s->using_vnet_hdr) {
+        iov_copy = g_new(struct iovec, iovcnt + 1);
         iov_copy[0].iov_base = &hdr;
         iov_copy[0].iov_len =  s->host_vnet_hdr_len;
         memcpy(&iov_copy[1], iov, iovcnt * sizeof(*iov));
-- 
2.34.1

Re: [PATCH 4/4] net/tap: Avoid variable-length array

[Francisco Iglesias]

On [2023 Aug 24] Thu 16:32:24, Peter Maydell wrote:
> Use a heap allocation instead of a variable length array in
> tap_receive_iov().
> 
> The codebase has very few VLAs, 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).
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Francisco Iglesias <frasse.iglesias@gmail.com>

> ---
>  net/tap.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/net/tap.c b/net/tap.c
> index 1bf085d4228..34b1e3f0918 100644
> --- a/net/tap.c
> +++ b/net/tap.c
> @@ -117,10 +117,11 @@ static ssize_t tap_receive_iov(NetClientState *nc, const struct iovec *iov,
>  {
>      TAPState *s = DO_UPCAST(TAPState, nc, nc);
>      const struct iovec *iovp = iov;
> -    struct iovec iov_copy[iovcnt + 1];
> +    g_autofree struct iovec *iov_copy = NULL;
>      struct virtio_net_hdr_mrg_rxbuf hdr = { };
>  
>      if (s->host_vnet_hdr_len && !s->using_vnet_hdr) {
> +        iov_copy = g_new(struct iovec, iovcnt + 1);
>          iov_copy[0].iov_base = &hdr;
>          iov_copy[0].iov_len =  s->host_vnet_hdr_len;
>          memcpy(&iov_copy[1], iov, iovcnt * sizeof(*iov));
> -- 
> 2.34.1
> 
> 

Re: [PATCH 0/4] net: avoid variable length arrays

[Peter Maydell]

Hi, Jason. This patchset has been reviewed -- do you want to
pick it up via the net tree?

thanks
-- PMM

On Thu, 24 Aug 2023 at 16:32, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> This patchset removes the use of variable length arrays in a couple
> of network devices and the net/ core code.  In one case we can switch
> to a fixed-sized array on the stack; in the other three we have to
> use a heap allocation.
>
> The codebase has very few VLAs, 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).
>
> Philippe had a go at these in  a patch in 2021:
> https://patchew.org/QEMU/20210505211047.1496765-1-philmd@redhat.com/20210505211047.1496765-16-philmd@redhat.com/
> but these are re-implementations, mostly.
>
> Usual disclaimer: I have tested these patches only with
> "make check" and "make check-avocado".
>
> thanks
> -- PMM
>
> Peter Maydell (4):
>   hw/net/fsl_etsec/rings.c: Avoid variable length array
>   hw/net/rocker: Avoid variable length array
>   net/dump: Avoid variable length array
>   net/tap: Avoid variable-length array
>
>  hw/net/fsl_etsec/rings.c      | 12 ++++++++++--
>  hw/net/rocker/rocker_of_dpa.c |  2 +-
>  net/dump.c                    |  2 +-
>  net/tap.c                     |  3 ++-
>  4 files changed, 14 insertions(+), 5 deletions(-)

Re: [PATCH 0/4] net: avoid variable length arrays

[Jason Wang]

On Tue, Sep 12, 2023 at 10:20 PM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> Hi, Jason. This patchset has been reviewed -- do you want to
> pick it up via the net tree?

Yes, I've queued this.

Thanks

>
> thanks
> -- PMM
>
> On Thu, 24 Aug 2023 at 16:32, Peter Maydell <peter.maydell@linaro.org> wrote:
> >
> > This patchset removes the use of variable length arrays in a couple
> > of network devices and the net/ core code.  In one case we can switch
> > to a fixed-sized array on the stack; in the other three we have to
> > use a heap allocation.
> >
> > The codebase has very few VLAs, 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).
> >
> > Philippe had a go at these in  a patch in 2021:
> > https://patchew.org/QEMU/20210505211047.1496765-1-philmd@redhat.com/20210505211047.1496765-16-philmd@redhat.com/
> > but these are re-implementations, mostly.
> >
> > Usual disclaimer: I have tested these patches only with
> > "make check" and "make check-avocado".
> >
> > thanks
> > -- PMM
> >
> > Peter Maydell (4):
> >   hw/net/fsl_etsec/rings.c: Avoid variable length array
> >   hw/net/rocker: Avoid variable length array
> >   net/dump: Avoid variable length array
> >   net/tap: Avoid variable-length array
> >
> >  hw/net/fsl_etsec/rings.c      | 12 ++++++++++--
> >  hw/net/rocker/rocker_of_dpa.c |  2 +-
> >  net/dump.c                    |  2 +-
> >  net/tap.c                     |  3 ++-
> >  4 files changed, 14 insertions(+), 5 deletions(-)
>