[PULL 08/18] hw/sd/sdcard: Avoid confusing address calculation in rpmb_calc_hmac

Philippe Mathieu-Daudé posted 18 patches 1 day, 17 hours ago
Maintainers: Jason Wang <jasowang@redhat.com>, Andrew Melnychenko <andrew@daynix.com>, Yuri Benditovich <yuri.benditovich@daynix.com>, Peter Maydell <peter.maydell@linaro.org>, Alistair Francis <alistair@alistair23.me>, "Edgar E. Iglesias" <edgar.iglesias@gmail.com>, "Michael S. Tsirkin" <mst@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Bin Meng <bmeng.cn@gmail.com>, Richard Henderson <richard.henderson@linaro.org>, Helge Deller <deller@gmx.de>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>, Li Zhijian <lizhijian@fujitsu.com>, Peter Xu <peterx@redhat.com>, Michael Roth <michael.roth@amd.com>, Kostiantyn Kostiuk <kkostiuk@redhat.com>, Fabiano Rosas <farosas@suse.de>, Laurent Vivier <lvivier@redhat.com>
[PULL 08/18] hw/sd/sdcard: Avoid confusing address calculation in rpmb_calc_hmac
Posted by Philippe Mathieu-Daudé 1 day, 17 hours ago
From: Jan Kiszka <jan.kiszka@siemens.com>

From the source frame, we initially need to copy out all fields after
data, thus starting from nonce on. Avoid expressing this indirectly by
pointing to the end of the data field - which also raised the attention
of Coverity (out-of-bound read /wrt data).

Resolves: CID 1642869
Reported-by: GuoHan Zhao <zhaoguohan@kylinos.cn>
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-ID: <4f7e1952-ecbd-4484-b128-9d02de3a7935@siemens.com>
[PMD: Add comment before the memcpy() call]
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/sd/sd.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index df5a36fad9d..40a75a43ffb 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1160,8 +1160,13 @@ static bool rpmb_calc_hmac(SDState *sd, const RPMBDataFrame *frame,
 
         assert(RPMB_HASH_LEN <= sizeof(sd->data));
 
-        memcpy((uint8_t *)buf + RPMB_DATA_LEN, &frame->data[RPMB_DATA_LEN],
+        /*
+         * We will hash everything from data field to the end of RPMBDataFrame.
+         */
+        memcpy((uint8_t *)buf + RPMB_DATA_LEN,
+               (uint8_t *)frame + offsetof(RPMBDataFrame, nonce),
                RPMB_HASH_LEN - RPMB_DATA_LEN);
+
         offset = lduw_be_p(&frame->address) * RPMB_DATA_LEN + sd_part_offset(sd);
         do {
             if (blk_pread(sd->blk, offset, RPMB_DATA_LEN, buf, 0) < 0) {
-- 
2.51.0