[haiku-commits] haiku: hrev56183 - src/add-ons/kernel/drivers/network/ether/usb_rndis

  • From: Adrien Destugues <pulkomandy@xxxxxxxxxxxxx>
  • To: haiku-commits@xxxxxxxxxxxxx
  • Date: Sat, 11 Jun 2022 19:03:53 +0000 (UTC)

hrev56183 adds 2 changesets to branch 'master'
old head: 802e16c55a80cdd5dacdfc0d04b0e6f522e857b1
new head: fb0214278266f4f3e55ad0fa00522a6f8f6bb68f
overview: 
https://git.haiku-os.org/haiku/log/?qt=range&q=fb0214278266+%5E802e16c55a80

----------------------------------------------------------------------------

4dda1c0369c8: usb_rndis: use the "data offset" field instead of hardcoding it
  
  Not sure if other phones could use another value, but it's better to
  follow the spec. Also add some bounds checks with traces for now if we
  see something strange.
  
  Change-Id: I5c7bc37c4730e6a08bf0bf10fed975bf2012102e
  Reviewed-on: https://review.haiku-os.org/c/haiku/+/5376
  Reviewed-by: waddlesplash <waddlesplash@xxxxxxxxx>
  Reviewed-by: Adrien Destugues <pulkomandy@xxxxxxxxxxxxx>

fb0214278266: usb_rndis: fix handling of multiple packets in one USB transaction
  
  I got my pointer math wrong because some things in RNDIS use uint32 as
  the base, but some things are in bytes. Most of the time this would result
  in an offset past the end of the USB buffer, so it would just lead to
  ignoring all but the first packet. But if the first packet was small enough,
  it would point somewhere still in the buffer, and we would read the wrong
  data.
  
  Fixes #17775
  
  Change-Id: I32ec0081336b1f772d4dc3099a0ac2c691aa12f0
  Reviewed-on: https://review.haiku-os.org/c/haiku/+/5377
  Reviewed-by: Adrien Destugues <pulkomandy@xxxxxxxxxxxxx>

                                   [ PulkoMandy <pulkomandy@xxxxxxxxxxxxx> ]

----------------------------------------------------------------------------

1 file changed, 11 insertions(+), 3 deletions(-)
.../drivers/network/ether/usb_rndis/RNDISDevice.cpp    | 14 +++++++++++---

############################################################################

Commit:      4dda1c0369c8b53d7dc5906b332bee14c3746c18
URL:         https://git.haiku-os.org/haiku/commit/?id=4dda1c0369c8
Author:      PulkoMandy <pulkomandy@xxxxxxxxxxxxx>
Date:        Fri Jun 10 19:37:25 2022 UTC
Committer:   Adrien Destugues <pulkomandy@xxxxxxxxxxxxx>
Commit-Date: Sat Jun 11 19:03:48 2022 UTC

usb_rndis: use the "data offset" field instead of hardcoding it

Not sure if other phones could use another value, but it's better to
follow the spec. Also add some bounds checks with traces for now if we
see something strange.

Change-Id: I5c7bc37c4730e6a08bf0bf10fed975bf2012102e
Reviewed-on: https://review.haiku-os.org/c/haiku/+/5376
Reviewed-by: waddlesplash <waddlesplash@xxxxxxxxx>
Reviewed-by: Adrien Destugues <pulkomandy@xxxxxxxxxxxxx>

----------------------------------------------------------------------------

diff --git a/src/add-ons/kernel/drivers/network/ether/usb_rndis/RNDISDevice.cpp 
b/src/add-ons/kernel/drivers/network/ether/usb_rndis/RNDISDevice.cpp
index b23b5ea6cb..1b17befba3 100644
--- a/src/add-ons/kernel/drivers/network/ether/usb_rndis/RNDISDevice.cpp
+++ b/src/add-ons/kernel/drivers/network/ether/usb_rndis/RNDISDevice.cpp
@@ -289,8 +289,13 @@ RNDISDevice::Read(uint8 *buffer, size_t *numBytes)
                        fActualLengthRead);
        }
 
