From: Linkui Xiao <[email protected]>

ixgbe_aci_send_cmd() saves only the first byte of the caller's buffer
before an EBUSY retry, using kmalloc() plus a single-byte store. When
retries are exhausted, the caller's output buffer is filled with
uninitialized slab bytes, because only the first byte was saved before
the retry. The firmware itself does not see this data, since the
retryable opcodes do not set LIBIE_AQ_FLAG_RD, but the caller still
receives garbage in its output buffer.

Replace the kmalloc() plus single-byte store with kmemdup() so the whole
indirect command buffer is saved before an EBUSY retry, skip the backup
when buf_size is 0, and key the restore off buf_cpy instead of buf,
exactly like ice_sq_send_cmd_retry() already does.

Fixes: c9e563cae19e ("ixgbe: add support for devlink reload")
Reviewed-by: Przemek Kitszel <[email protected]>
Signed-off-by: Linkui Xiao <[email protected]>
---
V1: https://lore.kernel.org/all/[email protected]/

V2:
- reword the impact description: the firmware does not see the
  uninitialized slab bytes, only the caller's output buffer does, since
  the retryable opcodes do not set LIBIE_AQ_FLAG_RD (Sashiko)
- fix the ice function reference: the kmemdup()-based backup lives in
  ice_sq_send_cmd_retry(), not ice_sq_send_cmd() (Sashiko)

 drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c 
b/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c
index 4d8ae5b56145..5dd88ee7ea58 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c
@@ -214,11 +214,10 @@ int ixgbe_aci_send_cmd(struct ixgbe_hw *hw, struct 
libie_aq_desc *desc,
 
        is_cmd_for_retry = ixgbe_should_retry_aci_send_cmd_execute(opcode);
        if (is_cmd_for_retry) {
-               if (buf) {
-                       buf_cpy = kmalloc(buf_size, GFP_KERNEL);
+               if (buf && buf_size) {
+                       buf_cpy = kmemdup(buf, buf_size, GFP_KERNEL);
                        if (!buf_cpy)
                                return -ENOMEM;
-                       *buf_cpy = *(u8 *)buf;
                }
                desc_cpy = *desc;
        }
@@ -234,7 +233,7 @@ int ixgbe_aci_send_cmd(struct ixgbe_hw *hw, struct 
libie_aq_desc *desc,
                    last_status != LIBIE_AQ_RC_EBUSY)
                        break;
 
-               if (buf)
+               if (buf_cpy)
                        memcpy(buf, buf_cpy, buf_size);
                *desc = desc_cpy;
 
-- 
2.25.1

Reply via email to