From: Minwoo Im <minwoo.im.dev@gmail.com>
To: Jeuk Kim <jeuk20.kim@samsung.com>
Cc: qemu-devel@nongnu.org, Minwoo Im <minwoo.im@samsung.com>,
Minwoo Im <minwoo.im.dev@gmail.com>,
Jeuk Kim <jeuk20.kim@gmail.com>,
Peter Maydell <peter.maydell@linaro.org>
Subject: [PATCH] hw/ufs: Fix potential bugs in MMIO read|write
Date: Sun, 23 Jun 2024 11:45:55 +0900 [thread overview]
Message-ID: <20240623024555.78697-1-minwoo.im.dev@gmail.com> (raw)
This patch fixes two points reported in coverity scan report [1]. Check
the MMIO access address with (addr + size), not just with the start offset
addr to make sure that the requested memory access not to exceed the
actual register region. We also updated (uint8_t *) to (uint32_t *) to
represent we are accessing the MMIO registers by dword-sized only.
[1] https://lore.kernel.org/qemu-devel/CAFEAcA82L-WZnHMW0X+Dr40bHM-EVq2ZH4DG4pdqop4xxDP2Og@mail.gmail.com/
Cc: Jeuk Kim <jeuk20.kim@gmail.com>
Reported-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Minwoo Im <minwoo.im.dev@gmail.com>
---
hw/ufs/ufs.c | 31 ++++++++++++++++---------------
1 file changed, 16 insertions(+), 15 deletions(-)
diff --git a/hw/ufs/ufs.c b/hw/ufs/ufs.c
index 71a88d221ced..bf2ff02ac6e5 100644
--- a/hw/ufs/ufs.c
+++ b/hw/ufs/ufs.c
@@ -55,17 +55,18 @@ static inline uint64_t ufs_reg_size(UfsHc *u)
return ufs_mcq_op_reg_addr(u, 0) + sizeof(u->mcq_op_reg);
}
-static inline bool ufs_is_mcq_reg(UfsHc *u, uint64_t addr)
+static inline bool ufs_is_mcq_reg(UfsHc *u, uint64_t addr, unsigned size)
{
uint64_t mcq_reg_addr = ufs_mcq_reg_addr(u, 0);
- return addr >= mcq_reg_addr && addr < mcq_reg_addr + sizeof(u->mcq_reg);
+ return (addr >= mcq_reg_addr &&
+ addr + size <= mcq_reg_addr + sizeof(u->mcq_reg));
}
-static inline bool ufs_is_mcq_op_reg(UfsHc *u, uint64_t addr)
+static inline bool ufs_is_mcq_op_reg(UfsHc *u, uint64_t addr, unsigned size)
{
uint64_t mcq_op_reg_addr = ufs_mcq_op_reg_addr(u, 0);
return (addr >= mcq_op_reg_addr &&
- addr < mcq_op_reg_addr + sizeof(u->mcq_op_reg));
+ addr + size <= mcq_op_reg_addr + sizeof(u->mcq_op_reg));
}
static MemTxResult ufs_addr_read(UfsHc *u, hwaddr addr, void *buf, int size)
@@ -774,25 +775,25 @@ static void ufs_write_mcq_op_reg(UfsHc *u, hwaddr offset, uint32_t data,
static uint64_t ufs_mmio_read(void *opaque, hwaddr addr, unsigned size)
{
UfsHc *u = (UfsHc *)opaque;
- uint8_t *ptr;
+ uint32_t *ptr;
uint64_t value;
uint64_t offset;
- if (addr < sizeof(u->reg)) {
+ if (addr + size <= sizeof(u->reg)) {
offset = addr;
- ptr = (uint8_t *)&u->reg;
- } else if (ufs_is_mcq_reg(u, addr)) {
+ ptr = (uint32_t *)&u->reg;
+ } else if (ufs_is_mcq_reg(u, addr, size)) {
offset = addr - ufs_mcq_reg_addr(u, 0);
- ptr = (uint8_t *)&u->mcq_reg;
- } else if (ufs_is_mcq_op_reg(u, addr)) {
+ ptr = (uint32_t *)&u->mcq_reg;
+ } else if (ufs_is_mcq_op_reg(u, addr, size)) {
offset = addr - ufs_mcq_op_reg_addr(u, 0);
- ptr = (uint8_t *)&u->mcq_op_reg;
+ ptr = (uint32_t *)&u->mcq_op_reg;
} else {
trace_ufs_err_invalid_register_offset(addr);
return 0;
}
- value = *(uint32_t *)(ptr + offset);
+ value = ptr[offset >> 2];
trace_ufs_mmio_read(addr, value, size);
return value;
}
@@ -804,11 +805,11 @@ static void ufs_mmio_write(void *opaque, hwaddr addr, uint64_t data,
trace_ufs_mmio_write(addr, data, size);
- if (addr < sizeof(u->reg)) {
+ if (addr + size <= sizeof(u->reg)) {
ufs_write_reg(u, addr, data, size);
- } else if (ufs_is_mcq_reg(u, addr)) {
+ } else if (ufs_is_mcq_reg(u, addr, size)) {
ufs_write_mcq_reg(u, addr - ufs_mcq_reg_addr(u, 0), data, size);
- } else if (ufs_is_mcq_op_reg(u, addr)) {
+ } else if (ufs_is_mcq_op_reg(u, addr, size)) {
ufs_write_mcq_op_reg(u, addr - ufs_mcq_op_reg_addr(u, 0), data, size);
} else {
trace_ufs_err_invalid_register_offset(addr);
--
2.34.1
next reply other threads:[~2024-06-23 2:47 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-06-23 2:45 Minwoo Im [this message]
2024-06-24 10:14 ` [PATCH] hw/ufs: Fix potential bugs in MMIO read|write Jeuk Kim
2024-06-24 10:27 ` Peter Maydell
2024-06-24 11:54 ` Jeuk Kim
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240623024555.78697-1-minwoo.im.dev@gmail.com \
--to=minwoo.im.dev@gmail.com \
--cc=jeuk20.kim@gmail.com \
--cc=jeuk20.kim@samsung.com \
--cc=minwoo.im@samsung.com \
--cc=peter.maydell@linaro.org \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.