Re: [Qemu-devel] [PATCH v2] net: rocker: fix an incorrect array bounds check

Re: [Qemu-devel] [PATCH v2] net: rocker: fix an incorrect array bounds check

[Peter Maydell]

On 22 December 2015 at 19:24, P J P <ppandit@redhat.com> wrote:
>   Hello Paolo, all
>
> Please see an updated patch below, as per suggestion in
>   -> https://lists.gnu.org/archive/html/qemu-devel/2015-12/msg04057.html

Could you submit patches in the usual git send-email format,
please? It's easier for maintainers to process them if they're
not in an odd arrangement that requires manual intervention.
(In particular, comments that aren't intended to go in the
final git commit message go below the '---' line. You can
look at other patches on the list to get the idea of how
they should look.)

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2] net: rocker: fix an incorrect array bounds check

[P J P]

+-- On Tue, 22 Dec 2015, Peter Maydell wrote --+
| Could you submit patches in the usual git send-email format,
| please? It's easier for maintainers to process them if they're
| not in an odd arrangement that requires manual intervention.
| (In particular, comments that aren't intended to go in the
| final git commit message go below the '---' line. You can
| look at other patches on the list to get the idea of how
| they should look.)

  Yes, surely will do. I did read about it here[*], just haven't gotten around 
to trying git send-email yet.

[*] -> http://qemu-project.org/Contribute/SubmitAPatch

Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F

Re: [Qemu-devel] [PATCH v2] net: rocker: fix an incorrect array bounds check

[Jason Wang]

On 12/23/2015 01:14 PM, P J P wrote:
> +-- On Tue, 22 Dec 2015, Peter Maydell wrote --+
> | Could you submit patches in the usual git send-email format,
> | please? It's easier for maintainers to process them if they're
> | not in an odd arrangement that requires manual intervention.
> | (In particular, comments that aren't intended to go in the
> | final git commit message go below the '---' line. You can
> | look at other patches on the list to get the idea of how
> | they should look.)
>
>   Yes, surely will do. I did read about it here[*], just haven't gotten around 
> to trying git send-email yet.
>
> [*] -> http://qemu-project.org/Contribute/SubmitAPatch
>
> Thank you.
> --
> Prasad J Pandit / Red Hat Product Security Team
> 47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F
>

Hi, patch looks good. Just wonder do you want to re-submit the patch
with 'git send-email'? (since git am does not work for this mail without
manual editing).

Thanks

Re: [Qemu-devel] [PATCH v2] net: rocker: fix an incorrect array bounds check

[P J P]

  Hello Jason, all

+-- On Mon, 28 Dec 2015, Jason Wang wrote --+
| On 12/23/2015 01:14 PM, P J P wrote:
| > +-- On Tue, 22 Dec 2015, Peter Maydell wrote --+
| > | Could you submit patches in the usual git send-email format,
| > | please?
| >
| >   Yes, surely will do. I did read about it here[*], just haven't gotten around 
| > to trying git send-email yet.
| 
| Hi, patch looks good. Just wonder do you want to re-submit the patch
| with 'git send-email'? (since git am does not work for this mail without
| manual editing).

Please see:
  -> https://lists.gnu.org/archive/html/qemu-devel/2015-12/msg04629.html

Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F

Re: [Qemu-devel] [PATCH v2] net: rocker: fix an incorrect array bounds check

[Jason Wang]