+       if (fReadHeader[2] + fReadHeader[3] > fReadHeader[1]) {
+               TRACE_ALWAYS("Received frame data goes past end of frame: %d + 
%d > %d", fReadHeader[2],
+                       fReadHeader[3], fReadHeader[1]);
+       }
+
        if (fReadHeader[4] != 0 || fReadHeader[5] != 0 || fReadHeader[6] != 0) {
-               TRACE_ALWAYS("Received frame has out of bound data: off %08" 
B_PRIx32 " len %08" B_PRIx32
+               TRACE_ALWAYS("Received frame has out of band data: off %08" 
B_PRIx32 " len %08" B_PRIx32
                        " count %08" B_PRIx32 "\n", fReadHeader[4], 
fReadHeader[5], fReadHeader[6]);
        }
 
@@ -304,7 +309,8 @@ RNDISDevice::Read(uint8 *buffer, size_t *numBytes)
        }
 
        *numBytes = fReadHeader[3];
-       memcpy(buffer, fReadHeader + 11, fReadHeader[3]);
+       int offset = fReadHeader[2] + 2 * sizeof(uint32);
+       memcpy(buffer, (uint8*)fReadHeader + offset, fReadHeader[3]);
 
        TRACE("Received data packet len %08" B_PRIx32 " data [off %08" B_PRIx32 
" len %08" B_PRIx32 "]\n",
                fReadHeader[1], fReadHeader[2], fReadHeader[3]);

############################################################################

Revision:    hrev56183
Commit:      fb0214278266f4f3e55ad0fa00522a6f8f6bb68f
URL:         https://git.haiku-os.org/haiku/commit/?id=fb0214278266
Author:      PulkoMandy <pulkomandy@xxxxxxxxxxxxx>
Date:        Sat Jun 11 17:39:11 2022 UTC
Committer:   Adrien Destugues <pulkomandy@xxxxxxxxxxxxx>
Commit-Date: Sat Jun 11 19:03:48 2022 UTC

Ticket:      https://dev.haiku-os.org/ticket/17775

usb_rndis: fix handling of multiple packets in one USB transaction

I got my pointer math wrong because some things in RNDIS use uint32 as
the base, but some things are in bytes. Most of the time this would result
in an offset past the end of the USB buffer, so it would just lead to
ignoring all but the first packet. But if the first packet was small enough,
it would point somewhere still in the buffer, and we would read the wrong
data.

Fixes #17775

Change-Id: I32ec0081336b1f772d4dc3099a0ac2c691aa12f0
Reviewed-on: https://review.haiku-os.org/c/haiku/+/5377
Reviewed-by: Adrien Destugues <pulkomandy@xxxxxxxxxxxxx>

----------------------------------------------------------------------------

diff --git a/src/add-ons/kernel/drivers/network/ether/usb_rndis/RNDISDevice.cpp 
b/src/add-ons/kernel/drivers/network/ether/usb_rndis/RNDISDevice.cpp
index 1b17befba3..c38e11758b 100644
--- a/src/add-ons/kernel/drivers/network/ether/usb_rndis/RNDISDevice.cpp
+++ b/src/add-ons/kernel/drivers/network/ether/usb_rndis/RNDISDevice.cpp
@@ -316,7 +316,9 @@ RNDISDevice::Read(uint8 *buffer, size_t *numBytes)
                fReadHeader[1], fReadHeader[2], fReadHeader[3]);
 
        // Advance to next packet
-       fReadHeader += fReadHeader[1];
+       fReadHeader = (uint32*)((uint8*)fReadHeader + fReadHeader[1]);
+
+       // Are we past the end of the buffer? If so, prepare to receive another 
one on the next read
        if ((uint32)((uint8*)fReadHeader - fReadBuffer) >= fActualLengthRead)
                fReadHeader = NULL;
 


Other related posts:

  • » [haiku-commits] haiku: hrev56183 - src/add-ons/kernel/drivers/network/ether/usb_rndis - Adrien Destugues