Hi Lorenzo Stoakes, On 2024-08-07 at 13:03:52 +0100, 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. Glad to know the repro environment is helpful. Thank you for your patch(I verified it's fixed) and it helps me learn more. > > (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! You are welcome. :) Best Regards, Thank you! > > 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> > --- > 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
