[Qemu-devel] 答复: [PATCH] intc: arm_gicv3: limit GICR ipriority index

[niuguoxiang]

I think only assert is not enough, because assert() depends on NDEBUG 
preprocessing, please check :



/usr/include/assert.h



37#if defined __cplusplus && __GNUC_PREREQ (2,95)

38# define __ASSERT_VOID_CAST static_cast<void>

39#else

40# define __ASSERT_VOID_CAST (void)

41#endif



48#ifdef  NDEBUG

49

50# define assert(expr)           (__ASSERT_VOID_CAST (0))



62#else /* Not NDEBUG.  */

68/* This prints an "Assertion failed" message and aborts.  */

69extern void __assert_fail (const char *__assertion, const char *__file,

70                           unsigned int __line, const char *__function)

71     __THROW __attribute__ ((__noreturn__));

 86#endif /* Not _ASSERT_H_DECLS */



88# define assert(expr)                                                   \

89  ((expr)                                                               \

90   ? __ASSERT_VOID_CAST (0)                                             \

91   : __assert_fail (__STRING(expr), __FILE__, __LINE__, __ASSERT_FUNCTION))

92



From above, for NDEBUG, it may do nothing when assert fail,

for Not NDEBUG, it will print and abort when assert fail.



So, for NDEBUG, assert will not work when offset is 0x10000d.



How do you think?



Br，

Guoxiang Niu



华为技术有限公司 Huawei Technologies Co., Ltd.







本邮件及其附件含有华为公司的保密信息，仅限于发送给上面地址中列出的个人或群组。禁

止任何其他人以任何形式使用（包括但不限于全部或部分地泄露、复制、或散发）本邮件中

的信息。如果您错收了本邮件，请您立即电话或邮件通知发件人并删除本邮件！

This e-mail and its attachments contain confidential information from HUAWEI, 
which

is intended only for the person or entity whose address is listed above. Any 
use of the

information contained herein in any way (including, but not limited to, total 
or partial

disclosure, reproduction, or dissemination) by persons other than the intended

recipient(s) is prohibited. If you receive this e-mail in error, please notify 
the sender by

phone or email immediately and delete it!



-----邮件原件-----
发件人: Peter Maydell [mailto:address@hidden
发送时间: 2017年9月5日 19:59
收件人: P J P
抄送: QEMU Developers; qemu-arm; niuguoxiang; Prasad J Pandit
主题: Re: [PATCH] intc: arm_gicv3: limit GICR ipriority index



On 5 September 2017 at 12:21, P J P <address@hidden<mailto:address@hidden>> 
wrote:

> From: Prasad J Pandit <address@hidden<mailto:address@hidden>>

>

> When reading or writing to GICR ipriority array, index 'irq'

> could go beyond its bounds; Restrict it within array limits.

>

> Reported-by: Guoxiang Niu <address@hidden<mailto:address@hidden>>

> Signed-off-by: Prasad J Pandit <address@hidden<mailto:address@hidden>>

> ---

>  hw/intc/arm_gicv3_redist.c | 4 ++--

>  1 file changed, 2 insertions(+), 2 deletions(-)

>

> diff --git a/hw/intc/arm_gicv3_redist.c b/hw/intc/arm_gicv3_redist.c

> index 77e5cfa327..7683c4cc7f 100644

> --- a/hw/intc/arm_gicv3_redist.c

> +++ b/hw/intc/arm_gicv3_redist.c

> @@ -187,7 +187,7 @@ static MemTxResult gicr_readl(GICv3CPUState *cs, hwaddr 
> offset,

>      case GICR_ICACTIVER0:

>          *data = gicr_read_bitmap_reg(cs, attrs, cs->gicr_iactiver0);

>          return MEMTX_OK;

> -    case GICR_IPRIORITYR ... GICR_IPRIORITYR + 0x1f:

> +    case GICR_IPRIORITYR ... GICR_IPRIORITYR + 0x1c:

>      {

>          int i, irq = offset - GICR_IPRIORITYR;

>          uint32_t value = 0;

> @@ -310,7 +310,7 @@ static MemTxResult gicr_writel(GICv3CPUState *cs, hwaddr 
> offset,

>      case GICR_ICACTIVER0:

>          gicr_write_clear_bitmap_reg(cs, attrs, &cs->gicr_iactiver0, value);

>          return MEMTX_OK;

> -    case GICR_IPRIORITYR ... GICR_IPRIORITYR + 0x1f:

> +    case GICR_IPRIORITYR ... GICR_IPRIORITYR + 0x1c:

>      {

>          int i, irq = offset - GICR_IPRIORITYR;



Why do you think the buffer can be overrun? These functions are the word (4 
byte) access functions, and they cannot be called with a non-4-aligned offset 
(see the asserts in

gicv3_redist_read() and gicv3_redist_write()).



thanks

-- PMM