Page MenuHomeFreeBSD

Don't splay on an out-of-range address
AbandonedPublic

Authored by dougm on Jan 22 2018, 7:43 PM.
Tags
None
Referenced Files
Unknown Object (File)
Dec 20 2023, 8:16 AM
Unknown Object (File)
Dec 13 2023, 3:57 AM
Unknown Object (File)
Nov 29 2023, 11:52 PM
Unknown Object (File)
Oct 12 2023, 7:28 AM
Unknown Object (File)
Jul 27 2023, 12:08 PM
Unknown Object (File)
Jun 29 2023, 3:32 AM
Unknown Object (File)
May 7 2023, 4:25 AM
Unknown Object (File)
Jan 4 2023, 11:49 PM
Subscribers
None

Details

Reviewers
alc
markj
kib
Summary

Every call to vm_map_entry_splay passes an address that is already the address of a node in the tree, or checks the map upper and lower bounds first. Except one place, in vm_map_lookup_entry. This change adds a check there, and avoids splaying in some cases.

Diff Detail

Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

dougm added reviewers: alc, markj, kib.
This revision is now accepted and ready to land.Jan 22 2018, 8:01 PM

The origin of this patch is that I am developing changes to vm_map_entry_splay that require the address to lie within the map range, and a Peter Holm test reveals that not to be the case in at least one instance:

vm_map_entry_splay(0,0,465,0,465,...) at vm_map_entry_splay+0x215/frame 0xdab0282c
vm_map_lookup_entry(dc5481c8,0,dab0295c,0,0,...) at vm_map_lookup_entry+0xaf/frame 0xdab02868
vm_map_lookup(dab02958,0,11,dab0295c,dab0294c,...) at vm_map_lookup+0x88/frame 0xdab02908
vm_fault_hold(dc5481c8,0,1,0,0,...) at vm_fault_hold+0xd1/frame 0xdab029e8
vm_fault(dc5481c8,0,1,0,0,...) at vm_fault+0x83/frame 0xdab02a10
trap_pfault(0,7fffffff,dc52644c,c1b89ea4,d83f9a00,...) at trap_pfault+0x109/frame 0xdab02a58

Alan observes that a change to vm_map_lookup_entry like the one proposed here is redundant along many code paths, and that perhaps I should be proposing a change to vm_map_lookup, or vm_fault_hold, or vm_fault, instead, along with changes to other paths to vm_map_lookup_entry that aren't bounds-checked. For that reason, this proposed change might be sitting idle for a while.

In D14019#294313, @dougm_rice.edu wrote:

The origin of this patch is that I am developing changes to vm_map_entry_splay that require the address to lie within the map range, and a Peter Holm test reveals that not to be the case in at least one instance:

Don't you need an assert in vm_map_entry_splay() then, that the address to splay is in the range ?

The map is not an argument to vm_map_entry_splay, so the range data is not available to form an assertion.

In D14019#294386, @dougm_rice.edu wrote:

The map is not an argument to vm_map_entry_splay, so the range data is not available to form an assertion.

Nothing prevents you from adding the corresponding argument, or perhaps better, from creating an inline wrapper function which takes three args, checks the address against limits and then call the splay. In the non-debug builds, where KASSERT is compiled to an empty code, it should be optimized out completely.

Returning early cannot mean leaving the *entry value uninitialized.

This revision now requires review to proceed.Jan 23 2018, 6:44 PM