public inbox for qemu-devel@nongnu.org
 help / color / mirror / Atom feed
* [PATCH v1] hw/i3c/dw-i3c: Fix uninitialized data use in short transfer
@ 2026-03-11  2:13 Jamin Lin
  2026-03-11 16:23 ` Cédric Le Goater
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Jamin Lin @ 2026-03-11  2:13 UTC (permalink / raw)
  To: Joe Komlodi, Cédric Le Goater, Nabih Estefan,
	open list:All patches CC here
  Cc: Jamin Lin, Troy Lee, Kane Chen

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


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v1] hw/i3c/dw-i3c: Fix uninitialized data use in short transfer
  2026-03-11  2:13 [PATCH v1] hw/i3c/dw-i3c: Fix uninitialized data use in short transfer Jamin Lin
@ 2026-03-11 16:23 ` Cédric Le Goater
  2026-03-11 16:24 ` Nabih Estefan
  2026-03-17  9:31 ` Cédric Le Goater
  2 siblings, 0 replies; 4+ messages in thread
From: Cédric Le Goater @ 2026-03-11 16:23 UTC (permalink / raw)
  To: Jamin Lin, Joe Komlodi, Nabih Estefan,
	open list:All patches CC here
  Cc: Troy Lee, Kane Chen

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);
>       }



^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v1] hw/i3c/dw-i3c: Fix uninitialized data use in short transfer
  2026-03-11  2:13 [PATCH v1] hw/i3c/dw-i3c: Fix uninitialized data use in short transfer Jamin Lin
  2026-03-11 16:23 ` Cédric Le Goater
@ 2026-03-11 16:24 ` Nabih Estefan
  2026-03-17  9:31 ` Cédric Le Goater
  2 siblings, 0 replies; 4+ messages in thread
From: Nabih Estefan @ 2026-03-11 16:24 UTC (permalink / raw)
  To: Jamin Lin
  Cc: Joe Komlodi, Cédric Le Goater, open list:All patches CC here,
	Troy Lee, Kane Chen

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


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v1] hw/i3c/dw-i3c: Fix uninitialized data use in short transfer
  2026-03-11  2:13 [PATCH v1] hw/i3c/dw-i3c: Fix uninitialized data use in short transfer Jamin Lin
  2026-03-11 16:23 ` Cédric Le Goater
  2026-03-11 16:24 ` Nabih Estefan
@ 2026-03-17  9:31 ` Cédric Le Goater
  2 siblings, 0 replies; 4+ messages in thread
From: Cédric Le Goater @ 2026-03-17  9:31 UTC (permalink / raw)
  To: Jamin Lin, Joe Komlodi, Nabih Estefan,
	open list:All patches CC here
  Cc: Troy Lee, Kane Chen

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.



^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-03-17  9:31 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-11  2:13 [PATCH v1] hw/i3c/dw-i3c: Fix uninitialized data use in short transfer Jamin Lin
2026-03-11 16:23 ` Cédric Le Goater
2026-03-11 16:24 ` Nabih Estefan
2026-03-17  9:31 ` Cédric Le Goater

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox