Re: [PATCH v4 1/7] userfaultfd: move core VMA manipulation logic to mm/userfaultfd.c

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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





[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [NTFS 3]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [NTFS 3]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux