1. Patchew
  2. QEMU
  3. [PATCH v5 0/6] sd: Add RPMB emulation to eMMC model
  4. View patch

[PATCH v5 4/6] hw/sd/sdcard: Handle RPMB MAC field

Jan Kiszka posted 6 patches 4 weeks ago
Maintainers: "Philippe Mathieu-Daudé" <philmd@linaro.org>, Bin Meng <bmeng.cn@gmail.com>
There is a newer version of this series
[PATCH v5 4/6] hw/sd/sdcard: Handle RPMB MAC field
Posted by Jan Kiszka 4 weeks ago 
From: Jan Kiszka <jan.kiszka@siemens.com>

Implement correct setting of the MAC field when passing RPMB frames back
to the guest. Also check the MAC on authenticated write requests.

This depends on HMAC support for QCRYPTO_HASH_ALGO_SHA256 which is
always available via glib - assert this, just to be safe.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 hw/sd/sd.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 81 insertions(+)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 918fe9f79f..759e284ac0 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -51,6 +51,7 @@
 #include "qemu/module.h"
 #include "sdmmc-internal.h"
 #include "trace.h"
+#include "crypto/hmac.h"
 
 //#define DEBUG_SD 1
 
@@ -118,6 +119,7 @@ typedef struct SDProto {
 } SDProto;
 
 #define RPMB_KEY_MAC_LEN    32
+#define RPMB_HASH_LEN       284
 
 typedef struct {
     uint8_t stuff_bytes[196];
@@ -1120,6 +1122,66 @@ static void sd_blk_write(SDState *sd, uint64_t addr, uint32_t len)
     }
 }
 
+static bool rpmb_calc_hmac(SDState *sd, RPMBDataFrame *frame,
+                           unsigned int num_blocks, uint8_t *mac)
+{
+    size_t mac_len = RPMB_KEY_MAC_LEN;
+    bool success = true;
+    Error *err = NULL;
+    QCryptoHmac *hmac;
+    uint64_t addr;
+
+    hmac = qcrypto_hmac_new(QCRYPTO_HASH_ALGO_SHA256, sd->rpmb_key,
+                            RPMB_KEY_MAC_LEN, &err);
+    if (!hmac) {
+        error_report_err(err);
+        return false;
+    }
+
+    /*
+     * This implies a read request because we only support single-block write
+     * requests so far.
+     */
+    if (num_blocks > 1) {
+        /*
+         * Unfortunately, the underlying crypto libraries do not allow us to
+         * migrate an active QCryptoHmac state. Therefore, we have to calculate
+         * the HMAC in one run. To avoid buffering a complete read sequence in
+         * SDState, reconstruct all frames except for the last one.
+         */
+        char *buf = (char *)sd->data;
+
+        memcpy(buf, frame->data, RPMB_HASH_LEN);
+        addr = be16_to_cpu(frame->address) * 256 + sd_part_offset(sd);
+        do {
+            if (blk_pread(sd->blk, addr, 256, buf, 0) < 0) {
+                fprintf(stderr, "sd_blk_read: read error on host side\n");
+                success = false;
+                break;
+            }
+            if (qcrypto_hmac_bytes(hmac, buf, RPMB_HASH_LEN, NULL, NULL,
+                                   &err) < 0) {
+                error_report_err(err);
+                success = false;
+                break;
+            }
+            addr += 256;
+        } while (--num_blocks > 1);
+    }
+
+    if (success &&
+        qcrypto_hmac_bytes(hmac, (const char*)frame->data, RPMB_HASH_LEN, &mac,
+                           &mac_len, &err) < 0) {
+        error_report_err(err);
+        success = false;
+    }
+    assert(!success || mac_len == RPMB_KEY_MAC_LEN);
+
+    qcrypto_hmac_free(hmac);
+
+    return success;
+}
+
 static void emmc_rpmb_blk_read(SDState *sd, uint64_t addr, uint32_t len)
 {
     uint16_t resp = be16_to_cpu(sd->rpmb_result.req_resp);
@@ -1142,6 +1204,17 @@ static void emmc_rpmb_blk_read(SDState *sd, uint64_t addr, uint32_t len)
             sd->rpmb_result.result = cpu_to_be16(RPMB_RESULT_READ_FAILURE |
                 (result & RPMB_RESULT_COUTER_EXPIRED));
         }
+        if (sd->multi_blk_cnt == 1 &&
+            !rpmb_calc_hmac(sd, &sd->rpmb_result,
+                            be16_to_cpu(sd->rpmb_result.block_count),
+                            sd->rpmb_result.key_mac)) {
+            memset(sd->rpmb_result.data, 0, sizeof(sd->rpmb_result.data));
+            sd->rpmb_result.result = cpu_to_be16(RPMB_RESULT_AUTH_FAILURE);
+        }
+    } else if (!rpmb_calc_hmac(sd, &sd->rpmb_result, 1,
+                               sd->rpmb_result.key_mac)) {
+        memset(sd->rpmb_result.data, 0, sizeof(sd->rpmb_result.data));
+        sd->rpmb_result.result = cpu_to_be16(RPMB_RESULT_AUTH_FAILURE);
     }
     memcpy(sd->data, &sd->rpmb_result, sizeof(sd->rpmb_result));
 
@@ -1153,6 +1226,7 @@ static void emmc_rpmb_blk_write(SDState *sd, uint64_t addr, uint32_t len)
 {
     RPMBDataFrame *frame = (RPMBDataFrame *)sd->data;
     uint16_t req = be16_to_cpu(frame->req_resp);
+    uint8_t mac[RPMB_KEY_MAC_LEN];
 
     if (req == RPMB_REQ_READ_RESULT) {
         /* just return the current result register */
@@ -1190,6 +1264,11 @@ static void emmc_rpmb_blk_write(SDState *sd, uint64_t addr, uint32_t len)
             sd->rpmb_result.result = cpu_to_be16(RPMB_RESULT_WRITE_FAILURE);
             break;
         }
+        if (!rpmb_calc_hmac(sd, frame, 1, mac) ||
+            memcmp(frame->key_mac, mac, RPMB_KEY_MAC_LEN) != 0) {
+            sd->rpmb_result.result = cpu_to_be16(RPMB_RESULT_AUTH_FAILURE);
+            break;
+        }
         if (be32_to_cpu(frame->write_counter) != sd->rpmb_write_counter) {
             sd->rpmb_result.result = cpu_to_be16(RPMB_RESULT_COUNTER_FAILURE);
             break;
@@ -3115,6 +3194,8 @@ static void emmc_class_init(ObjectClass *klass, const void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
     SDCardClass *sc = SDMMC_COMMON_CLASS(klass);
 
+    assert(qcrypto_hmac_supports(QCRYPTO_HASH_ALGO_SHA256));
+
     dc->desc = "eMMC";
     dc->realize = emmc_realize;
     device_class_set_props(dc, emmc_properties);
-- 
2.51.0
Re: [PATCH v5 4/6] hw/sd/sdcard: Handle RPMB MAC field
Posted by Philippe Mathieu-Daudé 1 week, 4 days ago 
Hi Jan,

On 17/10/25 14:03, Jan Kiszka wrote:
> From: Jan Kiszka <jan.kiszka@siemens.com>
> 
> Implement correct setting of the MAC field when passing RPMB frames back
> to the guest. Also check the MAC on authenticated write requests.
> 
> This depends on HMAC support for QCRYPTO_HASH_ALGO_SHA256 which is
> always available via glib - assert this, just to be safe.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>   hw/sd/sd.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 81 insertions(+)
> 
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index 918fe9f79f..759e284ac0 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -51,6 +51,7 @@
>   #include "qemu/module.h"
>   #include "sdmmc-internal.h"
>   #include "trace.h"
> +#include "crypto/hmac.h"
>   
>   //#define DEBUG_SD 1
>   
> @@ -118,6 +119,7 @@ typedef struct SDProto {
>   } SDProto;
>   
>   #define RPMB_KEY_MAC_LEN    32
> +#define RPMB_HASH_LEN       284
>   
>   typedef struct {
>       uint8_t stuff_bytes[196];
> @@ -1120,6 +1122,66 @@ static void sd_blk_write(SDState *sd, uint64_t addr, uint32_t len)
>       }
>   }
>   
> +static bool rpmb_calc_hmac(SDState *sd, RPMBDataFrame *frame,

const frame.

> +                           unsigned int num_blocks, uint8_t *mac)
> +{
> +    size_t mac_len = RPMB_KEY_MAC_LEN;
> +    bool success = true;
> +    Error *err = NULL;
> +    QCryptoHmac *hmac;

Preferably:

        g_autoptr(QCryptoHmac) hmac = NULL;

> +    uint64_t addr;

Maybe better named 'offset'.

> +
> +    hmac = qcrypto_hmac_new(QCRYPTO_HASH_ALGO_SHA256, sd->rpmb_key,
> +                            RPMB_KEY_MAC_LEN, &err);
> +    if (!hmac) {
> +        error_report_err(err);
> +        return false;
> +    }
> +
> +    /*
> +     * This implies a read request because we only support single-block write
> +     * requests so far.
> +     */
> +    if (num_blocks > 1) {
> +        /*
> +         * Unfortunately, the underlying crypto libraries do not allow us to
> +         * migrate an active QCryptoHmac state. Therefore, we have to calculate
> +         * the HMAC in one run. To avoid buffering a complete read sequence in
> +         * SDState, reconstruct all frames except for the last one.
> +         */
> +        char *buf = (char *)sd->data;
Why not plain 'void *'? Ah, qcrypto_hmac_bytes() takes a 'char *', 
odd... [Update, now about to be fixed:
https://lore.kernel.org/qemu-devel/20251103133727.423041-4-berrange@redhat.com/]

Better safe than sorry:

            assert(RPMB_HASH_LEN <= sizeof(sd->data));

> +        memcpy(buf, frame->data, RPMB_HASH_LEN);

Nitpicking, no need to fill the block we'll overwrite:

            memcpy(&buf[256], &frame->data[256], RPMB_HASH_LEN - 256);

> +        addr = be16_to_cpu(frame->address) * 256 + sd_part_offset(sd);
Personally I prefer the ld/st API, and I'd rather see it used
consistently within hw/sd/. So (matter of taste):

            block_index = ldw_be_p(&frame->address);
            block_offset = block_index * 256 + sd_part_offset(sd);

> +        do {
> +            if (blk_pread(sd->blk, addr, 256, buf, 0) < 0) {
> +                fprintf(stderr, "sd_blk_read: read error on host side\n");

Although a pre-existing pattern in this file, no new fprintf(stderr)
please. Better use the Error* API, otherwise error_report().

> +                success = false;
> +                break;
> +            }
> +            if (qcrypto_hmac_bytes(hmac, buf, RPMB_HASH_LEN, NULL, NULL,> +                                   &err) < 0) {
> +                error_report_err(err);
> +                success = false;
> +                break;
> +            }
> +            addr += 256;
> +        } while (--num_blocks > 1);
> +    }
> +
> +    if (success &&
> +        qcrypto_hmac_bytes(hmac, (const char*)frame->data, RPMB_HASH_LEN, &mac,
> +                           &mac_len, &err) < 0) {
> +        error_report_err(err);

Ideally Error* should to be propagated to upper layers.

Here SDCardClass::read_byte() and SDCardClass::write_byte()
don't expect failure, so OK -- maybe they should ... --.

> +        success = false;
> +    }
> +    assert(!success || mac_len == RPMB_KEY_MAC_LEN);
> +
> +    qcrypto_hmac_free(hmac);
> +
> +    return success;
> +}
> +
>   static void emmc_rpmb_blk_read(SDState *sd, uint64_t addr, uint32_t len)
>   {
>       uint16_t resp = be16_to_cpu(sd->rpmb_result.req_resp);
> @@ -1142,6 +1204,17 @@ static void emmc_rpmb_blk_read(SDState *sd, uint64_t addr, uint32_t len)
>               sd->rpmb_result.result = cpu_to_be16(RPMB_RESULT_READ_FAILURE |
>                   (result & RPMB_RESULT_COUTER_EXPIRED));
>           }
> +        if (sd->multi_blk_cnt == 1 &&
> +            !rpmb_calc_hmac(sd, &sd->rpmb_result,
> +                            be16_to_cpu(sd->rpmb_result.block_count),
> +                            sd->rpmb_result.key_mac)) {
> +            memset(sd->rpmb_result.data, 0, sizeof(sd->rpmb_result.data));
> +            sd->rpmb_result.result = cpu_to_be16(RPMB_RESULT_AUTH_FAILURE);
> +        }
> +    } else if (!rpmb_calc_hmac(sd, &sd->rpmb_result, 1,
> +                               sd->rpmb_result.key_mac)) {
> +        memset(sd->rpmb_result.data, 0, sizeof(sd->rpmb_result.data));
> +        sd->rpmb_result.result = cpu_to_be16(RPMB_RESULT_AUTH_FAILURE);
>       }
>       memcpy(sd->data, &sd->rpmb_result, sizeof(sd->rpmb_result));
>   
> @@ -1153,6 +1226,7 @@ static void emmc_rpmb_blk_write(SDState *sd, uint64_t addr, uint32_t len)
>   {
>       RPMBDataFrame *frame = (RPMBDataFrame *)sd->data;
>       uint16_t req = be16_to_cpu(frame->req_resp);
> +    uint8_t mac[RPMB_KEY_MAC_LEN];
>   
>       if (req == RPMB_REQ_READ_RESULT) {
>           /* just return the current result register */
> @@ -1190,6 +1264,11 @@ static void emmc_rpmb_blk_write(SDState *sd, uint64_t addr, uint32_t len)
>               sd->rpmb_result.result = cpu_to_be16(RPMB_RESULT_WRITE_FAILURE);
>               break;
>           }
> +        if (!rpmb_calc_hmac(sd, frame, 1, mac) ||
> +            memcmp(frame->key_mac, mac, RPMB_KEY_MAC_LEN) != 0) {
> +            sd->rpmb_result.result = cpu_to_be16(RPMB_RESULT_AUTH_FAILURE);
> +            break;
> +        }
>           if (be32_to_cpu(frame->write_counter) != sd->rpmb_write_counter) {
>               sd->rpmb_result.result = cpu_to_be16(RPMB_RESULT_COUNTER_FAILURE);
>               break;
> @@ -3115,6 +3194,8 @@ static void emmc_class_init(ObjectClass *klass, const void *data)
>       DeviceClass *dc = DEVICE_CLASS(klass);
>       SDCardClass *sc = SDMMC_COMMON_CLASS(klass);
>   
> +    assert(qcrypto_hmac_supports(QCRYPTO_HASH_ALGO_SHA256));
> +
>       dc->desc = "eMMC";
>       dc->realize = emmc_realize;
>       device_class_set_props(dc, emmc_properties);
I'd like to squash on this patch:
-- >8 --
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index f2260796db8..6e4eeeda157 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1129,3 +1129,3 @@ static void sd_blk_write(SDState *sd, uint64_t 
addr, uint32_t len)

-static bool rpmb_calc_hmac(SDState *sd, RPMBDataFrame *frame,
+static bool rpmb_calc_hmac(SDState *sd, const RPMBDataFrame *frame,
                             unsigned int num_blocks, uint8_t *mac)
@@ -1133,5 +1133,5 @@ static bool rpmb_calc_hmac(SDState *sd, 
RPMBDataFrame *frame,
      size_t mac_len = RPMB_KEY_MAC_LEN;
+    g_autoptr(QCryptoHmac) hmac = NULL;
      bool success = true;
      Error *err = NULL;
-    QCryptoHmac *hmac;
      uint64_t addr;
@@ -1156,9 +1156,12 @@ static bool rpmb_calc_hmac(SDState *sd, 
RPMBDataFrame *frame,
           */
-        char *buf = (char *)sd->data;
+        void *buf = sd->data;

-        memcpy(buf, frame->data, RPMB_HASH_LEN);
-        addr = be16_to_cpu(frame->address) * 256 + sd_part_offset(sd);
+        assert(RPMB_HASH_LEN <= sizeof(sd->data));
+        memcpy(&buf[RPMB_DATA_LEN], &frame->data[RPMB_DATA_LEN],
+               RPMB_HASH_LEN - RPMB_DATA_LEN);
+
+        addr = be16_to_cpu(frame->address) * RPMB_DATA_LEN + 
sd_part_offset(sd);
          do {
-            if (blk_pread(sd->blk, addr, 256, buf, 0) < 0) {
-                fprintf(stderr, "sd_blk_read: read error on host side\n");
+            if (blk_pread(sd->blk, addr, RPMB_DATA_LEN, buf, 0) < 0) {
+                error_report("sd_blk_read: read error on host side");
                  success = false;
@@ -1172,3 +1175,3 @@ static bool rpmb_calc_hmac(SDState *sd, 
RPMBDataFrame *frame,
              }
-            addr += 256;
+            addr += RPMB_DATA_LEN;
          } while (--num_blocks > 1);
@@ -1177,3 +1180,3 @@ static bool rpmb_calc_hmac(SDState *sd, 
RPMBDataFrame *frame,
      if (success &&
-        qcrypto_hmac_bytes(hmac, (const char*)frame->data, 
RPMB_HASH_LEN, &mac,
+        qcrypto_hmac_bytes(hmac, frame->data, RPMB_HASH_LEN, &mac,
                             &mac_len, &err) < 0) {
@@ -1184,4 +1187,2 @@ static bool rpmb_calc_hmac(SDState *sd, 
RPMBDataFrame *frame,

-    qcrypto_hmac_free(hmac);
-
      return success;
---

Regards,

Phil.
Re: [PATCH v5 4/6] hw/sd/sdcard: Handle RPMB MAC field
Posted by Jan Kiszka 1 week, 3 days ago 
On 03.11.25 16:18, Philippe Mathieu-Daudé wrote:
> Hi Jan,
> 
> On 17/10/25 14:03, Jan Kiszka wrote:
>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>
>> Implement correct setting of the MAC field when passing RPMB frames back
>> to the guest. Also check the MAC on authenticated write requests.
>>
>> This depends on HMAC support for QCRYPTO_HASH_ALGO_SHA256 which is
>> always available via glib - assert this, just to be safe.
>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>> ---
>>   hw/sd/sd.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 81 insertions(+)
>>
>> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
>> index 918fe9f79f..759e284ac0 100644
>> --- a/hw/sd/sd.c
>> +++ b/hw/sd/sd.c
>> @@ -51,6 +51,7 @@
>>   #include "qemu/module.h"
>>   #include "sdmmc-internal.h"
>>   #include "trace.h"
>> +#include "crypto/hmac.h"
>>     //#define DEBUG_SD 1
>>   @@ -118,6 +119,7 @@ typedef struct SDProto {
>>   } SDProto;
>>     #define RPMB_KEY_MAC_LEN    32
>> +#define RPMB_HASH_LEN       284
>>     typedef struct {
>>       uint8_t stuff_bytes[196];
>> @@ -1120,6 +1122,66 @@ static void sd_blk_write(SDState *sd, uint64_t
>> addr, uint32_t len)
>>       }
>>   }
>>   +static bool rpmb_calc_hmac(SDState *sd, RPMBDataFrame *frame,
> 
> const frame.
> 
>> +                           unsigned int num_blocks, uint8_t *mac)
>> +{
>> +    size_t mac_len = RPMB_KEY_MAC_LEN;
>> +    bool success = true;
>> +    Error *err = NULL;
>> +    QCryptoHmac *hmac;
> 
> Preferably:
> 
>        g_autoptr(QCryptoHmac) hmac = NULL;
> 
>> +    uint64_t addr;
> 
> Maybe better named 'offset'.
> 
>> +
>> +    hmac = qcrypto_hmac_new(QCRYPTO_HASH_ALGO_SHA256, sd->rpmb_key,
>> +                            RPMB_KEY_MAC_LEN, &err);
>> +    if (!hmac) {
>> +        error_report_err(err);
>> +        return false;
>> +    }
>> +
>> +    /*
>> +     * This implies a read request because we only support single-
>> block write
>> +     * requests so far.
>> +     */
>> +    if (num_blocks > 1) {
>> +        /*
>> +         * Unfortunately, the underlying crypto libraries do not
>> allow us to
>> +         * migrate an active QCryptoHmac state. Therefore, we have to
>> calculate
>> +         * the HMAC in one run. To avoid buffering a complete read
>> sequence in
>> +         * SDState, reconstruct all frames except for the last one.
>> +         */
>> +        char *buf = (char *)sd->data;
> Why not plain 'void *'? Ah, qcrypto_hmac_bytes() takes a 'char *',
> odd... [Update, now about to be fixed:
> https://lore.kernel.org/qemu-devel/20251103133727.423041-4-
> berrange@redhat.com/]
> 
> Better safe than sorry:
> 
>            assert(RPMB_HASH_LEN <= sizeof(sd->data));
> 
>> +        memcpy(buf, frame->data, RPMB_HASH_LEN);
> 
> Nitpicking, no need to fill the block we'll overwrite:
> 
>            memcpy(&buf[256], &frame->data[256], RPMB_HASH_LEN - 256);
> 
>> +        addr = be16_to_cpu(frame->address) * 256 + sd_part_offset(sd);
> Personally I prefer the ld/st API, and I'd rather see it used
> consistently within hw/sd/. So (matter of taste):
> 
>            block_index = ldw_be_p(&frame->address);
>            block_offset = block_index * 256 + sd_part_offset(sd);
> 
>> +        do {
>> +            if (blk_pread(sd->blk, addr, 256, buf, 0) < 0) {
>> +                fprintf(stderr, "sd_blk_read: read error on host
>> side\n");
> 
> Although a pre-existing pattern in this file, no new fprintf(stderr)
> please. Better use the Error* API, otherwise error_report().
> 
>> +                success = false;
>> +                break;
>> +            }
>> +            if (qcrypto_hmac_bytes(hmac, buf, RPMB_HASH_LEN, NULL,
>> NULL,> +                                   &err) < 0) {
>> +                error_report_err(err);
>> +                success = false;
>> +                break;
>> +            }
>> +            addr += 256;
>> +        } while (--num_blocks > 1);
>> +    }
>> +
>> +    if (success &&
>> +        qcrypto_hmac_bytes(hmac, (const char*)frame->data,
>> RPMB_HASH_LEN, &mac,
>> +                           &mac_len, &err) < 0) {
>> +        error_report_err(err);
> 
> Ideally Error* should to be propagated to upper layers.
> 
> Here SDCardClass::read_byte() and SDCardClass::write_byte()
> don't expect failure, so OK -- maybe they should ... --.
> 

So, leave it like it is for now?

>> +        success = false;
>> +    }
>> +    assert(!success || mac_len == RPMB_KEY_MAC_LEN);
>> +
>> +    qcrypto_hmac_free(hmac);
>> +
>> +    return success;
>> +}
>> +
>>   static void emmc_rpmb_blk_read(SDState *sd, uint64_t addr, uint32_t
>> len)
>>   {
>>       uint16_t resp = be16_to_cpu(sd->rpmb_result.req_resp);
>> @@ -1142,6 +1204,17 @@ static void emmc_rpmb_blk_read(SDState *sd,
>> uint64_t addr, uint32_t len)
>>               sd->rpmb_result.result =
>> cpu_to_be16(RPMB_RESULT_READ_FAILURE |
>>                   (result & RPMB_RESULT_COUTER_EXPIRED));
>>           }
>> +        if (sd->multi_blk_cnt == 1 &&
>> +            !rpmb_calc_hmac(sd, &sd->rpmb_result,
>> +                            be16_to_cpu(sd->rpmb_result.block_count),
>> +                            sd->rpmb_result.key_mac)) {
>> +            memset(sd->rpmb_result.data, 0, sizeof(sd-
>> >rpmb_result.data));
>> +            sd->rpmb_result.result =
>> cpu_to_be16(RPMB_RESULT_AUTH_FAILURE);
>> +        }
>> +    } else if (!rpmb_calc_hmac(sd, &sd->rpmb_result, 1,
>> +                               sd->rpmb_result.key_mac)) {
>> +        memset(sd->rpmb_result.data, 0, sizeof(sd->rpmb_result.data));
>> +        sd->rpmb_result.result = cpu_to_be16(RPMB_RESULT_AUTH_FAILURE);
>>       }
>>       memcpy(sd->data, &sd->rpmb_result, sizeof(sd->rpmb_result));
>>   @@ -1153,6 +1226,7 @@ static void emmc_rpmb_blk_write(SDState *sd,
>> uint64_t addr, uint32_t len)
>>   {
>>       RPMBDataFrame *frame = (RPMBDataFrame *)sd->data;
>>       uint16_t req = be16_to_cpu(frame->req_resp);
>> +    uint8_t mac[RPMB_KEY_MAC_LEN];
>>         if (req == RPMB_REQ_READ_RESULT) {
>>           /* just return the current result register */
>> @@ -1190,6 +1264,11 @@ static void emmc_rpmb_blk_write(SDState *sd,
>> uint64_t addr, uint32_t len)
>>               sd->rpmb_result.result =
>> cpu_to_be16(RPMB_RESULT_WRITE_FAILURE);
>>               break;
>>           }
>> +        if (!rpmb_calc_hmac(sd, frame, 1, mac) ||
>> +            memcmp(frame->key_mac, mac, RPMB_KEY_MAC_LEN) != 0) {
>> +            sd->rpmb_result.result =
>> cpu_to_be16(RPMB_RESULT_AUTH_FAILURE);
>> +            break;
>> +        }
>>           if (be32_to_cpu(frame->write_counter) != sd-
>> >rpmb_write_counter) {
>>               sd->rpmb_result.result =
>> cpu_to_be16(RPMB_RESULT_COUNTER_FAILURE);
>>               break;
>> @@ -3115,6 +3194,8 @@ static void emmc_class_init(ObjectClass *klass,
>> const void *data)
>>       DeviceClass *dc = DEVICE_CLASS(klass);
>>       SDCardClass *sc = SDMMC_COMMON_CLASS(klass);
>>   +    assert(qcrypto_hmac_supports(QCRYPTO_HASH_ALGO_SHA256));
>> +
>>       dc->desc = "eMMC";
>>       dc->realize = emmc_realize;
>>       device_class_set_props(dc, emmc_properties);
> I'd like to squash on this patch:
> -- >8 --
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index f2260796db8..6e4eeeda157 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -1129,3 +1129,3 @@ static void sd_blk_write(SDState *sd, uint64_t
> addr, uint32_t len)
> 
> -static bool rpmb_calc_hmac(SDState *sd, RPMBDataFrame *frame,
> +static bool rpmb_calc_hmac(SDState *sd, const RPMBDataFrame *frame,
>                             unsigned int num_blocks, uint8_t *mac)
> @@ -1133,5 +1133,5 @@ static bool rpmb_calc_hmac(SDState *sd,
> RPMBDataFrame *frame,
>      size_t mac_len = RPMB_KEY_MAC_LEN;
> +    g_autoptr(QCryptoHmac) hmac = NULL;
>      bool success = true;
>      Error *err = NULL;
> -    QCryptoHmac *hmac;
>      uint64_t addr;
> @@ -1156,9 +1156,12 @@ static bool rpmb_calc_hmac(SDState *sd,
> RPMBDataFrame *frame,
>           */
> -        char *buf = (char *)sd->data;
> +        void *buf = sd->data;
> 
> -        memcpy(buf, frame->data, RPMB_HASH_LEN);
> -        addr = be16_to_cpu(frame->address) * 256 + sd_part_offset(sd);
> +        assert(RPMB_HASH_LEN <= sizeof(sd->data));
> +        memcpy(&buf[RPMB_DATA_LEN], &frame->data[RPMB_DATA_LEN],
> +               RPMB_HASH_LEN - RPMB_DATA_LEN);
> +
> +        addr = be16_to_cpu(frame->address) * RPMB_DATA_LEN +
> sd_part_offset(sd);
>          do {
> -            if (blk_pread(sd->blk, addr, 256, buf, 0) < 0) {
> -                fprintf(stderr, "sd_blk_read: read error on host side\n");
> +            if (blk_pread(sd->blk, addr, RPMB_DATA_LEN, buf, 0) < 0) {
> +                error_report("sd_blk_read: read error on host side");
>                  success = false;
> @@ -1172,3 +1175,3 @@ static bool rpmb_calc_hmac(SDState *sd,
> RPMBDataFrame *frame,
>              }
> -            addr += 256;
> +            addr += RPMB_DATA_LEN;
>          } while (--num_blocks > 1);
> @@ -1177,3 +1180,3 @@ static bool rpmb_calc_hmac(SDState *sd,
> RPMBDataFrame *frame,
>      if (success &&
> -        qcrypto_hmac_bytes(hmac, (const char*)frame->data,
> RPMB_HASH_LEN, &mac,
> +        qcrypto_hmac_bytes(hmac, frame->data, RPMB_HASH_LEN, &mac,
>                             &mac_len, &err) < 0) {
> @@ -1184,4 +1187,2 @@ static bool rpmb_calc_hmac(SDState *sd,
> RPMBDataFrame *frame,
> 
> -    qcrypto_hmac_free(hmac);
> -
>      return success;
> ---
> 
> Regards,
> 
> Phil.

Same story: Will pick it up and test.

Jan

-- 
Siemens AG, Foundational Technologies
Linux Expert Center

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.