1. Patchew
  2. QEMU
  3. View series

[PATCH v1] hw/i3c/dw-i3c: Fix uninitialized data use in short transfer

Jamin Lin posted 1 patch 3 weeks, 6 days ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260311021319.1053774-1-jamin._5Flin@aspeedtech.com
Maintainers: Joe Komlodi <komlodi@google.com>, "Cédric Le Goater" <clg@kaod.org>, Jamin Lin <jamin_lin@aspeedtech.com>, Nabih Estefan <nabihestefan@google.com>
hw/i3c/dw-i3c.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
[PATCH v1] hw/i3c/dw-i3c: Fix uninitialized data use in short transfer
Posted by Jamin Lin 3 weeks, 6 days ago 
Coverity reports that dw_i3c_short_transfer() may pass an
uninitialized buffer to dw_i3c_send().

The immediate cause is the use of `data[len] += arg.byte0`, which
reads from an uninitialized element of the buffer. Replace this with
a simple assignment.

Additionally, avoid calling dw_i3c_send() when the constructed payload
length is zero. In that case the transfer has no data phase, so the
controller can transition to the idle state directly.

This resolves the Coverity UNINIT warning and clarifies the handling
of zero-length short transfers.

Resolves: Coverity CID 1645555
Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
 hw/i3c/dw-i3c.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/hw/i3c/dw-i3c.c b/hw/i3c/dw-i3c.c
index 3d8b95a14c..aa6c27c8de 100644
--- a/hw/i3c/dw-i3c.c
+++ b/hw/i3c/dw-i3c.c
@@ -1211,7 +1211,7 @@ static void dw_i3c_short_transfer(DWI3C *s, DWI3CTransferCmd cmd,
          * ignored.
          */
         if (cmd.dbp) {
-            data[len] += arg.byte0;
+            data[len] = arg.byte0;
             len++;
         }
     }
@@ -1226,10 +1226,16 @@ static void dw_i3c_short_transfer(DWI3C *s, DWI3CTransferCmd cmd,
         len++;
     }
 
