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 Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 13243C433F5 for ; Thu, 19 May 2022 01:51:41 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 81A486B0072; Wed, 18 May 2022 21:51:40 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 7C8C26B0073; Wed, 18 May 2022 21:51:40 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 693676B0074; Wed, 18 May 2022 21:51:40 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id 55DEE6B0072 for ; Wed, 18 May 2022 21:51:40 -0400 (EDT) Received: from smtpin27.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay12.hostedemail.com (Postfix) with ESMTP id 2461812058B for ; Thu, 19 May 2022 01:51:40 +0000 (UTC) X-FDA: 79480815960.27.C600EDE Received: from szxga01-in.huawei.com (szxga01-in.huawei.com [45.249.212.187]) by imf10.hostedemail.com (Postfix) with ESMTP id A6AB2C00A0 for ; Thu, 19 May 2022 01:51:08 +0000 (UTC) Received: from kwepemi100023.china.huawei.com (unknown [172.30.72.54]) by szxga01-in.huawei.com (SkyGuard) with ESMTP id 4L3XqS5yZnzgYNF; Thu, 19 May 2022 09:50:12 +0800 (CST) Received: from kwepemm600013.china.huawei.com (7.193.23.68) by kwepemi100023.china.huawei.com (7.221.188.59) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2375.24; Thu, 19 May 2022 09:51:35 +0800 Received: from [10.174.178.46] (10.174.178.46) by kwepemm600013.china.huawei.com (7.193.23.68) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2375.24; Thu, 19 May 2022 09:51:34 +0800 Subject: Re: [PATCH -next] exec: Remove redundant check in do_open_execat/uselib To: Kees Cook , Andrew Morton CC: , , , , References: <20220518081227.1278192-1-chengzhihao1@huawei.com> <20220518104601.fc21907008231b60a0e54a8e@linux-foundation.org> <202205181215.D448675BEA@keescook> From: Zhihao Cheng Message-ID: Date: Thu, 19 May 2022 09:51:34 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.5.0 MIME-Version: 1.0 In-Reply-To: <202205181215.D448675BEA@keescook> Content-Type: text/plain; charset="gbk"; format=flowed Content-Transfer-Encoding: 8bit X-Originating-IP: [10.174.178.46] X-ClientProxiedBy: dggems703-chm.china.huawei.com (10.3.19.180) To kwepemm600013.china.huawei.com (7.193.23.68) X-CFilter-Loop: Reflected X-Rspamd-Queue-Id: A6AB2C00A0 X-Stat-Signature: z1n15u1phz66udyq5mutanuw3qdqgiz7 X-Rspam-User: Authentication-Results: imf10.hostedemail.com; dkim=none; dmarc=pass (policy=quarantine) header.from=huawei.com; spf=pass (imf10.hostedemail.com: domain of chengzhihao1@huawei.com designates 45.249.212.187 as permitted sender) smtp.mailfrom=chengzhihao1@huawei.com X-Rspamd-Server: rspam09 X-HE-Tag: 1652925068-795865 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: ÔÚ 2022/5/19 3:17, Kees Cook дµÀ: >>> WARNON(path_noexec(&file->f_path)) // path_noexec() checks fail > > Did you encounter this in the real world? I found the problem by running fuzz test.(syzkaller) Here is a brief reproducer. 1. Apply diff 2. Complie and run repo.c diff diff --git a/fs/exec.c b/fs/exec.c index e3e55d5e0be1..388d38b87e9a 100644 --- a/fs/exec.c +++ b/fs/exec.c @@ -897,6 +897,7 @@ EXPORT_SYMBOL(transfer_args_to_stack); #endif /* CONFIG_MMU */ +#include static struct file *do_open_execat(int fd, struct filename *name, int flags) { struct file *file; @@ -925,9 +926,15 @@ static struct file *do_open_execat(int fd, struct filename *name, int flags) * and check again at the very end too. */ err = -EACCES; + if (!strcmp(file->f_path.dentry->d_iname, "my_bin")) { + pr_err("wait ...\n"); + msleep(3000); + } if (WARN_ON_ONCE(!S_ISREG(file_inode(file)->i_mode) || - path_noexec(&file->f_path))) + path_noexec(&file->f_path))) { + pr_err("exec %pd %d %d %s\n", file->f_path.dentry, file->f_path.mnt->mnt_flags & MNT_NOEXEC, file->f_path.mnt->mnt_sb->s_iflags & SB_I_NOEXEC, file->f_path.mnt->mnt_sb->s_type->name); goto exit; + } err = deny_write_access(file); if (err) repo.c int main(void) { int ret; system("umount temp 2>&1 > /dev/null"); system("mount -t tmpfs none temp"); system("echo 12312 > temp/my_bin && chmod +x temp/my_bin"); ret = fork(); if (ret < 0) { perror("fork fail"); return 0; } if (ret == 0) { system("mount -oremount,noexec temp"); exit(0); } else { execve("/root/temp/my_bin", NULL, 0); //syscall(__NR_uselib, "/root/temp/my_bin"); } return 0; } > >> >> You're saying this is a race condition? A concurrent remount causes >> this warning? > > It seems not an unreasonable thing to warn about. Perhaps since it's > technically reachable from userspace, it could be downgraded to > pr_warn(), but I certainly don't want to remove the checks. > > I'd like to leave this as-is, since we _do_ want to find the cases where > we're about to allow an exec and a very important security check was NOT > handled. >I think removing redundant checking is okay, do_open_execat/uselib has initialized the acc_mode and open_flag for exec file, the check is equivalent to check in may_open(). Remount(noexec) operations can alos happen after the latest check, double check has no means for the concurrent situation. The MNT_NOEXEC flag only affects the open operation, it won't cause any problems that an opened bin file is executing in a non-exec mounted filesystem.