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@xxxxxxxx
From: Andrew MortonDate: 2022-04-13 05:22To: lipeifengSubject: Re: [PATCH] mm: fix align-error when get_addr in unmapped_area_topdownOn Tue, 12 Apr 2022 16:10:14 +0800 lipeifeng@xxxxxxxx wrote:> From: lipeifeng <lipeifeng@xxxxxxxx>>> 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 totrigger 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
