Re: Re: [PATCH] mm: fix align-error when get_addr in unmapped_area_topdown

["lipeifeng@oppo.com" <lipeifeng@oppo.com>]

[-- Attachment #1: Type: text/plain, Size: 6081 bytes --]


and 
Hi Andrew Morton：

> What are the runtime affects of this bug?
It will give a bad addr which is not at the desired aligment to user if trigger, 
which maybe cause some device addr access exception.

My understanding was that it is really rare to trigger and i don't know
whether it will tirgger in user-scence, the reason is in "how to trigger it".

> how are you able to trigger it?
My understanding was that it is really rare to trigger,
only if the following conditions are met:

1. (info->high_limit - info->low_limit) < (info->length + info->align_mask)
2. There is a addr space in mm->mm_rb
     ----
       |
       |
       |
      gap_end = vma_start_gap(vma0_uesd)    -------------------------------- -------------------|
       |                                                                                                                                           |
       |                                                                                                                                           |
       |                                                                                                                                           |
      info->high_limit                   -------------------|                                                                   |   > info->length
       |                                                                       |                                                                   |  
       |                                                                       |                                                                   |
       |                                                                       |   < info->length +   info->align_mask     |
      info->low_limit                                                |                                                                   |
       |                                                                       |                                                                   |
     gap_start =  vma_end_gap(vma1_used)    -----|----------------------------------------------|       
       |
    
In the above case,:
2.1 we will get gep_end firstly：
> while (true) {
> /* Visit right subtree if it looks promising */
> gap_start = vma->vm_prev ? vm_end_gap(vma->vm_prev) : 0;
> if (gap_start <= high_limit && vma->vm_rb.rb_right) {
> struct vm_area_struct *right =
> rb_entry(vma->vm_rb.rb_right,
> struct vm_area_struct, vm_rb);
> if (right->rb_subtree_gap >= length) {
> vma = right;
> continue;
> }
> }
>
> check_current:
> /* Check if current node has a suitable gap */
> gap_end = vm_start_gap(vma);
> if (gap_end < low_limit)
> return -ENOMEM;
> if (gap_start <= high_limit &&
>    gap_end > gap_start && gap_end - gap_start >= length) {
> gap_end_tmp = gap_end - info->length;
> gap_end_tmp -= (gap_end_tmp - info->align_offset) & info->align_mask;
> if (gap_end_tmp >= gap_start)
> goto found;
>
> }

2.2 we clip it with the original high_limit:
but if info->high_limit - info->low_limit < info->length + info->align_mask,
we can not promise get a addr at the desired alignment, it will cause to return a addr in align error.
   
> found:
> /* We found a suitable gap. Clip it with the original high_limit. */
> if (gap_end > info->high_limit)
> gap_end = info->high_limit;
>
> found_highest:
> /* Compute highest gap address at the desired alignment */
> gap_end -= info->length;
> gap_end -= (gap_end - info->align_offset) & info->align_mask;
> VM_BUG_ON(gap_end < info->low_limit);
> VM_BUG_ON(gap_end < gap_start);
> return gap_end;


The patch must require: info->high_limit - gap_start  > info->length + info->align_mask.
So that when gap_end = info->high_limit, we can get a right addr at the desired alignment by gap_end.

Thank you very much indeed for asking such nice problems 
so that i can try my best to explain how the patch work.

pls let me know if there are any problem in the patch, thxs.



lipeifeng@oppo.com
 
From: Andrew Morton
Date: 2022-04-13 05:22
To: lipeifeng
CC: michel; hughd; linux-mm; linux-kernel; 21cnbao; zhangshiming
Subject: Re: [PATCH] mm: fix align-error when get_addr in unmapped_area_topdown
On Tue, 12 Apr 2022 16:10:14 +0800 lipeifeng@oppo.com wrote:
 
> From: lipeifeng <lipeifeng@oppo.com>
> 
> when we found a suitable gap_end(> info->high_limit), gap_end
> must be set to info->high_limit. And we will get the gap_end
> after computing highest gap address at the desired alignment.
> 
> 2096 found:
> 2097         if (gap_end > info->high_limit)
> 2098                 gap_end = info->high_limit;
> 2099
> 2100 found_highest:
> 2101         gap_end -= info->length;
> 2102         gap_end -= (gap_end - info->align_offset) & info->align_mask;
> 2103
> 2104         VM_BUG_ON(gap_end < info->low_limit);
> 2105         VM_BUG_ON(gap_end < gap_start);
> 2106         return gap_end;
> 
> so we must promise: info->high_limit - info->low_limit >=
> info->length + info->align_mask.
> Or in rare cases(info->high_limit - info->low_limit <
> info->length + info->align_mask) we will get the addr in
> align-error if found suitable gap_end(> info->high_limit).
> 
 
Thanks.
 
What are the runtime affects of this bug, and how are you able to
trigger it?
 
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -2009,7 +2009,6 @@ static unsigned long unmapped_area_topdown(struct vm_unmapped_area_info *info)
>  if (length < info->length)
>  return -ENOMEM;
>  
> - length = info->length;
>  /*
>  * Adjust search limits by the desired length.
>  * See implementation comment at top of unmapped_area().
> @@ -2021,6 +2020,8 @@ static unsigned long unmapped_area_topdown(struct vm_unmapped_area_info *info)
>  
>  if (info->low_limit > high_limit)
>  return -ENOMEM;
> +
> + length = info->length;
>  low_limit = info->low_limit + length;
>  
>  /* Check highest gap, which does not precede any rbtree node */
> -- 
> 2.7.4

[-- Attachment #2: Type: text/html, Size: 16842 bytes --]