Re: [PATCH] HID: mcp2221: Fix heap buffer overflow in mcp2221_raw_event()

[Benoît Sevens]

On Tue, 12 May 2026 at 17:47, Jiri Kosina <jikos@xxxxxxxxxx> wrote:
>
> On Wed, 15 Apr 2026, Benoit Sevens wrote:
>
> > From: Benoît Sevens <bsevens@xxxxxxxxxx>
> >
> > A heap buffer overflow can occur in the mcp2221_raw_event() function
> > when handling I2C read responses. The driver failed to check if the
> > total incoming data length fits within the originally allocated buffer
> > `mcp->rxbuf`.
> >
> > Fix this by introducing `rxbuf_len` to `struct mcp2221` to keep track
> > of the allocated buffer size. Initialize it in `mcp_i2c_smbus_read()`
> > and `mcp_smbus_xfer()`, and ensure the copied data length combined with
> > the current index does not exceed this length in `mcp2221_raw_event()`.
> >
> > Signed-off-by: Benoît Sevens <bsevens@xxxxxxxxxx>
>
> Unfortunately the patch seems to have been somehow mangled by your mail
> client:
>
>         patching file drivers/hid/hid-mcp2221.c
>         Hunk #1 succeeded at 126 (offset 7 lines).
>         Hunk #2 FAILED at 324.
>         patch: **** malformed patch at line 173: u16 addr,
>
> Fix for the same issue has later been submitted by Florian Pradines [1],
> so I will be taking that one and adding you as Reported-by: or so, ok?
>
> Thanks.
>
> [1] https://lore.kernel.org/all/20260509094517.2691246-1-florian.pradines@xxxxxxxxx/
>
> --
> Jiri Kosina
> SUSE Labs
>

Hi Jiri,

That sounds good to me. Thanks for adding me in the Reported-by

One thing though: Florian's patch doesn't set `mcp->rxbuf_size` in all
places. I believe it should be set in every place where `mcp->rxbuf`
is set. There are 2 places in `mcp_smbus_xfer` where it is missing
(see my original patch):

```c
@@ -538,6 +541,7 @@ static int mcp_smbus_xfer(struct i2c_adapter
*adapter, u16 addr,

                        mcp->rxbuf_idx = 0;
                        mcp->rxbuf = data->block;
+                       mcp->rxbuf_size = sizeof(data->block);
                        mcp->txbuf[0] = MCP2221_I2C_GET_DATA;
                        ret = mcp_send_data_req_status(mcp, mcp->txbuf, 1);
                        if (ret)
@@ -561,6 +565,7 @@ static int mcp_smbus_xfer(struct i2c_adapter
*adapter, u16 addr,

                        mcp->rxbuf_idx = 0;
                        mcp->rxbuf = data->block;
+                       mcp->rxbuf_size = sizeof(data->block);
                        mcp->txbuf[0] = MCP2221_I2C_GET_DATA;
                        ret = mcp_send_data_req_status(mcp, mcp->txbuf, 1);
                        if (ret)
```

Thanks,

Benoit