David Hildenbrand <david@xxxxxxxxxx> wrote:
On 21.11.24 14:44, David Hildenbrand wrote:
On 21.11.24 13:41, Jeongjun Park wrote:
vma_adjust_trans_huge() uses find_vma() to get the VMA, but find_vma() uses
the returned pointer without any verification, even though it may return NULL.
In this case, NULL pointer dereference may occur, so to prevent this,
vma_adjust_trans_huge() should be fix to verify the return value of find_vma().
Cc: <stable@xxxxxxxxxxxxxxx>
Fixes: 685405020b9f ("mm/khugepaged: stop using vma linked list")
If that's an issue, wouldn't it have predated that commit?
struct vm_area_struct *next = vma->vm_next;
unsigned long nstart = next->vm_start;
Would have also assumed that there is a next VMA that can be
dereferenced, no?
And looking into the details, we only assume that there is a next VMA if
we are explicitly told to by the caller of vma_adjust_trans_huge() using
"adjust_next".
There is only one such caller,
vma_merge_existing_range()->commit_merge() where we set adj_start ->
"adjust_next" where we seem to have a guarantee that there is a next VMA.
I also thought that it would not be a problem in general cases, but I think
that there may be a special case (for example, a race condition...?) that can
occur in certain conditions, although I have not found it yet.
In addition, most functions except this one unconditionally check the return
value of find_vma(), so I think it would be better to handle the return value
of find_vma() consistently in this function as well, rather than taking the
risk and leaving it alone just because it seems to be okay.