On 8/7/24 14:03, Lorenzo Stoakes wrote: > On Wed, Aug 07, 2024 at 11:45:56AM GMT, Pengfei Xu wrote: >> Hi Lorenzo Stoakes, >> >> Greetings! >> >> I used syzkaller and found >> KASAN: slab-use-after-free Read in userfaultfd_set_ctx in next-20240805. >> >> Bisected the first bad commit: >> 4651ba8201cf userfaultfd: move core VMA manipulation logic to mm/userfaultfd.c >> >> All detailed info: https://github.com/xupengfe/syzkaller_logs/tree/main/240806_122723_userfaultfd_set_ct > > [snip] > > Andrew - As this is so small, could you take this as a fix-patch? The fix > is enclosed below. > > > Pengfei - Sorry for the delay on getting this resolved, I was struggling to > repro with my usual dev setup, after trying a lot of things I ended up > using the supplied repro env and was able to do so there. > > (I suspect that VMAs are laid out slightly differently in my usual arch base > image perhaps based on tunables, and this was the delta on that!) > > Regardless, I was able to identify the cause - we incorrectly pass a stale > pointer to userfaultfd_reset_ctx() if a merge is performed in > userfaultfd_clear_vma(). > > This was a subtle mistake on my part, I don't see any other instances like > this in the patch. > > Syzkaller managed to get this merge to happen and kasan picked up on it, so > thank you very much for supplying the infra! > > The fix itself is very simple, a one-liner, enclosed below. > > ----8<---- > From 193abd1c3a51e6bf1d85ddfe01845e9713336970 Mon Sep 17 00:00:00 2001 > From: Lorenzo Stoakes <lorenzo.stoakes@xxxxxxxxxx> > Date: Wed, 7 Aug 2024 12:44:27 +0100 > Subject: [PATCH] mm: userfaultfd: fix user-after-free in > userfaultfd_clear_vma() > > After invoking vma_modify_flags_uffd() in userfaultfd_clear_vma(), we may > have merged the vma, and depending on the kind of merge, deleted the vma, > rendering the vma pointer invalid. > > The code incorrectly referenced this now possibly invalid vma pointer when > invoking userfaultfd_reset_ctx(). > > If no merge is possible, vma_modify_flags_uffd() performs a split and > returns the original vma. Therefore the correct approach is to simply pass > the ret pointer to userfaultfd_ret_ctx(). > > Reported-by: Pengfei Xu <pengfei.xu@xxxxxxxxx> > Fixes: e310f2b78a77 ("userfaultfd: move core VMA manipulation logic to mm/userfaultfd.c") > Closes: https://lore.kernel.org/all/ZrLt9HIxV9QiZotn@xxxxxxxxxxxxxxxx/ > Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@xxxxxxxxxx> Acked-by: Vlastimil Babka <vbabka@xxxxxxx> > --- > mm/userfaultfd.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/mm/userfaultfd.c b/mm/userfaultfd.c > index 3b7715ecf292..966e6c81a685 100644 > --- a/mm/userfaultfd.c > +++ b/mm/userfaultfd.c > @@ -1813,7 +1813,7 @@ struct vm_area_struct *userfaultfd_clear_vma(struct vma_iterator *vmi, > * the current one has not been updated yet. > */ > if (!IS_ERR(ret)) > - userfaultfd_reset_ctx(vma); > + userfaultfd_reset_ctx(ret); > > return ret; > } > -- > 2.45.2
