Skip to content

Commit e12b11f

Browse files
minwooimJeuk Kim
authored and
Jeuk Kim
committed
hw/ufs: Fix potential bugs in MMIO read|write
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> Reviewed-by: Jeuk Kim <jeuk20.kim@samsung.com> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Message-Id: <20240623024555.78697-1-minwoo.im.dev@gmail.com> Signed-off-by: Jeuk Kim <jeuk20.kim@samsung.com>
1 parent 3665dd6 commit e12b11f

File tree

1 file changed

+16
-15
lines changed

1 file changed

+16
-15
lines changed

hw/ufs/ufs.c

+16-15
Original file line numberDiff line numberDiff line change
@@ -55,17 +55,18 @@ static inline uint64_t ufs_reg_size(UfsHc *u)
5555
return ufs_mcq_op_reg_addr(u, 0) + sizeof(u->mcq_op_reg);
5656
}
5757

58-
static inline bool ufs_is_mcq_reg(UfsHc *u, uint64_t addr)
58+
static inline bool ufs_is_mcq_reg(UfsHc *u, uint64_t addr, unsigned size)
5959
{
6060
uint64_t mcq_reg_addr = ufs_mcq_reg_addr(u, 0);
61-
return addr >= mcq_reg_addr && addr < mcq_reg_addr + sizeof(u->mcq_reg);
61+
return (addr >= mcq_reg_addr &&
62+
addr + size <= mcq_reg_addr + sizeof(u->mcq_reg));
6263
}
6364

64-
static inline bool ufs_is_mcq_op_reg(UfsHc *u, uint64_t addr)
65+
static inline bool ufs_is_mcq_op_reg(UfsHc *u, uint64_t addr, unsigned size)
6566
{
6667
uint64_t mcq_op_reg_addr = ufs_mcq_op_reg_addr(u, 0);
6768
return (addr >= mcq_op_reg_addr &&
68-
addr < mcq_op_reg_addr + sizeof(u->mcq_op_reg));
69+
addr + size <= mcq_op_reg_addr + sizeof(u->mcq_op_reg));
6970
}
7071

7172
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,
774775
static uint64_t ufs_mmio_read(void *opaque, hwaddr addr, unsigned size)
775776
{
776777
UfsHc *u = (UfsHc *)opaque;
777-
uint8_t *ptr;
778+
uint32_t *ptr;
778779
uint64_t value;
779780
uint64_t offset;
780781

781-
if (addr < sizeof(u->reg)) {
782+
if (addr + size <= sizeof(u->reg)) {
782783
offset = addr;
783-
ptr = (uint8_t *)&u->reg;
784-
} else if (ufs_is_mcq_reg(u, addr)) {
784+
ptr = (uint32_t *)&u->reg;
785+
} else if (ufs_is_mcq_reg(u, addr, size)) {
785786
offset = addr - ufs_mcq_reg_addr(u, 0);
786-
ptr = (uint8_t *)&u->mcq_reg;
787-
} else if (ufs_is_mcq_op_reg(u, addr)) {
787+
ptr = (uint32_t *)&u->mcq_reg;
788+
} else if (ufs_is_mcq_op_reg(u, addr, size)) {
788789
offset = addr - ufs_mcq_op_reg_addr(u, 0);
789-
ptr = (uint8_t *)&u->mcq_op_reg;
790+
ptr = (uint32_t *)&u->mcq_op_reg;
790791
} else {
791792
trace_ufs_err_invalid_register_offset(addr);
792793
return 0;
793794
}
794795

795-
value = *(uint32_t *)(ptr + offset);
796+
value = ptr[offset >> 2];
796797
trace_ufs_mmio_read(addr, value, size);
797798
return value;
798799
}
@@ -804,11 +805,11 @@ static void ufs_mmio_write(void *opaque, hwaddr addr, uint64_t data,
804805

805806
trace_ufs_mmio_write(addr, data, size);
806807

807-
if (addr < sizeof(u->reg)) {
808+
if (addr + size <= sizeof(u->reg)) {
808809
ufs_write_reg(u, addr, data, size);
809-
} else if (ufs_is_mcq_reg(u, addr)) {
810+
} else if (ufs_is_mcq_reg(u, addr, size)) {
810811
ufs_write_mcq_reg(u, addr - ufs_mcq_reg_addr(u, 0), data, size);
811-
} else if (ufs_is_mcq_op_reg(u, addr)) {
812+
} else if (ufs_is_mcq_op_reg(u, addr, size)) {
812813
ufs_write_mcq_op_reg(u, addr - ufs_mcq_op_reg_addr(u, 0), data, size);
813814
} else {
814815
trace_ufs_err_invalid_register_offset(addr);

0 commit comments

Comments
 (0)