From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.1 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,INCLUDES_PATCH,MAILING_LIST_MULTI, MENTIONS_GIT_HOSTING,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id ACBE8C433DF for ; Tue, 4 Aug 2020 20:58:51 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 92D8922D08 for ; Tue, 4 Aug 2020 20:58:51 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1596574731; bh=dtol9Ums/BnhF/0mrqyXgMMO0Dg0T0IcgMhAWs7Cpc4=; h=Date:From:To:Cc:Subject:References:In-Reply-To:List-ID:From; b=wMmd+L/YudixD1QdgtBgKyqcs1bDm25AoXj38DfD2B6clQ61ct5ptQ8W+kATpxX24 zEr4sniXKZ3iCmVDEseILu8gtPK9H2gePTpK+mmK55/cMQNXWMiHgR4YD4JdnX2AW6 I1IwtNW5BcVtZ/mbSeno4/YVsfksqgKiga6n/C2o= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728548AbgHDU6u (ORCPT ); Tue, 4 Aug 2020 16:58:50 -0400 Received: from mail.kernel.org ([198.145.29.99]:42850 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728394AbgHDU6s (ORCPT ); Tue, 4 Aug 2020 16:58:48 -0400 Received: from gmail.com (unknown [104.132.1.76]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id BCEFF2086A; Tue, 4 Aug 2020 20:58:47 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1596574728; bh=dtol9Ums/BnhF/0mrqyXgMMO0Dg0T0IcgMhAWs7Cpc4=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=HOjBDJkp94PpyHMCmrl7tSHNej/Cr+XBZUXcsfPLqD5Ny/cMs7p+4+dsx4mOwaX4x oMnMOtKJ/OExtbF+iF2OTWDctUS5Izc4qRGgB1tG/ZvARbF45Ed8eQwLNT1jwzmXR5 6CLfVzF54c5nIqm3rrD/iqnAkV0Dlu6vQHahVM4U= Date: Tue, 4 Aug 2020 13:58:46 -0700 From: Eric Biggers To: Lokesh Gidra Cc: Alexander Viro , Stephen Smalley , casey@schaufler-ca.com, James Morris , Kalesh Singh , Daniel Colascione , Suren Baghdasaryan , Linux FS Devel , linux-kernel , Nick Kralevich , Jeffrey Vander Stoep , Calin Juravle , kernel-team@android.com, yanfei.xu@windriver.com, syzbot+75867c44841cb6373570@syzkaller.appspotmail.com Subject: Re: [PATCH] Userfaultfd: Avoid double free of userfault_ctx and remove O_CLOEXEC Message-ID: <20200804205846.GB1992048@gmail.com> References: <20200804203155.2181099-1-lokeshgidra@google.com> <20200804204543.GA1992048@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Aug 04, 2020 at 01:49:30PM -0700, Lokesh Gidra wrote: > On Tue, Aug 4, 2020 at 1:45 PM Eric Biggers wrote: > > > > On Tue, Aug 04, 2020 at 01:31:55PM -0700, Lokesh Gidra wrote: > > > when get_unused_fd_flags returns error, ctx will be freed by > > > userfaultfd's release function, which is indirectly called by fput(). > > > Also, if anon_inode_getfile_secure() returns an error, then > > > userfaultfd_ctx_put() is called, which calls mmdrop() and frees ctx. > > > > > > Also, the O_CLOEXEC was inadvertently added to the call to > > > get_unused_fd_flags() [1]. > > > > > > Adding Al Viro's suggested-by, based on [2]. > > > > > > [1] https://lore.kernel.org/lkml/1f69c0ab-5791-974f-8bc0-3997ab1d61ea@dancol.org/ > > > [2] https://lore.kernel.org/lkml/20200719165746.GJ2786714@ZenIV.linux.org.uk/ > > > > > > Fixes: d08ac70b1e0d (Wire UFFD up to SELinux) > > > Suggested-by: Al Viro > > > Reported-by: syzbot+75867c44841cb6373570@syzkaller.appspotmail.com > > > Signed-off-by: Lokesh Gidra > > > > What branch does this patch apply to? Neither mainline nor linux-next works. > > > On James Morris' tree (secure_uffd_v5.9 branch). > For those of us not "in the know", that apparently means branch secure_uffd_v5.9 of https://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git Perhaps it would make more sense to resend your original patch series with this fix folded in? > diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c > index ae859161908f..e15eb8fdc083 100644 > --- a/fs/userfaultfd.c > +++ b/fs/userfaultfd.c > @@ -2042,24 +2042,18 @@ SYSCALL_DEFINE1(userfaultfd, int, flags) > O_RDWR | (flags & UFFD_SHARED_FCNTL_FLAGS), > NULL); > if (IS_ERR(file)) { > - fd = PTR_ERR(file); > - goto out; > + userfaultfd_ctx_put(ctx); > + return PTR_ERR(file); > } > > - fd = get_unused_fd_flags(O_RDONLY | O_CLOEXEC); > + fd = get_unused_fd_flags(O_RDONLY); > if (fd < 0) { > fput(file); > - goto out; > + return fd; > } > > ctx->owner = file_inode(file); > fd_install(fd, file); > - > -out: > - if (fd < 0) { > - mmdrop(ctx->mm); > - kmem_cache_free(userfaultfd_ctx_cachep, ctx); > - } > return fd; This introduces the opposite bug: now it's hardcoded to *not* use O_CLOEXEC, instead of using the flag the user passed in the flags argument to the syscall. - Eric