Skip to content

Commit c103a75

Browse files
ljskernelgregkh
authored andcommitted
mm/vma: add give_up_on_oom option on modify/merge, use in uffd release
commit 41e6ddc upstream. Currently, if a VMA merge fails due to an OOM condition arising on commit merge or a failure to duplicate anon_vma's, we report this so the caller can handle it. However there are cases where the caller is only ostensibly trying a merge, and doesn't mind if it fails due to this condition. Since we do not want to introduce an implicit assumption that we only actually modify VMAs after OOM conditions might arise, add a 'give up on oom' option and make an explicit contract that, should this flag be set, we absolutely will not modify any VMAs should OOM arise and just bail out. Since it'd be very unusual for a user to try to vma_modify() with this flag set but be specifying a range within a VMA which ends up being split (which can fail due to rlimit issues, not only OOM), we add a debug warning for this condition. The motivating reason for this is uffd release - syzkaller (and Pedro Falcato's VERY astute analysis) found a way in which an injected fault on allocation, triggering an OOM condition on commit merge, would result in uffd code becoming confused and treating an error value as if it were a VMA pointer. To avoid this, we make use of this new VMG flag to ensure that this never occurs, utilising the fact that, should we be clearing entire VMAs, we do not wish an OOM event to be reported to us. Many thanks to Pedro Falcato for his excellent analysis and Jann Horn for his insightful and intelligent analysis of the situation, both of whom were instrumental in this fix. Link: https://lkml.kernel.org/r/20250321100937.46634-1-lorenzo.stoakes@oracle.com Reported-by: syzbot+20ed41006cf9d842c2b5@syzkaller.appspotmail.com Closes: https://lore.kernel.org/all/67dc67f0.050a0220.25ae54.001e.GAE@google.com/ Fixes: 47b16d0 ("mm: abort vma_modify() on merge out of memory failure") Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com> Suggested-by: Pedro Falcato <pfalcato@suse.de> Suggested-by: Jann Horn <jannh@google.com> Cc: <stable@vger.kernel.org> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent 5e43fbe commit c103a75

3 files changed

Lines changed: 53 additions & 7 deletions

File tree

mm/userfaultfd.c

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1898,14 +1898,22 @@ struct vm_area_struct *userfaultfd_clear_vma(struct vma_iterator *vmi,
18981898
unsigned long end)
18991899
{
19001900
struct vm_area_struct *ret;
1901+
bool give_up_on_oom = false;
1902+
1903+
/*
1904+
* If we are modifying only and not splitting, just give up on the merge
1905+
* if OOM prevents us from merging successfully.
1906+
*/
1907+
if (start == vma->vm_start && end == vma->vm_end)
1908+
give_up_on_oom = true;
19011909

19021910
/* Reset ptes for the whole vma range if wr-protected */
19031911
if (userfaultfd_wp(vma))
19041912
uffd_wp_range(vma, start, end - start, false);
19051913

19061914
ret = vma_modify_flags_uffd(vmi, prev, vma, start, end,
19071915
vma->vm_flags & ~__VM_UFFD_FLAGS,
1908-
NULL_VM_UFFD_CTX);
1916+
NULL_VM_UFFD_CTX, give_up_on_oom);
19091917

19101918
/*
19111919
* In the vma_merge() successful mprotect-like case 8:
@@ -1956,7 +1964,8 @@ int userfaultfd_register_range(struct userfaultfd_ctx *ctx,
19561964
new_flags = (vma->vm_flags & ~__VM_UFFD_FLAGS) | vm_flags;
19571965
vma = vma_modify_flags_uffd(&vmi, prev, vma, start, vma_end,
19581966
new_flags,
1959-
(struct vm_userfaultfd_ctx){ctx});
1967+
(struct vm_userfaultfd_ctx){ctx},
1968+
/* give_up_on_oom = */false);
19601969
if (IS_ERR(vma))
19611970
return PTR_ERR(vma);
19621971

mm/vma.c

Lines changed: 34 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -903,7 +903,13 @@ static __must_check struct vm_area_struct *vma_merge_existing_range(
903903
if (anon_dup)
904904
unlink_anon_vmas(anon_dup);
905905

906-
vmg->state = VMA_MERGE_ERROR_NOMEM;
906+
/*
907+
* We've cleaned up any cloned anon_vma's, no VMAs have been
908+
* modified, no harm no foul if the user requests that we not
909+
* report this and just give up, leaving the VMAs unmerged.
910+
*/
911+
if (!vmg->give_up_on_oom)
912+
vmg->state = VMA_MERGE_ERROR_NOMEM;
907913
return NULL;
908914
}
909915

