[PATCH v4] USB: serial: garmin_gps: validate packet data length in nat_receive()

[PATCH v4] USB: serial: garmin_gps: validate packet data length in nat_receive()

[Hui Peng]

In nat_receive() (called from garmin_write() when userspace writes to
/dev/ttyUSBn in MODE_NATIVE mode), while garmin_data_p->insize is less
than GARMIN_PKTHDR_LENGTH (12), the loop copies up to 12 bytes into
garmin_data_p->inbuffer. Once garmin_data_p->insize reaches
GARMIN_PKTHDR_LENGTH, nat_receive() computes the total packet size as:

  len = GARMIN_PKTHDR_LENGTH + getDataLength(garmin_data_p->inbuffer);

Because getDataLength() returns a signed int from __le32_to_cpup(), a
12-byte header with a negative 32-bit length field (such as 0xfffffff5,
i.e., -11) causes len to underflow to a small positive value (12 + (-11)
= 1) without any bounds check at that point. Then garmin_data_p->insize
>= len (12 >= 1) evaluates to true and nat_receive() calls
garmin_write_bulk(port, inbuffer, 1, 0), which allocates a 1-byte slab
buffer via kmemdup() and immediately reads 4 bytes from it in
getLayerId(buffer) (and again in garmin_write_bulk_callback() when the
URB completes):

  BUG: KASAN: slab-out-of-bounds in garmin_write_bulk.constprop.0+0x3eb/0x500
  Read of size 4 at addr ffff888002cc0ba0 by task init/1
  Call Trace:
   <TASK>
   dump_stack_lvl+0x70/0xa0
   print_report+0x153/0x4c6
   kasan_report+0xf1/0x120
   garmin_write_bulk.constprop.0+0x3eb/0x500
   garmin_write+0x60f/0x1450
   serial_write+0x123/0x200
   n_tty_write+0x8ea/0xeb0
   file_tty_write.isra.0+0x44f/0x7a0
   vfs_write+0x671/0xd20

Validate the unsigned 32-bit data length (dlen >= GPS_IN_BUFSIZ -
GARMIN_PKTHDR_LENGTH, matching the existing len >= GPS_IN_BUFSIZ bound)
in nat_receive() as soon as the 12-byte header is present, resetting
insize and returning -EINVPKT on invalid lengths so garmin_write_bulk()
is only ever called with a full valid packet header (12 <= len <
GPS_IN_BUFSIZ).

Tested in QEMU against Linux 7.3.0-rc3 by emulating a Garmin USB GPS
device (091e:0003) via dummy_hcd + raw-gadget, switching /dev/ttyUSB0
to MODE_NATIVE via PRIV_PKTID_SET_MODE, and writing a 12-byte native
packet with data length 0xfffffff5: on the unfixed kernel this triggers
KASAN slab-out-of-bounds reads in garmin_write_bulk() and
garmin_write_bulk_callback(), whereas on the fixed kernel nat_receive()
rejects the packet with -EINVPKT and 0 KASAN faults.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Hui Peng <benquike@gmail.com>
---
Changes in v4:
- Kept the fix strictly inside nat_receive() and dropped the other hunks
  from v3 (garmin_write_bulk_callback(), gsp_send(), and changing the
  return type of getDataLength() / getPacketId()) per Greg
  Kroah-Hartman:
  1. Assigning u32 dlen = getDataLength(garmin_data_p->inbuffer) locally
     in nat_receive() and rejecting dlen >= GPS_IN_BUFSIZ -
     GARMIN_PKTHDR_LENGTH (matching the existing len >= GPS_IN_BUFSIZ
     check at the top of the loop) prevents both negative/underflowed
     lengths (< 12 bytes) and oversized lengths without needing to touch
     getDataLength() across the file.
  2. The out-of-bounds read in garmin_write_bulk() and
     garmin_write_bulk_callback() was only reachable because
     nat_receive() allowed len to underflow below GARMIN_PKTHDR_LENGTH
     (12 bytes). All other callers of garmin_write_bulk() always pass at
     least GARMIN_PKTHDR_LENGTH bytes, making a separate length check in
     garmin_write_bulk_callback() redundant.
  3. In MODE_GARMIN_SERIAL, gsp_receive() already bounds insize to
     MAX_SERIAL_PKT_SIZ + 2 before calling gsp_send(), so the extra
     bounds check in gsp_send() was an unrelated defensive check.