-    if (dw_i3c_send(s, data, len, &bytes_sent, is_i2c)) {
-        err = DW_I3C_RESP_QUEUE_ERR_I2C_NACK;
+    if (len > 0) {
+        if (dw_i3c_send(s, data, len, &bytes_sent, is_i2c)) {
+            err = DW_I3C_RESP_QUEUE_ERR_I2C_NACK;
+        } else {
+            /* Only go to an idle state on a successful transfer. */
+            ARRAY_FIELD_DP32(s->regs, PRESENT_STATE, CM_TFR_ST_STATUS,
+                             DW_I3C_TRANSFER_STATE_IDLE);
+        }
     } else {
-        /* Only go to an idle state on a successful transfer. */
+        /* No payload bytes for this short transfer. */
         ARRAY_FIELD_DP32(s->regs, PRESENT_STATE, CM_TFR_ST_STATUS,
                          DW_I3C_TRANSFER_STATE_IDLE);
     }
-- 
2.43.0
Re: [PATCH v1] hw/i3c/dw-i3c: Fix uninitialized data use in short transfer
Posted by Cédric Le Goater 3 weeks ago 
On 3/11/26 03:13, Jamin Lin wrote:
> Coverity reports that dw_i3c_short_transfer() may pass an
> uninitialized buffer to dw_i3c_send().
> 
> The immediate cause is the use of `data[len] += arg.byte0`, which
> reads from an uninitialized element of the buffer. Replace this with
> a simple assignment.
> 
> Additionally, avoid calling dw_i3c_send() when the constructed payload
> length is zero. In that case the transfer has no data phase, so the
> controller can transition to the idle state directly.
> 
> This resolves the Coverity UNINIT warning and clarifies the handling
> of zero-length short transfers.
> 
> Resolves: Coverity CID 1645555
> Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> ---
>   hw/i3c/dw-i3c.c | 14 ++++++++++----
>   1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/i3c/dw-i3c.c b/hw/i3c/dw-i3c.c
> index 3d8b95a14c..aa6c27c8de 100644
> --- a/hw/i3c/dw-i3c.c
> +++ b/hw/i3c/dw-i3c.c
> @@ -1211,7 +1211,7 @@ static void dw_i3c_short_transfer(DWI3C *s, DWI3CTransferCmd cmd,
>            * ignored.
>            */
>           if (cmd.dbp) {
> -            data[len] += arg.byte0;
> +            data[len] = arg.byte0;
>               len++;
>           }
>       }
> @@ -1226,10 +1226,16 @@ static void dw_i3c_short_transfer(DWI3C *s, DWI3CTransferCmd cmd,
>           len++;
>       }
>   
> -    if (dw_i3c_send(s, data, len, &bytes_sent, is_i2c)) {
> -        err = DW_I3C_RESP_QUEUE_ERR_I2C_NACK;
> +    if (len > 0) {
> +        if (dw_i3c_send(s, data, len, &bytes_sent, is_i2c)) {
> +            err = DW_I3C_RESP_QUEUE_ERR_I2C_NACK;
> +        } else {
> +            /* Only go to an idle state on a successful transfer. */
> +            ARRAY_FIELD_DP32(s->regs, PRESENT_STATE, CM_TFR_ST_STATUS,
> +                             DW_I3C_TRANSFER_STATE_IDLE);
> +        }
>       } else {
> -        /* Only go to an idle state on a successful transfer. */
> +        /* No payload bytes for this short transfer. */
>           ARRAY_FIELD_DP32(s->regs, PRESENT_STATE, CM_TFR_ST_STATUS,
>                            DW_I3C_TRANSFER_STATE_IDLE);
>       }

Applied to aspeed-next.

Thanks,

C.
Re: [PATCH v1] hw/i3c/dw-i3c: Fix uninitialized data use in short transfer
Posted by Nabih Estefan 3 weeks, 6 days ago 
Nabih Estefan (he/him) |  Software Engineer |   nabihestefan@google.com



On Tue, Mar 10, 2026 at 7:13 PM Jamin Lin <jamin_lin@aspeedtech.com> wrote:
>
> Coverity reports that dw_i3c_short_transfer() may pass an
> uninitialized buffer to dw_i3c_send().
>
> The immediate cause is the use of `data[len] += arg.byte0`, which
> reads from an uninitialized element of the buffer. Replace this with
> a simple assignment.
>
> Additionally, avoid calling dw_i3c_send() when the constructed payload
> length is zero. In that case the transfer has no data phase, so the
> controller can transition to the idle state directly.
>
> This resolves the Coverity UNINIT warning and clarifies the handling
> of zero-length short transfers.
>
> Resolves: Coverity CID 1645555
> Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>

Reviewed-by: Nabih Estefan <nabihestefan@google.com>

Thanks!
- Nabih

> ---
>  hw/i3c/dw-i3c.c | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/hw/i3c/dw-i3c.c b/hw/i3c/dw-i3c.c
> index 3d8b95a14c..aa6c27c8de 100644
> --- a/hw/i3c/dw-i3c.c
> +++ b/hw/i3c/dw-i3c.c
> @@ -1211,7 +1211,7 @@ static void dw_i3c_short_transfer(DWI3C *s, DWI3CTransferCmd cmd,
>           * ignored.
>           */
>          if (cmd.dbp) {
> -            data[len] += arg.byte0;
> +            data[len] = arg.byte0;
>              len++;
>          }
>      }
> @@ -1226,10 +1226,16 @@ static void dw_i3c_short_transfer(DWI3C *s, DWI3CTransferCmd cmd,
>          len++;
>      }
>
> -    if (dw_i3c_send(s, data, len, &bytes_sent, is_i2c)) {
> -        err = DW_I3C_RESP_QUEUE_ERR_I2C_NACK;
> +    if (len > 0) {
> +        if (dw_i3c_send(s, data, len, &bytes_sent, is_i2c)) {
> +            err = DW_I3C_RESP_QUEUE_ERR_I2C_NACK;
> +        } else {
> +            /* Only go to an idle state on a successful transfer. */
> +            ARRAY_FIELD_DP32(s->regs, PRESENT_STATE, CM_TFR_ST_STATUS,
> +                             DW_I3C_TRANSFER_STATE_IDLE);
> +        }
>      } else {
> -        /* Only go to an idle state on a successful transfer. */
> +        /* No payload bytes for this short transfer. */
>          ARRAY_FIELD_DP32(s->regs, PRESENT_STATE, CM_TFR_ST_STATUS,
>                           DW_I3C_TRANSFER_STATE_IDLE);
>      }
> --
> 2.43.0
Re: [PATCH v1] hw/i3c/dw-i3c: Fix uninitialized data use in short transfer
Posted by Cédric Le Goater 3 weeks, 6 days ago 
On 3/11/26 03:13, Jamin Lin wrote:
> Coverity reports that dw_i3c_short_transfer() may pass an
> uninitialized buffer to dw_i3c_send().
> 
> The immediate cause is the use of `data[len] += arg.byte0`, which
> reads from an uninitialized element of the buffer. Replace this with
> a simple assignment.
> 
> Additionally, avoid calling dw_i3c_send() when the constructed payload
> length is zero. In that case the transfer has no data phase, so the
> controller can transition to the idle state directly.
> 
> This resolves the Coverity UNINIT warning and clarifies the handling
> of zero-length short transfers.
> 
> Resolves: Coverity CID 1645555
> Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>

Reviewed-by: Cédric Le Goater <clg@redhat.com>

> ---
>   hw/i3c/dw-i3c.c | 14 ++++++++++----
>   1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/i3c/dw-i3c.c b/hw/i3c/dw-i3c.c
> index 3d8b95a14c..aa6c27c8de 100644
> --- a/hw/i3c/dw-i3c.c
> +++ b/hw/i3c/dw-i3c.c
> @@ -1211,7 +1211,7 @@ static void dw_i3c_short_transfer(DWI3C *s, DWI3CTransferCmd cmd,
>            * ignored.
>            */
>           if (cmd.dbp) {
> -            data[len] += arg.byte0;
> +            data[len] = arg.byte0;
>               len++;
>           }
>       }
> @@ -1226,10 +1226,16 @@ static void dw_i3c_short_transfer(DWI3C *s, DWI3CTransferCmd cmd,
>           len++;
>       }
>   
> -    if (dw_i3c_send(s, data, len, &bytes_sent, is_i2c)) {
> -        err = DW_I3C_RESP_QUEUE_ERR_I2C_NACK;
> +    if (len > 0) {
> +        if (dw_i3c_send(s, data, len, &bytes_sent, is_i2c)) {
> +            err = DW_I3C_RESP_QUEUE_ERR_I2C_NACK;
> +        } else {
> +            /* Only go to an idle state on a successful transfer. */
> +            ARRAY_FIELD_DP32(s->regs, PRESENT_STATE, CM_TFR_ST_STATUS,
> +                             DW_I3C_TRANSFER_STATE_IDLE);
> +        }
>       } else {
> -        /* Only go to an idle state on a successful transfer. */
> +        /* No payload bytes for this short transfer. */
>           ARRAY_FIELD_DP32(s->regs, PRESENT_STATE, CM_TFR_ST_STATUS,
>                            DW_I3C_TRANSFER_STATE_IDLE);
>       }

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.