@@ -916,7 +922,15 @@ static __must_check struct vm_area_struct *vma_merge_existing_range(
916922
abort:
917923
vma_iter_set(vmg->vmi, start);
918924
vma_iter_load(vmg->vmi);
919-
vmg->state = VMA_MERGE_ERROR_NOMEM;
925+
926+
/*
927+
* This means we have failed to clone anon_vma's correctly, but no
928+
* actual changes to VMAs have occurred, so no harm no foul - if the
929+
* user doesn't want this reported and instead just wants to give up on
930+
* the merge, allow it.
931+
*/
932+
if (!vmg->give_up_on_oom)
933+
vmg->state = VMA_MERGE_ERROR_NOMEM;
920934
return NULL;
921935
}
922936

@@ -1076,9 +1090,15 @@ int vma_expand(struct vma_merge_struct *vmg)
10761090
return 0;
10771091

10781092
nomem:
1079-
vmg->state = VMA_MERGE_ERROR_NOMEM;
10801093
if (anon_dup)
10811094
unlink_anon_vmas(anon_dup);
1095+
/*
1096+
* If the user requests that we just give upon OOM, we are safe to do so
1097+
* here, as commit merge provides this contract to us. Nothing has been
1098+
* changed - no harm no foul, just don't report it.
1099+
*/
1100+
if (!vmg->give_up_on_oom)
1101+
vmg->state = VMA_MERGE_ERROR_NOMEM;
10821102
return -ENOMEM;
10831103
}
10841104

@@ -1520,6 +1540,13 @@ static struct vm_area_struct *vma_modify(struct vma_merge_struct *vmg)
15201540
if (vmg_nomem(vmg))
15211541
return ERR_PTR(-ENOMEM);
15221542

1543+
/*
1544+
* Split can fail for reasons other than OOM, so if the user requests
1545+
* this it's probably a mistake.
1546+
*/
1547+
VM_WARN_ON(vmg->give_up_on_oom &&
1548+
(vma->vm_start != start || vma->vm_end != end));
1549+
15231550
/* Split any preceding portion of the VMA. */
15241551
if (vma->vm_start < start) {
15251552
int err = split_vma(vmg->vmi, vma, start, 1);
@@ -1588,12 +1615,15 @@ struct vm_area_struct
15881615
struct vm_area_struct *vma,
15891616
unsigned long start, unsigned long end,
15901617
unsigned long new_flags,
1591-
struct vm_userfaultfd_ctx new_ctx)
1618+
struct vm_userfaultfd_ctx new_ctx,
1619+
bool give_up_on_oom)
15921620
{
15931621
VMG_VMA_STATE(vmg, vmi, prev, vma, start, end);
15941622

15951623
vmg.flags = new_flags;
15961624
vmg.uffd_ctx = new_ctx;
1625+
if (give_up_on_oom)
1626+
vmg.give_up_on_oom = true;
15971627

15981628
return vma_modify(&vmg);
15991629
}

mm/vma.h

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,12 @@ struct vma_merge_struct {
8787
struct anon_vma_name *anon_name;
8888
enum vma_merge_flags merge_flags;
8989
enum vma_merge_state state;
90+
91+
/*
92+
* If a merge is possible, but an OOM error occurs, give up and don't
93+
* execute the merge, returning NULL.
94+
*/
95+
bool give_up_on_oom :1;
9096
};
9197

9298
static inline bool vmg_nomem(struct vma_merge_struct *vmg)
@@ -206,7 +212,8 @@ __must_check struct vm_area_struct
206212
struct vm_area_struct *vma,
207213
unsigned long start, unsigned long end,
208214
unsigned long new_flags,
209-
struct vm_userfaultfd_ctx new_ctx);
215+
struct vm_userfaultfd_ctx new_ctx,
216+
bool give_up_on_oom);
210217

211218
__must_check struct vm_area_struct
212219
*vma_merge_new_range(struct vma_merge_struct *vmg);

0 commit comments

Comments
 (0)