On 12/28/2015 06:54 PM, P J P wrote:
> From: Prasad J Pandit <pjp@fedoraproject.org>
>
> While processing transmit(tx) descriptors in 'tx_consume' routine
> the switch emulator suffers from an off-by-one error, if a
> descriptor was to have more than allowed(ROCKER_TX_FRAGS_MAX=16)
> fragments. Fix an incorrect bounds check to avoid it.
>
> Reported-by: Qinghao Tang <luodalongde@gmail.com>
> Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org>
> ---
>  hw/net/rocker/rocker.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/hw/net/rocker/rocker.c b/hw/net/rocker/rocker.c
> index c57f1a6..2e77e50 100644
> --- a/hw/net/rocker/rocker.c
> +++ b/hw/net/rocker/rocker.c
> @@ -232,6 +232,9 @@ static int tx_consume(Rocker *r, DescInfo *info)
>          frag_addr = rocker_tlv_get_le64(tlvs[ROCKER_TLV_TX_FRAG_ATTR_ADDR]);
>          frag_len = rocker_tlv_get_le16(tlvs[ROCKER_TLV_TX_FRAG_ATTR_LEN]);
>  
> +        if (iovcnt >= ROCKER_TX_FRAGS_MAX) {
> +            goto err_too_many_frags;
> +        }
>          iov[iovcnt].iov_len = frag_len;
>          iov[iovcnt].iov_base = g_malloc(frag_len);
>          if (!iov[iovcnt].iov_base) {
> @@ -244,10 +247,7 @@ static int tx_consume(Rocker *r, DescInfo *info)
>              err = -ROCKER_ENXIO;
>              goto err_bad_io;
>          }
> -
> -        if (++iovcnt > ROCKER_TX_FRAGS_MAX) {
> -            goto err_too_many_frags;
> -        }
> +        iovcnt++;
>      }
>  
>      if (iovcnt) {

Applied in my -net.

Thanks

[Qemu-devel] [PATCH v2] net: rocker: fix an incorrect array bounds check

[P J P]

From: Prasad J Pandit <pjp@fedoraproject.org>

While processing transmit(tx) descriptors in 'tx_consume' routine
the switch emulator suffers from an off-by-one error, if a
descriptor was to have more than allowed(ROCKER_TX_FRAGS_MAX=16)
fragments. Fix an incorrect bounds check to avoid it.

Reported-by: Qinghao Tang <luodalongde@gmail.com>
Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org>
---
 hw/net/rocker/rocker.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/net/rocker/rocker.c b/hw/net/rocker/rocker.c
index c57f1a6..2e77e50 100644
--- a/hw/net/rocker/rocker.c
+++ b/hw/net/rocker/rocker.c
@@ -232,6 +232,9 @@ static int tx_consume(Rocker *r, DescInfo *info)
         frag_addr = rocker_tlv_get_le64(tlvs[ROCKER_TLV_TX_FRAG_ATTR_ADDR]);
         frag_len = rocker_tlv_get_le16(tlvs[ROCKER_TLV_TX_FRAG_ATTR_LEN]);
 
+        if (iovcnt >= ROCKER_TX_FRAGS_MAX) {
+            goto err_too_many_frags;
+        }
         iov[iovcnt].iov_len = frag_len;
         iov[iovcnt].iov_base = g_malloc(frag_len);
         if (!iov[iovcnt].iov_base) {
@@ -244,10 +247,7 @@ static int tx_consume(Rocker *r, DescInfo *info)
             err = -ROCKER_ENXIO;
             goto err_bad_io;
         }
-
-        if (++iovcnt > ROCKER_TX_FRAGS_MAX) {
-            goto err_too_many_frags;
-        }
+        iovcnt++;
     }
 
     if (iovcnt) {
-- 
2.4.3

Re: [Qemu-devel] [PATCH v2] net: rocker: fix an incorrect array bounds check

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 2111 bytes --]

On Wed, Dec 23, 2015 at 12:54:18AM +0530, P J P wrote:
>   Hello Paolo, all
> 
> Please see an updated patch below, as per suggestion in
>   -> https://lists.gnu.org/archive/html/qemu-devel/2015-12/msg04057.html

Adding Jason Wang (net maintainer) on CC.

