Hello Kees, Thanks for your time spent on that! > On 27 Feb 2018, at 23:52, Kees Cook <keescook@xxxxxxxxxxxx> wrote: > > I'd like more details on the threat model here; if it's just a matter > of .so loading order, I wonder if load order randomization would get a > comparable level of uncertainty without the memory fragmentation, > like: > https://android-review.googlesource.com/c/platform/bionic/+/178130/2 > If glibc, for example, could do this too, it would go a long way to > improving things. Obviously, it's not as extreme as loading stuff all > over the place, but it seems like the effect for an attack would be > similar. The search _area_ remains small, but the ordering wouldn't be > deterministic any more. > I’m afraid library order randomization wouldn’t help much, there are several cases described in chapter 2 here: http://www.openwall.com/lists/oss-security/2018/02/27/5 when it is possible to bypass ASLR. I’m agree library randomizaiton is a good improvement but after my patch I think not much valuable. On my GitHub https://github.com/blackzert/aslur I provided tests and will make them 'all in one’ chain later. > It would be worth spelling out the "not recommended" bit some more > too: this fragments the mmap space, which has some serious issues on > smaller address spaces if you get into a situation where you cannot > allocate a hole large enough between the other allocations. > I’m agree, that's the point. >> vm_unmapped_area(struct vm_unmapped_area_info *info) >> { >> + if (current->flags & PF_RANDOMIZE) >> + return unmapped_area_random(info); > > I think this will need a larger knob -- doing this by default is > likely to break stuff, I'd imagine? Bikeshedding: I'm not sure if this > should be setting "3" for /proc/sys/kernel/randomize_va_space, or a > separate one like /proc/sys/mm/randomize_mmap_allocation. I will improve it like you said. It looks like a better option. >> + // first lets find right border with unmapped_area_topdown > > Nit: kernel comments are /* */. (It's a good idea to run patches > through scripts/checkpatch.pl first.) > Sorry, I will fix it. Thanks! >> + if (!rb_parent(prev)) >> + return -ENOMEM; >> + vma = rb_entry(rb_parent(prev), >> + struct vm_area_struct, vm_rb); >> + if (prev == vma->vm_rb.rb_right) { >> + gap_start = vma->vm_prev ? >> + vm_end_gap(vma->vm_prev) : 0; >> + goto check_current_down; >> + } >> + } >> + } > > Everything from here up is identical to the existing > unmapped_area_topdown(), yes? This likely needs to be refactored > instead of copy/pasted, and adjust to handle both unmapped_area() and > unmapped_area_topdown(). > This part also keeps ‘right_vma' as a border. If it is ok, that combined version will return vma struct, I’ll do it. >> + /* Go back up the rbtree to find next candidate node */ >> + while (true) { >> + struct rb_node *prev = &vma->vm_rb; >> + >> + if (!rb_parent(prev)) >> + BUG(); // this should not happen >> + vma = rb_entry(rb_parent(prev), >> + struct vm_area_struct, vm_rb); >> + if (prev == vma->vm_rb.rb_left) { >> + gap_start = vm_end_gap(vma->vm_prev); >> + gap_end = vm_start_gap(vma); >> + if (vma == right_vma) > > mm/mmap.c: In function ‘unmapped_area_random’: > mm/mmap.c:1939:8: warning: ‘vma’ may be used uninitialized in this > function [-Wmaybe-uninitialized] > if (vma == right_vma) > ^ Thanks, fixed! >> + break; >> + goto check_current_up; >> + } >> + } >> + } > > What are the two phases here? Could this second one get collapsed into > the first? > Let me explain. 1. we use current implementation to get larger address. Remember it as ‘right_vma’. 2. we walk tree from mm->mmap what is lowest vma. 3. we check if current vma gap satisfies length and low/high constrains 4. if so, we call random() to decide if we choose it. This how we randomly choos e vma and gap 5. we walk tree from lowest vma to highest and ignore subtrees with less gap. we do it until reach ‘right_vma’ Once we found gap, we may randomly choose address inside it. >> + addr = get_random_long() % ((high - low) >> PAGE_SHIFT); >> + addr = low + (addr << PAGE_SHIFT); >> + return addr; >> > > How large are the gaps intended to be? Looking at the gaps on > something like Xorg they differ a lot. Sorry, I can’t get clue. What's the context? You tried patch or whats the case? Thanks, Ilya -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxx. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href
