Re: [Qemu-devel] Re: [PATCH] Make page_find() return 0 for too-large addresses

[Anthony Liguori <anthony@codemonkey.ws>]

Eduardo Habkost wrote:
> On Fri, Sep 12, 2008 at 05:14:06PM -0300, Eduardo Habkost wrote:
>   
>> On Fri, Sep 12, 2008 at 02:50:33PM -0500, Anthony Liguori wrote:
>>     
>>> Eduardo Habkost wrote:
>>>       
>>>> On some cases, such as under KVM, tb_invalidate_phys_page_range()
>>>> may be called for large addresses, when qemu is configured to more than
>>>> 4GB of RAM.
>>>>
>>>> On these cases, qemu was crashing because it was using an index too
>>>> large for l1_map[], that supports only 32-bit addresses when compiling
>>>> without CONFIG_USER_ONLY.
>>>>   
>>>>         
> <snip>
>   
>> BTW, I've just noticed page_find_alloc() has this:
>>
>> #if TARGET_LONG_BITS > 32
>>     /* Host memory outside guest VM.  For 32-bit targets we have already
>>        excluded high addresses.  */
>>     if (index > ((target_ulong)L2_SIZE * L1_SIZE))
>>         return NULL;
>> #endif
>>
>> So, we can just use a similar check on page_find().
>>     
>
> New patch, reusing the range check from page_find_alloc() on
> page_find(). Untested.
>   

Have you tested this patch yet?  I like to avoid being the first one to 
test something when it's not my code :-)

Regards,

Anthony Liguori

> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> Index: qemu/exec.c
> ===================================================================
> --- qemu/exec.c	(revisão 5200)
> +++ qemu/exec.c	(cópia de trabalho)
> @@ -279,17 +279,24 @@ static void page_init(void)
>  #endif
>  }
>  
> -static inline PageDesc *page_find_alloc(target_ulong index)
> +static inline PageDesc **page_l1_map(target_ulong index)
>  {
> -    PageDesc **lp, *p;
> -
>  #if TARGET_LONG_BITS > 32
>      /* Host memory outside guest VM.  For 32-bit targets we have already
>         excluded high addresses.  */
>      if (index > ((target_ulong)L2_SIZE * L1_SIZE))
>          return NULL;
>  #endif
> -    lp = &l1_map[index >> L2_BITS];
> +    return &l1_map[index >> L2_BITS];
> +}
> +
> +static inline PageDesc *page_find_alloc(target_ulong index)
> +{
> +    PageDesc **lp, *p;
> +    lp = page_l1_map(index);
> +    if (!lp)
> +        return NULL;
> +
>      p = *lp;
>      if (!p) {
>          /* allocate if not found */
> @@ -316,9 +323,12 @@ static inline PageDesc *page_find_alloc(
>  
>  static inline PageDesc *page_find(target_ulong index)
>  {
> -    PageDesc *p;
> +    PageDesc **lp, *p;
> +    lp = page_l1_map(index);
> +    if (!lp)
> +        return NULL;
>  
> -    p = l1_map[index >> L2_BITS];
> +    p = *lp;
>      if (!p)
>          return 0;
>      return p + (index & (L2_SIZE - 1));
>
>
>