- Added Cc: stable@vger.kernel.org and QEMU reproduction details.

 drivers/usb/serial/garmin_gps.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/serial/garmin_gps.c b/drivers/usb/serial/garmin_gps.c
index 8020149f5658..a3f599043a4d 100644
--- a/drivers/usb/serial/garmin_gps.c
+++ b/drivers/usb/serial/garmin_gps.c
@@ -769,8 +769,14 @@ static int nat_receive(struct garmin_data *garmin_data_p,
 
 		/* do we have a complete packet ? */
 		if (garmin_data_p->insize >= GARMIN_PKTHDR_LENGTH) {
-			len = GARMIN_PKTHDR_LENGTH+
-			   getDataLength(garmin_data_p->inbuffer);
+			u32 dlen = getDataLength(garmin_data_p->inbuffer);
+
+			if (dlen >= GPS_IN_BUFSIZ - GARMIN_PKTHDR_LENGTH) {
+				garmin_data_p->insize = 0;
+				result = -EINVPKT;
+				break;
+			}
+			len = GARMIN_PKTHDR_LENGTH + dlen;
 			if (garmin_data_p->insize >= len) {
 				garmin_write_bulk(garmin_data_p->port,
 						   garmin_data_p->inbuffer,
-- 
2.49.0

Re: [PATCH v4] USB: serial: garmin_gps: validate packet data length in nat_receive()

[krzk]

On Mon, 21 Sep 2026 03:02:36 +0000, Hui Peng wrote:
> In nat_receive() (called from garmin_write() when userspace writes to
> /dev/ttyUSBn in MODE_NATIVE mode), while garmin_data_p->insize is less
> than GARMIN_PKTHDR_LENGTH (12), the loop copies up to 12 bytes into
> garmin_data_p->inbuffer. Once garmin_data_p->insize reaches
> GARMIN_PKTHDR_LENGTH, nat_receive() computes the total packet size as:
> 
>   len = GARMIN_PKTHDR_LENGTH + getDataLength(garmin_data_p->inbuffer);
> 
> Because getDataLength() returns a signed int from __le32_to_cpup(), a
> 12-byte header with a negative 32-bit length field (such as 0xfffffff5,
> i.e., -11) causes len to underflow to a small positive value (12 + (-11)
> = 1) without any bounds check at that point. Then garmin_data_p->insize
> >= len (12 >= 1) evaluates to true and nat_receive() calls
> garmin_write_bulk(port, inbuffer, 1, 0), which allocates a 1-byte slab
> buffer via kmemdup() and immediately reads 4 bytes from it in
> getLayerId(buffer) (and again in garmin_write_bulk_callback() when the
> URB completes):
> 
>   BUG: KASAN: slab-out-of-bounds in garmin_write_bulk.constprop.0+0x3eb/0x500
>   Read of size 4 at addr ffff888002cc0ba0 by task init/1
>   Call Trace:
>    <TASK>
>    dump_stack_lvl+0x70/0xa0
>    print_report+0x153/0x4c6
>    kasan_report+0xf1/0x120
>    garmin_write_bulk.constprop.0+0x3eb/0x500
>    garmin_write+0x60f/0x1450
>    serial_write+0x123/0x200
>    n_tty_write+0x8ea/0xeb0
>    file_tty_write.isra.0+0x44f/0x7a0
>    vfs_write+0x671/0xd20
> 
> Validate the unsigned 32-bit data length (dlen >= GPS_IN_BUFSIZ -
> GARMIN_PKTHDR_LENGTH, matching the existing len >= GPS_IN_BUFSIZ bound)
> in nat_receive() as soon as the 12-byte header is present, resetting
> insize and returning -EINVPKT on invalid lengths so garmin_write_bulk()
> is only ever called with a full valid packet header (12 <= len <
> GPS_IN_BUFSIZ).
> 
> Tested in QEMU against Linux 7.3.0-rc3 by emulating a Garmin USB GPS
> device (091e:0003) via dummy_hcd + raw-gadget, switching /dev/ttyUSB0
> to MODE_NATIVE via PRIV_PKTID_SET_MODE, and writing a 12-byte native
> packet with data length 0xfffffff5: on the unfixed kernel this triggers
> KASAN slab-out-of-bounds reads in garmin_write_bulk() and
> garmin_write_bulk_callback(), whereas on the fixed kernel nat_receive()
> rejects the packet with -EINVPKT and 0 KASAN faults.
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Hui Peng <benquike@gmail.com>
> ---
> Changes in v4:
> - Kept the fix strictly inside nat_receive() and dropped the other hunks
>   from v3 (garmin_write_bulk_callback(), gsp_send(), and changing the
>   return type of getDataLength() / getPacketId()) per Greg
>   Kroah-Hartman:
>   1. Assigning u32 dlen = getDataLength(garmin_data_p->inbuffer) locally
>      in nat_receive() and rejecting dlen >= GPS_IN_BUFSIZ -
>      GARMIN_PKTHDR_LENGTH (matching the existing len >= GPS_IN_BUFSIZ
>      check at the top of the loop) prevents both negative/underflowed
>      lengths (< 12 bytes) and oversized lengths without needing to touch
>      getDataLength() across the file.
>   2. The out-of-bounds read in garmin_write_bulk() and
>      garmin_write_bulk_callback() was only reachable because
>      nat_receive() allowed len to underflow below GARMIN_PKTHDR_LENGTH
>      (12 bytes). All other callers of garmin_write_bulk() always pass at
>      least GARMIN_PKTHDR_LENGTH bytes, making a separate length check in
>      garmin_write_bulk_callback() redundant.
>   3. In MODE_GARMIN_SERIAL, gsp_receive() already bounds insize to
>      MAX_SERIAL_PKT_SIZ + 2 before calling gsp_send(), so the extra
>      bounds check in gsp_send() was an unrelated defensive check.
> - Added Cc: stable@vger.kernel.org and QEMU reproduction details.
> 
>  drivers/usb/serial/garmin_gps.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 


You sent multiple independent patches, to multiple independent
subsystems. The amount of these patches clearly suggest this was
AI generated and most likely not tested.

More importantly, you sent all this work without properly organizing
relevant patches into patchsets. This makes reviewing difficult
and might cause multiple reviewers to address the same issue.
Replying to the entire set is impossible and requires handling each
patch independently, instead of applying or discarding the set.
Maintainers also won't see the bigger picture of your work. Quite
worrying.

This is on the verge of hostile patch: bomb us with so many
contributions, we won't be able to handle them in efficient manner,
like responding ONCE to ask you to slow down.  Considering all this
is untested and LLM generated, I have even more doubts whether this
should be considered for review.

Please read kernel documentation BEFORE posting more work. It will
explain you how to identify subsystems, how to organize your work per
subsystem, how to document usage of LLM and how what you should not
do if this was posted in a good faith.

Best regards,
Krzysztof

Re: [PATCH v4] USB: serial: garmin_gps: validate packet data length in nat_receive()

[krzk]

On Mon, 21 Sep 2026 03:02:36 +0000, Hui Peng wrote:
> In nat_receive() (called from garmin_write() when userspace writes to
> /dev/ttyUSBn in MODE_NATIVE mode), while garmin_data_p->insize is less
> than GARMIN_PKTHDR_LENGTH (12), the loop copies up to 12 bytes into
> garmin_data_p->inbuffer. Once garmin_data_p->insize reaches
> GARMIN_PKTHDR_LENGTH, nat_receive() computes the total packet size as:
> 
>   len = GARMIN_PKTHDR_LENGTH + getDataLength(garmin_data_p->inbuffer);
> 
> Because getDataLength() returns a signed int from __le32_to_cpup(), a
> 12-byte header with a negative 32-bit length field (such as 0xfffffff5,
> i.e., -11) causes len to underflow to a small positive value (12 + (-11)
> = 1) without any bounds check at that point. Then garmin_data_p->insize
> >= len (12 >= 1) evaluates to true and nat_receive() calls
> garmin_write_bulk(port, inbuffer, 1, 0), which allocates a 1-byte slab
> buffer via kmemdup() and immediately reads 4 bytes from it in
> getLayerId(buffer) (and again in garmin_write_bulk_callback() when the
> URB completes):
> 
>   BUG: KASAN: slab-out-of-bounds in garmin_write_bulk.constprop.0+0x3eb/0x500
>   Read of size 4 at addr ffff888002cc0ba0 by task init/1
>   Call Trace:
>    <TASK>
>    dump_stack_lvl+0x70/0xa0
>    print_report+0x153/0x4c6
>    kasan_report+0xf1/0x120
>    garmin_write_bulk.constprop.0+0x3eb/0x500
>    garmin_write+0x60f/0x1450
>    serial_write+0x123/0x200
>    n_tty_write+0x8ea/0xeb0
>    file_tty_write.isra.0+0x44f/0x7a0
>    vfs_write+0x671/0xd20
> 
> Validate the unsigned 32-bit data length (dlen >= GPS_IN_BUFSIZ -
> GARMIN_PKTHDR_LENGTH, matching the existing len >= GPS_IN_BUFSIZ bound)
> in nat_receive() as soon as the 12-byte header is present, resetting
> insize and returning -EINVPKT on invalid lengths so garmin_write_bulk()
> is only ever called with a full valid packet header (12 <= len <
> GPS_IN_BUFSIZ).
> 
> Tested in QEMU against Linux 7.3.0-rc3 by emulating a Garmin USB GPS
> device (091e:0003) via dummy_hcd + raw-gadget, switching /dev/ttyUSB0
> to MODE_NATIVE via PRIV_PKTID_SET_MODE, and writing a 12-byte native
> packet with data length 0xfffffff5: on the unfixed kernel this triggers
> KASAN slab-out-of-bounds reads in garmin_write_bulk() and
> garmin_write_bulk_callback(), whereas on the fixed kernel nat_receive()
> rejects the packet with -EINVPKT and 0 KASAN faults.
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Hui Peng <benquike@gmail.com>


You sent multiple independent patches, to multiple independent
subsystems. The amount of these patches clearly suggest this was
AI generated and most likely not tested.

More importantly, you sent all this work without properly organizing
relevant patches into patchsets. This makes reviewing difficult
and might cause multiple reviewers to address the same issue.
Replying to the entire set is impossible and requires handling each
patch independently, instead of applying or discarding the set.
Maintainers also won't see the bigger picture of your work. Quite
worrying.

This is on the verge of hostile patch: bomb us with so many
contributions, we won't be able to handle them in efficient manner,
like responding ONCE to ask you to slow down.  Considering all this
is untested and LLM generated, I have even more doubts whether this
should be considered for review.

Please read kernel documentation BEFORE posting more work. It will
explain you how to identify subsystems, how to organize your work per
subsystem, how to document usage of LLM and how what you should not
do if this was posted in a good faith.

Best regards,
Krzysztof

[PATCH v4] USB: serial: garmin_gps: validate packet data length in nat_receive()

[Hui Peng]

In nat_receive() (called from garmin_write() when userspace writes to
/dev/ttyUSBn in MODE_NATIVE mode), while garmin_data_p->insize is less
than GARMIN_PKTHDR_LENGTH (12), the loop copies up to 12 bytes into
garmin_data_p->inbuffer. Once garmin_data_p->insize reaches
GARMIN_PKTHDR_LENGTH, nat_receive() computes the total packet size as:

  len = GARMIN_PKTHDR_LENGTH + getDataLength(garmin_data_p->inbuffer);

Because getDataLength() returns a signed int from __le32_to_cpup(), a
12-byte header with a negative 32-bit length field (such as 0xfffffff5,
i.e., -11) causes len to underflow to a small positive value (12 + (-11)
= 1) without any bounds check at that point. Then garmin_data_p->insize
>= len (12 >= 1) evaluates to true and nat_receive() calls
garmin_write_bulk(port, inbuffer, 1, 0), which allocates a 1-byte slab
buffer via kmemdup() and immediately reads 4 bytes from it in
getLayerId(buffer) (and again in garmin_write_bulk_callback() when the
URB completes):

  BUG: KASAN: slab-out-of-bounds in garmin_write_bulk.constprop.0+0x3eb/0x500
  Read of size 4 at addr ffff888002cc0ba0 by task init/1
  Call Trace:
   <TASK>
   dump_stack_lvl+0x70/0xa0
   print_report+0x153/0x4c6
   kasan_report+0xf1/0x120
   garmin_write_bulk.constprop.0+0x3eb/0x500
   garmin_write+0x60f/0x1450
   serial_write+0x123/0x200
   n_tty_write+0x8ea/0xeb0
   file_tty_write.isra.0+0x44f/0x7a0
   vfs_write+0x671/0xd20

Validate the unsigned 32-bit data length (dlen >= GPS_IN_BUFSIZ -
GARMIN_PKTHDR_LENGTH, matching the existing len >= GPS_IN_BUFSIZ bound)
in nat_receive() as soon as the 12-byte header is present, resetting
insize and returning -EINVPKT on invalid lengths so garmin_write_bulk()
is only ever called with a full valid packet header (12 <= len <
GPS_IN_BUFSIZ).

Tested in QEMU against Linux 7.3.0-rc3 by emulating a Garmin USB GPS
device (091e:0003) via dummy_hcd + raw-gadget, switching /dev/ttyUSB0
to MODE_NATIVE via PRIV_PKTID_SET_MODE, and writing a 12-byte native
packet with data length 0xfffffff5: on the unfixed kernel this triggers
KASAN slab-out-of-bounds reads in garmin_write_bulk() and
garmin_write_bulk_callback(), whereas on the fixed kernel nat_receive()
rejects the packet with -EINVPKT and 0 KASAN faults.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Hui Peng <benquike@gmail.com>
---
Changes in v4:
- Kept the fix strictly inside nat_receive() and dropped the other hunks
  from v3 (garmin_write_bulk_callback(), gsp_send(), and changing the
  return type of getDataLength() / getPacketId()) per Greg
  Kroah-Hartman:
  1. Assigning u32 dlen = getDataLength(garmin_data_p->inbuffer) locally
     in nat_receive() and rejecting dlen >= GPS_IN_BUFSIZ -
     GARMIN_PKTHDR_LENGTH (matching the existing len >= GPS_IN_BUFSIZ
     check at the top of the loop) prevents both negative/underflowed
     lengths (< 12 bytes) and oversized lengths without needing to touch
     getDataLength() across the file.
  2. The out-of-bounds read in garmin_write_bulk() and
     garmin_write_bulk_callback() was only reachable because
     nat_receive() allowed len to underflow below GARMIN_PKTHDR_LENGTH
     (12 bytes). All other callers of garmin_write_bulk() always pass at
     least GARMIN_PKTHDR_LENGTH bytes, making a separate length check in
     garmin_write_bulk_callback() redundant.
  3. In MODE_GARMIN_SERIAL, gsp_receive() already bounds insize to
     MAX_SERIAL_PKT_SIZ + 2 before calling gsp_send(), so the extra
     bounds check in gsp_send() was an unrelated defensive check.
- Added Cc: stable@vger.kernel.org and QEMU reproduction details.

 drivers/usb/serial/garmin_gps.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/serial/garmin_gps.c b/drivers/usb/serial/garmin_gps.c
index 8020149f5658..a3f599043a4d 100644
--- a/drivers/usb/serial/garmin_gps.c
+++ b/drivers/usb/serial/garmin_gps.c
@@ -769,8 +769,14 @@ static int nat_receive(struct garmin_data *garmin_data_p,
 
 		/* do we have a complete packet ? */
 		if (garmin_data_p->insize >= GARMIN_PKTHDR_LENGTH) {
-			len = GARMIN_PKTHDR_LENGTH+
-			   getDataLength(garmin_data_p->inbuffer);
+			u32 dlen = getDataLength(garmin_data_p->inbuffer);
+
+			if (dlen >= GPS_IN_BUFSIZ - GARMIN_PKTHDR_LENGTH) {
+				garmin_data_p->insize = 0;
+				result = -EINVPKT;
+				break;
+			}
+			len = GARMIN_PKTHDR_LENGTH + dlen;
 			if (garmin_data_p->insize >= len) {
 				garmin_write_bulk(garmin_data_p->port,
 						   garmin_data_p->inbuffer,
-- 
2.49.0

Re: [PATCH v4] USB: serial: garmin_gps: validate packet data length in nat_receive()

[Johan Hovold]

On Wed, Sep 30, 2026 at 07:52:47AM +0000, Hui Peng wrote:
> In nat_receive() (called from garmin_write() when userspace writes to
> /dev/ttyUSBn in MODE_NATIVE mode), while garmin_data_p->insize is less
> than GARMIN_PKTHDR_LENGTH (12), the loop copies up to 12 bytes into
> garmin_data_p->inbuffer. Once garmin_data_p->insize reaches
> GARMIN_PKTHDR_LENGTH, nat_receive() computes the total packet size as:
> 
>   len = GARMIN_PKTHDR_LENGTH + getDataLength(garmin_data_p->inbuffer);
> 
> Because getDataLength() returns a signed int from __le32_to_cpup(), a
> 12-byte header with a negative 32-bit length field (such as 0xfffffff5,
> i.e., -11) causes len to underflow to a small positive value (12 + (-11)
> = 1) without any bounds check at that point. Then garmin_data_p->insize
> >= len (12 >= 1) evaluates to true and nat_receive() calls
> garmin_write_bulk(port, inbuffer, 1, 0), which allocates a 1-byte slab
> buffer via kmemdup() and immediately reads 4 bytes from it in
> getLayerId(buffer) (and again in garmin_write_bulk_callback() when the
> URB completes):

> Validate the unsigned 32-bit data length (dlen >= GPS_IN_BUFSIZ -
> GARMIN_PKTHDR_LENGTH, matching the existing len >= GPS_IN_BUFSIZ bound)
> in nat_receive() as soon as the 12-byte header is present, resetting
> insize and returning -EINVPKT on invalid lengths so garmin_write_bulk()
> is only ever called with a full valid packet header (12 <= len <
> GPS_IN_BUFSIZ).

> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Hui Peng <benquike@gmail.com>
> ---
> Changes in v4:
> - Kept the fix strictly inside nat_receive() and dropped the other hunks
>   from v3 (garmin_write_bulk_callback(), gsp_send(), and changing the
>   return type of getDataLength() / getPacketId()) per Greg
>   Kroah-Hartman:
>   1. Assigning u32 dlen = getDataLength(garmin_data_p->inbuffer) locally
>      in nat_receive() and rejecting dlen >= GPS_IN_BUFSIZ -
>      GARMIN_PKTHDR_LENGTH (matching the existing len >= GPS_IN_BUFSIZ
>      check at the top of the loop) prevents both negative/underflowed
>      lengths (< 12 bytes) and oversized lengths without needing to touch
>      getDataLength() across the file.
>   2. The out-of-bounds read in garmin_write_bulk() and
>      garmin_write_bulk_callback() was only reachable because
>      nat_receive() allowed len to underflow below GARMIN_PKTHDR_LENGTH
>      (12 bytes). All other callers of garmin_write_bulk() always pass at
>      least GARMIN_PKTHDR_LENGTH bytes, making a separate length check in
>      garmin_write_bulk_callback() redundant.
>   3. In MODE_GARMIN_SERIAL, gsp_receive() already bounds insize to
>      MAX_SERIAL_PKT_SIZ + 2 before calling gsp_send(), so the extra
>      bounds check in gsp_send() was an unrelated defensive check.
> - Added Cc: stable@vger.kernel.org and QEMU reproduction details.
> 
>  drivers/usb/serial/garmin_gps.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/serial/garmin_gps.c b/drivers/usb/serial/garmin_gps.c
> index 8020149f5658..a3f599043a4d 100644
> --- a/drivers/usb/serial/garmin_gps.c
> +++ b/drivers/usb/serial/garmin_gps.c
> @@ -769,8 +769,14 @@ static int nat_receive(struct garmin_data *garmin_data_p,
>  
>  		/* do we have a complete packet ? */
>  		if (garmin_data_p->insize >= GARMIN_PKTHDR_LENGTH) {
> -			len = GARMIN_PKTHDR_LENGTH+
> -			   getDataLength(garmin_data_p->inbuffer);
> +			u32 dlen = getDataLength(garmin_data_p->inbuffer);
> +
> +			if (dlen >= GPS_IN_BUFSIZ - GARMIN_PKTHDR_LENGTH) {
> +				garmin_data_p->insize = 0;
> +				result = -EINVPKT;
> +				break;
> +			}
> +			len = GARMIN_PKTHDR_LENGTH + dlen;

Shouldn't you fix the sanity check at the top of the loop instead so
that we never reach this conditional in case of a malformed header?

>  			if (garmin_data_p->insize >= len) {
>  				garmin_write_bulk(garmin_data_p->port,
>  						   garmin_data_p->inbuffer,

Johan

Re: [PATCH v4] USB: serial: garmin_gps: validate packet data length in nat_receive()

[Johan Hovold]

On Fri, Oct 02, 2026 at 11:50:08AM +0200, Johan Hovold wrote:
> On Wed, Sep 30, 2026 at 07:52:47AM +0000, Hui Peng wrote:
> > In nat_receive() (called from garmin_write() when userspace writes to
> > /dev/ttyUSBn in MODE_NATIVE mode), while garmin_data_p->insize is less
> > than GARMIN_PKTHDR_LENGTH (12), the loop copies up to 12 bytes into
> > garmin_data_p->inbuffer. Once garmin_data_p->insize reaches
> > GARMIN_PKTHDR_LENGTH, nat_receive() computes the total packet size as:
> > 
> >   len = GARMIN_PKTHDR_LENGTH + getDataLength(garmin_data_p->inbuffer);
> > 
> > Because getDataLength() returns a signed int from __le32_to_cpup(), a
> > 12-byte header with a negative 32-bit length field (such as 0xfffffff5,
> > i.e., -11) causes len to underflow to a small positive value (12 + (-11)
> > = 1) without any bounds check at that point. Then garmin_data_p->insize
> > >= len (12 >= 1) evaluates to true and nat_receive() calls
> > garmin_write_bulk(port, inbuffer, 1, 0), which allocates a 1-byte slab
> > buffer via kmemdup() and immediately reads 4 bytes from it in
> > getLayerId(buffer) (and again in garmin_write_bulk_callback() when the
> > URB completes):
> 
> > Validate the unsigned 32-bit data length (dlen >= GPS_IN_BUFSIZ -
> > GARMIN_PKTHDR_LENGTH, matching the existing len >= GPS_IN_BUFSIZ bound)
> > in nat_receive() as soon as the 12-byte header is present, resetting
> > insize and returning -EINVPKT on invalid lengths so garmin_write_bulk()
> > is only ever called with a full valid packet header (12 <= len <
> > GPS_IN_BUFSIZ).
> 
> > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> > Cc: stable@vger.kernel.org
> > Assisted-by: LLM
> > Signed-off-by: Hui Peng <benquike@gmail.com>

> >  drivers/usb/serial/garmin_gps.c | 10 ++++++++--
> >  1 file changed, 8 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/usb/serial/garmin_gps.c b/drivers/usb/serial/garmin_gps.c
> > index 8020149f5658..a3f599043a4d 100644
> > --- a/drivers/usb/serial/garmin_gps.c
> > +++ b/drivers/usb/serial/garmin_gps.c
> > @@ -769,8 +769,14 @@ static int nat_receive(struct garmin_data *garmin_data_p,
> >  
> >  		/* do we have a complete packet ? */
> >  		if (garmin_data_p->insize >= GARMIN_PKTHDR_LENGTH) {
> > -			len = GARMIN_PKTHDR_LENGTH+
> > -			   getDataLength(garmin_data_p->inbuffer);
> > +			u32 dlen = getDataLength(garmin_data_p->inbuffer);
> > +
> > +			if (dlen >= GPS_IN_BUFSIZ - GARMIN_PKTHDR_LENGTH) {
> > +				garmin_data_p->insize = 0;
> > +				result = -EINVPKT;
> > +				break;
> > +			}
> > +			len = GARMIN_PKTHDR_LENGTH + dlen;
> 
> Shouldn't you fix the sanity check at the top of the loop instead so
> that we never reach this conditional in case of a malformed header?

I took another look at this and this check needs to go here where you
have it. The check at the start of loop now essentially become redundant
however. I'll keep it as a safe guard (e.g. to not rely on the driver
clearing the buffer when switching mode) but add a comment about it
being redundant, while also moving the comment and dev_dbg().

> >  			if (garmin_data_p->insize >= len) {
> >  				garmin_write_bulk(garmin_data_p->port,
> >  						   garmin_data_p->inbuffer,

I'll queue this one up with my amendments when the merge window closes
this time.

But in the future, don't spam the lists with dozens of LLM generated
patches while ignoring review feedback for weeks, or your patches will
just be treated as LLM generated reports.
 
Johan