* [PATCH v2] [USB][serial/garmin_gps] Fix signed integer underflow and OOB read on packet length
[not found] <2026091908-imprint-rejoicing-6933@gregkh>
@ 2026-09-19 8:08 ` Hui Peng
2026-09-19 11:28 ` [PATCH v3] USB: serial: garmin_gps: fix " Hui Peng
0 siblings, 1 reply; 6+ messages in thread
From: Hui Peng @ 2026-09-19 8:08 UTC (permalink / raw)
To: Johan Hovold, Greg Kroah-Hartman; +Cc: linux-usb, linux-kernel, Hui Peng
getDataLength() returns a signed int from __le32_to_cpup((__le32
*)(garmin_data_p + 8)). When bit 31 is set (e.g. 0x80000004), len is
negative (-2147483644), bypassing the upper bound check (GSP_INITIAL_OFFSET
+ len > GSP_MAX_BUFSIZ) in gsp_send() and triggering a KASAN slab-out-of-
bounds read in garmin_write_bulk() when GARMIN_PKTHDR_LENGTH + len wraps
around to 16.
Kernel stack trace:
BUG: KASAN: slab-out-of-bounds in garmin_write_bulk+0x164/0x3b0
Read of size 16 at addr ffff88800791400c by task poc_verify/188
Call Trace:
<TASK>
dump_stack_lvl+0x4d/0x70
print_report+0xc4/0x610
kasan_report+0xb8/0xf0
kasan_check_range+0x118/0x190
memcpy+0x24/0x60
garmin_write_bulk+0x164/0x3b0
gsp_send+0x218/0x490
garmin_write+0x142/0x2c0
tty_write+0x294/0x540
vfs_write+0x412/0x640
</TASK>
Note that garmin_write_bulk() and garmin_write_bulk_callback() are
triggered from local userspace calling write(fd, ...) on /dev/ttyUSB0
(even with a normal USB device attached) by writing a 12-byte packet
with a negative 32-bit length field (e.g. 0x80ffffff) at byte offset 4.
Kernel stack trace (Linux 7.3.0-rc3):
==================================================================
BUG: KASAN: slab-out-of-bounds in garmin_write_bulk+0x922/0xbf0
Read of size 12 at addr ffff88800f8ef958 by task usb_poc_verify/162
Call Trace:
<TASK>
dump_stack_lvl+0x70/0xa0
print_report+0x153/0x4c6
kasan_report+0xf1/0x120
kasan_check_range+0x11c/0x200
__asan_memcpy+0x29/0x70
garmin_write_bulk+0x922/0xbf0
garmin_write+0x3e6/0x770
serial_write+0x167/0x2b0
n_tty_write+0x5f4/0x1020
file_tty_write.isra.0+0x411/0x760
vfs_write+0x671/0xd20
ksys_write+0x1bb/0x210
do_syscall_64+0xda/0x4b0
</TASK>
==================================================================
Assisted-by: LLM
Signed-off-by: Hui Peng <benquike@gmail.com>
---
v2: Send to linux-usb@vger.kernel.org via git send-email with Assisted-by: LLM tag and clarify local userspace write() trigger.
drivers/usb/serial/garmin_gps.c | 20 +++++++++++++++-----
1 file changed, 15 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/serial/garmin_gps.c b/drivers/usb/serial/garmin_gps.c
index 8020149f5..ea10af159 100644
--- a/drivers/usb/serial/garmin_gps.c
+++ b/drivers/usb/serial/garmin_gps.c
@@ -208,12 +208,12 @@ static inline int getLayerId(const __u8 *usbPacket)
return __le32_to_cpup((__le32 *)(usbPacket));
}
-static inline int getPacketId(const __u8 *usbPacket)
+static inline __u32 getPacketId(const __u8 *usbPacket)
{
return __le32_to_cpup((__le32 *)(usbPacket+4));
}
-static inline int getDataLength(const __u8 *usbPacket)
+static inline __u32 getDataLength(const __u8 *usbPacket)
{
return __le32_to_cpup((__le32 *)(usbPacket+8));
}
@@ -607,6 +607,10 @@ static int gsp_send(struct garmin_data *garmin_data_p,
if (k >= GARMIN_PKTHDR_LENGTH) {
pktid = getPacketId(garmin_data_p->outbuffer);
datalen = getDataLength(garmin_data_p->outbuffer);
+ if (datalen < 0 || datalen > GPS_OUT_BUFSIZ - GARMIN_PKTHDR_LENGTH) {
+ garmin_data_p->outsize = 0;
+ return -3;
+ }
i = GARMIN_PKTHDR_LENGTH + datalen;
if (k < i)
return 0;
@@ -769,8 +773,13 @@ 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;
+ break;
+ }
+ len = GARMIN_PKTHDR_LENGTH + dlen;
if (garmin_data_p->insize >= len) {
garmin_write_bulk(garmin_data_p->port,
garmin_data_p->inbuffer,
@@ -951,7 +960,8 @@ static void garmin_write_bulk_callback(struct urb *urb)
struct garmin_data *garmin_data_p =
usb_get_serial_port_data(port);
- if (getLayerId(urb->transfer_buffer) == GARMIN_LAYERID_APPL) {
+ if (urb->transfer_buffer_length >= 5 &&
+ getLayerId(urb->transfer_buffer) == GARMIN_LAYERID_APPL) {
if (garmin_data_p->mode == MODE_GARMIN_SERIAL) {
gsp_send_ack(garmin_data_p,
--
2.55.0.1082.g2b9226bbc0-goog
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v3] USB: serial: garmin_gps: fix signed integer underflow and OOB read on packet length
2026-09-19 8:08 ` [PATCH v2] [USB][serial/garmin_gps] Fix signed integer underflow and OOB read on packet length Hui Peng
@ 2026-09-19 11:28 ` Hui Peng
2026-09-20 5:14 ` Greg KH
0 siblings, 1 reply; 6+ messages in thread
From: Hui Peng @ 2026-09-19 11:28 UTC (permalink / raw)
To: johan, gregkh; +Cc: linux-usb, linux-kernel
getDataLength() returns a signed int from __le32_to_cpup((__le32
*)(garmin_data_p + 8)). When bit 31 is set (e.g. 0x80000004), len is
negative (-2147483644), bypassing the upper bound check (GSP_INITIAL_OFFSET
+ len > GSP_MAX_BUFSIZ) in gsp_send() and triggering a KASAN slab-out-of-
bounds read in garmin_write_bulk() when GARMIN_PKTHDR_LENGTH + len wraps
around to 16.
Kernel stack trace:
BUG: KASAN: slab-out-of-bounds in garmin_write_bulk+0x164/0x3b0
Read of size 16 at addr ffff88800791400c by task poc_verify/188
Call Trace:
<TASK>
dump_stack_lvl+0x4d/0x70
print_report+0xc4/0x610
kasan_report+0xb8/0xf0
kasan_check_range+0x118/0x190
memcpy+0x24/0x60
garmin_write_bulk+0x164/0x3b0
gsp_send+0x218/0x490
garmin_write+0x142/0x2c0
tty_write+0x294/0x540
vfs_write+0x412/0x640
</TASK>
Note that garmin_write_bulk() and garmin_write_bulk_callback() are
triggered from local userspace calling write(fd, ...) on /dev/ttyUSB0
(even with a normal USB device attached) by writing a 12-byte packet
with a negative 32-bit length field (e.g. 0x80ffffff) at byte offset 4.
Kernel stack trace (Linux 7.3.0-rc3):
==================================================================
BUG: KASAN: slab-out-of-bounds in garmin_write_bulk+0x922/0xbf0
Read of size 12 at addr ffff88800f8ef958 by task usb_poc_verify/162
Call Trace:
<TASK>
dump_stack_lvl+0x70/0xa0
print_report+0x153/0x4c6
kasan_report+0xf1/0x120
kasan_check_range+0x11c/0x200
__asan_memcpy+0x29/0x70
garmin_write_bulk+0x922/0xbf0
garmin_write+0x3e6/0x770
serial_write+0x167/0x2b0
n_tty_write+0x5f4/0x1020
file_tty_write.isra.0+0x411/0x760
vfs_write+0x671/0xd20
ksys_write+0x1bb/0x210
do_syscall_64+0xda/0x4b0
</TASK>
==================================================================
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Assisted-by: LLM
Signed-off-by: Hui Peng <benquike@gmail.com>
---
v3: Add a Fixes: tag and use the conventional subject prefix.
The length arithmetic in gsp_send() and nat_receive() is unchanged
since the initial git import - garmin_gps.c is already present in
1da177e4c3f4 with the same signed getDataLength() and the same missing
bound - so that is the tag. The later commits git blame surfaces are
not introducers: af6d780b5787 ("garmin_gps: Coding style") and
fb571101af63 ("USB: serial: fix compare_const_fl.cocci warnings") are
cosmetic, and b4072f46e57f only re-nested an existing condition.
Greg asked on an earlier posting whether this is an untrusted-device
issue or something a local user can trigger. To be explicit: hunks 1
and 2 are reached from userspace. gsp_send() consumes the buffer that
write() fills via garmin_write(), so a local user with access to
/dev/ttyUSBn can drive the length arithmetic directly without any
malicious hardware. Hunk 3 is the device-input side and is plain
hardening in the sense of
Documentation/process/threat-model.rst - we trust the device, and I am
not claiming otherwise.
On scope, since it is fair to ask why hunk 3 is here at all: the
urb->transfer_buffer_length >= 5 guard in
garmin_write_bulk_callback() covers a read added later, by
468d13623b6c ("USB: serial: garmin_gps: fixes package loss if used
from gpsbabel"). I kept it in the same patch because the short buffer
it reads past is produced by the same unvalidated length arithmetic,
but I am happy to split it out if you would rather have it separate.
The datalen < 0 test does still do something despite the accessors
becoming __u32: gsp_send() assigns into an int datalen. If you would
rather I made datalen a u32 and kept only the upper bound, say so and
I will respin.
v2: Send to linux-usb@vger.kernel.org via git send-email with Assisted-by: LLM tag and clarify local userspace write() trigger.
drivers/usb/serial/garmin_gps.c | 20 +++++++++++++++-----
1 file changed, 15 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/serial/garmin_gps.c b/drivers/usb/serial/garmin_gps.c
index 8020149f5..ea10af159 100644
--- a/drivers/usb/serial/garmin_gps.c
+++ b/drivers/usb/serial/garmin_gps.c
@@ -208,12 +208,12 @@ static inline int getLayerId(const __u8 *usbPacket)
return __le32_to_cpup((__le32 *)(usbPacket));
}
-static inline int getPacketId(const __u8 *usbPacket)
+static inline __u32 getPacketId(const __u8 *usbPacket)
{
return __le32_to_cpup((__le32 *)(usbPacket+4));
}
-static inline int getDataLength(const __u8 *usbPacket)
+static inline __u32 getDataLength(const __u8 *usbPacket)
{
return __le32_to_cpup((__le32 *)(usbPacket+8));
}
@@ -607,6 +607,10 @@ static int gsp_send(struct garmin_data *garmin_data_p,
if (k >= GARMIN_PKTHDR_LENGTH) {
pktid = getPacketId(garmin_data_p->outbuffer);
datalen = getDataLength(garmin_data_p->outbuffer);
+ if (datalen < 0 || datalen > GPS_OUT_BUFSIZ - GARMIN_PKTHDR_LENGTH) {
+ garmin_data_p->outsize = 0;
+ return -3;
+ }
i = GARMIN_PKTHDR_LENGTH + datalen;
if (k < i)
return 0;
@@ -769,8 +773,13 @@ 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;
+ break;
+ }
+ len = GARMIN_PKTHDR_LENGTH + dlen;
if (garmin_data_p->insize >= len) {
garmin_write_bulk(garmin_data_p->port,
garmin_data_p->inbuffer,
@@ -951,7 +960,8 @@ static void garmin_write_bulk_callback(struct urb *urb)
struct garmin_data *garmin_data_p =
usb_get_serial_port_data(port);
- if (getLayerId(urb->transfer_buffer) == GARMIN_LAYERID_APPL) {
+ if (urb->transfer_buffer_length >= 5 &&
+ getLayerId(urb->transfer_buffer) == GARMIN_LAYERID_APPL) {
if (garmin_data_p->mode == MODE_GARMIN_SERIAL) {
gsp_send_ack(garmin_data_p,
--
2.55.0.1082.g2b9226bbc0-goog
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3] USB: serial: garmin_gps: fix signed integer underflow and OOB read on packet length
2026-09-19 11:28 ` [PATCH v3] USB: serial: garmin_gps: fix " Hui Peng
@ 2026-09-20 5:14 ` Greg KH
2026-09-21 3:02 ` [PATCH v4] USB: serial: garmin_gps: validate packet data length in nat_receive() Hui Peng
0 siblings, 1 reply; 6+ messages in thread
From: Greg KH @ 2026-09-20 5:14 UTC (permalink / raw)
To: Hui Peng; +Cc: johan, linux-usb, linux-kernel
On Sat, Sep 19, 2026 at 11:28:18AM +0000, Hui Peng wrote:
> getDataLength() returns a signed int from __le32_to_cpup((__le32
> *)(garmin_data_p + 8)). When bit 31 is set (e.g. 0x80000004), len is
> negative (-2147483644), bypassing the upper bound check (GSP_INITIAL_OFFSET
> + len > GSP_MAX_BUFSIZ) in gsp_send() and triggering a KASAN slab-out-of-
> bounds read in garmin_write_bulk() when GARMIN_PKTHDR_LENGTH + len wraps
> around to 16.
>
> Kernel stack trace:
> BUG: KASAN: slab-out-of-bounds in garmin_write_bulk+0x164/0x3b0
> Read of size 16 at addr ffff88800791400c by task poc_verify/188
> Call Trace:
> <TASK>
> dump_stack_lvl+0x4d/0x70
> print_report+0xc4/0x610
> kasan_report+0xb8/0xf0
> kasan_check_range+0x118/0x190
> memcpy+0x24/0x60
> garmin_write_bulk+0x164/0x3b0
> gsp_send+0x218/0x490
> garmin_write+0x142/0x2c0
> tty_write+0x294/0x540
> vfs_write+0x412/0x640
> </TASK>
>
>
> Note that garmin_write_bulk() and garmin_write_bulk_callback() are
> triggered from local userspace calling write(fd, ...) on /dev/ttyUSB0
> (even with a normal USB device attached) by writing a 12-byte packet
> with a negative 32-bit length field (e.g. 0x80ffffff) at byte offset 4.
>
> Kernel stack trace (Linux 7.3.0-rc3):
> ==================================================================
> BUG: KASAN: slab-out-of-bounds in garmin_write_bulk+0x922/0xbf0
> Read of size 12 at addr ffff88800f8ef958 by task usb_poc_verify/162
> Call Trace:
> <TASK>
> dump_stack_lvl+0x70/0xa0
> print_report+0x153/0x4c6
> kasan_report+0xf1/0x120
> kasan_check_range+0x11c/0x200
> __asan_memcpy+0x29/0x70
> garmin_write_bulk+0x922/0xbf0
> garmin_write+0x3e6/0x770
> serial_write+0x167/0x2b0
> n_tty_write+0x5f4/0x1020
> file_tty_write.isra.0+0x411/0x760
> vfs_write+0x671/0xd20
> ksys_write+0x1bb/0x210
> do_syscall_64+0xda/0x4b0
> </TASK>
> ==================================================================
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Assisted-by: LLM
> Signed-off-by: Hui Peng <benquike@gmail.com>
> ---
> v3: Add a Fixes: tag and use the conventional subject prefix.
>
> The length arithmetic in gsp_send() and nat_receive() is unchanged
> since the initial git import - garmin_gps.c is already present in
> 1da177e4c3f4 with the same signed getDataLength() and the same missing
> bound - so that is the tag. The later commits git blame surfaces are
> not introducers: af6d780b5787 ("garmin_gps: Coding style") and
> fb571101af63 ("USB: serial: fix compare_const_fl.cocci warnings") are
> cosmetic, and b4072f46e57f only re-nested an existing condition.
>
> Greg asked on an earlier posting whether this is an untrusted-device
> issue or something a local user can trigger. To be explicit: hunks 1
> and 2 are reached from userspace. gsp_send() consumes the buffer that
> write() fills via garmin_write(), so a local user with access to
> /dev/ttyUSBn can drive the length arithmetic directly without any
> malicious hardware. Hunk 3 is the device-input side and is plain
> hardening in the sense of
> Documentation/process/threat-model.rst - we trust the device, and I am
> not claiming otherwise.
So as this patch is doing multiple things, please split it into multiple
patches, like our documentation asks your LLM to do :)
thanks,
greg k-h
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v4] USB: serial: garmin_gps: validate packet data length in nat_receive()
2026-09-20 5:14 ` Greg KH
@ 2026-09-21 3:02 ` Hui Peng
2026-09-21 15:10 ` krzk
2026-09-21 15:16 ` krzk
0 siblings, 2 replies; 6+ messages in thread
From: Hui Peng @ 2026-09-21 3:02 UTC (permalink / raw)
To: Johan Hovold, Greg Kroah-Hartman
Cc: linux-usb, linux-kernel, stable, 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4] USB: serial: garmin_gps: validate packet data length in nat_receive()
2026-09-21 3:02 ` [PATCH v4] USB: serial: garmin_gps: validate packet data length in nat_receive() Hui Peng
@ 2026-09-21 15:10 ` krzk
2026-09-21 15:16 ` krzk
1 sibling, 0 replies; 6+ messages in thread
From: krzk @ 2026-09-21 15:10 UTC (permalink / raw)
To: Hui Peng
Cc: Johan Hovold, Greg Kroah-Hartman, stable, linux-usb, linux-kernel
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4] USB: serial: garmin_gps: validate packet data length in nat_receive()
2026-09-21 3:02 ` [PATCH v4] USB: serial: garmin_gps: validate packet data length in nat_receive() Hui Peng
2026-09-21 15:10 ` krzk
@ 2026-09-21 15:16 ` krzk
1 sibling, 0 replies; 6+ messages in thread
From: krzk @ 2026-09-21 15:16 UTC (permalink / raw)
To: Hui Peng
Cc: Johan Hovold, stable, Greg Kroah-Hartman, linux-kernel, linux-usb
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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-09-21 15:16 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <2026091908-imprint-rejoicing-6933@gregkh>
2026-09-19 8:08 ` [PATCH v2] [USB][serial/garmin_gps] Fix signed integer underflow and OOB read on packet length Hui Peng
2026-09-19 11:28 ` [PATCH v3] USB: serial: garmin_gps: fix " Hui Peng
2026-09-20 5:14 ` Greg KH
2026-09-21 3:02 ` [PATCH v4] USB: serial: garmin_gps: validate packet data length in nat_receive() Hui Peng
2026-09-21 15:10 ` krzk
2026-09-21 15:16 ` krzk
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®