> ===
> >From 344a487d637be20b3fb110bec36cb703e7f6ecaa Mon Sep 17 00:00:00 2001
> From: Prasad J Pandit <pjp@fedoraproject.org>
> Date: Wed, 23 Dec 2015 00:40:13 +0530
> Subject: [PATCH v2] net: rocker: fix an incorrect array bounds check
> 
> While processing transmit(tx) descriptors in 'tx_consume' routine
> the switch emulator suffers from an off-by-one error, if a
> descriptor was to have more than allowed(ROCKER_TX_FRAGS_MAX=16)
> fragments. Fix an incorrect bounds check to avoid it.
> 
> Reported-by: Qinghao Tang <luodalongde@gmail.com>
> Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org>
> ---
>  hw/net/rocker/rocker.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/net/rocker/rocker.c b/hw/net/rocker/rocker.c
> index c57f1a6..2e77e50 100644
> --- a/hw/net/rocker/rocker.c
> +++ b/hw/net/rocker/rocker.c
> @@ -232,6 +232,9 @@ static int tx_consume(Rocker *r, DescInfo *info)
>          frag_addr = rocker_tlv_get_le64(tlvs[ROCKER_TLV_TX_FRAG_ATTR_ADDR]);
>          frag_len = rocker_tlv_get_le16(tlvs[ROCKER_TLV_TX_FRAG_ATTR_LEN]);
> 
> +        if (iovcnt >= ROCKER_TX_FRAGS_MAX) {
> +            goto err_too_many_frags;
> +        }
>          iov[iovcnt].iov_len = frag_len;
>          iov[iovcnt].iov_base = g_malloc(frag_len);
>          if (!iov[iovcnt].iov_base) {
> @@ -244,10 +247,7 @@ static int tx_consume(Rocker *r, DescInfo *info)
>              err = -ROCKER_ENXIO;
>              goto err_bad_io;
>          }
> -
> -        if (++iovcnt > ROCKER_TX_FRAGS_MAX) {
> -            goto err_too_many_frags;
> -        }
> +        iovcnt++;
>      }
> 
>      if (iovcnt) {
> -- 
> 2.4.3
> ===
> 
> 
> Thank you.
> --
> Prasad J Pandit / Red Hat Product Security Team
> 47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[Qemu-devel] [PATCH v2] net: rocker: fix an incorrect array bounds check

[P J P]

   Hello Paolo, all

Please see an updated patch below, as per suggestion in
   -> https://lists.gnu.org/archive/html/qemu-devel/2015-12/msg04057.html

===
>From 344a487d637be20b3fb110bec36cb703e7f6ecaa Mon Sep 17 00:00:00 2001
From: Prasad J Pandit <pjp@fedoraproject.org>
Date: Wed, 23 Dec 2015 00:40:13 +0530
Subject: [PATCH v2] net: rocker: fix an incorrect array bounds check

While processing transmit(tx) descriptors in 'tx_consume' routine
the switch emulator suffers from an off-by-one error, if a
descriptor was to have more than allowed(ROCKER_TX_FRAGS_MAX=16)
fragments. Fix an incorrect bounds check to avoid it.

Reported-by: Qinghao Tang <luodalongde@gmail.com>
Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org>
---
  hw/net/rocker/rocker.c | 8 ++++----
  1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/net/rocker/rocker.c b/hw/net/rocker/rocker.c
index c57f1a6..2e77e50 100644
--- a/hw/net/rocker/rocker.c
+++ b/hw/net/rocker/rocker.c
@@ -232,6 +232,9 @@ static int tx_consume(Rocker *r, DescInfo *info)
          frag_addr = rocker_tlv_get_le64(tlvs[ROCKER_TLV_TX_FRAG_ATTR_ADDR]);
          frag_len = rocker_tlv_get_le16(tlvs[ROCKER_TLV_TX_FRAG_ATTR_LEN]);

+        if (iovcnt >= ROCKER_TX_FRAGS_MAX) {
+            goto err_too_many_frags;
+        }
          iov[iovcnt].iov_len = frag_len;
          iov[iovcnt].iov_base = g_malloc(frag_len);
          if (!iov[iovcnt].iov_base) {
@@ -244,10 +247,7 @@ static int tx_consume(Rocker *r, DescInfo *info)
              err = -ROCKER_ENXIO;
              goto err_bad_io;
          }
-
-        if (++iovcnt > ROCKER_TX_FRAGS_MAX) {
-            goto err_too_many_frags;
-        }
+        iovcnt++;
      }

      if (iovcnt) {
-- 
2.4.3
===


Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F