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 22E40C54EE9 for ; Tue, 13 Sep 2022 22:03:43 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 54AF96B0071; Tue, 13 Sep 2022 18:03:42 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 4F9AE8D0002; Tue, 13 Sep 2022 18:03:42 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 39BC88D0001; Tue, 13 Sep 2022 18:03:42 -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 2AAC86B0071 for ; Tue, 13 Sep 2022 18:03:42 -0400 (EDT) Received: from smtpin25.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id EC91BA0188 for ; Tue, 13 Sep 2022 22:03:41 +0000 (UTC) X-FDA: 79908439842.25.DA9C29F Received: from mail-pl1-f178.google.com (mail-pl1-f178.google.com [209.85.214.178]) by imf14.hostedemail.com (Postfix) with ESMTP id 937E31000A8 for ; Tue, 13 Sep 2022 22:03:41 +0000 (UTC) Received: by mail-pl1-f178.google.com with SMTP id c2so13213664plo.3 for ; Tue, 13 Sep 2022 15:03:41 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date; bh=NYBaVLRrXbe6H9m02gqcPbG6Tfzo/qSbO9657IGVEGU=; b=KuISZK38k47UtmNXtHsh4Po1U2TsA0Mq68x0dGj6QlRcjnGjQLfIMVVq5qDWJyXQw8 WMJholrGNEk4kauKtc3e/KRRBfmQUhO7Ut2oI1J5/tlZh8/c7k61DUishzbTcQ/pIFNR PQ8v21RMBDyFDfqYoXGSelP7mhC+hP/KrYq4w= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date; bh=NYBaVLRrXbe6H9m02gqcPbG6Tfzo/qSbO9657IGVEGU=; b=tDLrvneMEOoyoJNdFqonIVyIonpak6BzFyzubLRSNSJi8hsVt9AjSr2lmeYPs8ORqZ QiBG/1zX1u7xNAmaUU7V37MbxYeciSyfsfBEOqGwgf0veG04ZXcGH0nKPZyKYo0ZPUxY JXX2zDtC63Wr5OWuC5RnRHlSMGbeNyx3yfmyiyRaZ96XqD9JOseEoJBSvgsvxq96eOSV ciP5gWfFpWE16MlzswZHWm35pdLIxVYYkRQy53SrPuYRcNYo1wkyD+qxkS+jKzEq058U lsJFKgsvSwqv2jSecU9crRXkPq5CLgIzuYO7wQfMaAsa2LidlFtz2k0HzrAUW3NMi8yd CddQ== X-Gm-Message-State: ACgBeo1NLe7v1S7uBhvBvPnZhfH+KJ6h1WLgiuzvTZiEP01fP0hJr2+B 3ujkw6cMkHcumgj1Y2lA+QZjaA== X-Google-Smtp-Source: AA6agR6CNtB1UOHoxi9MrDvudnFn/NsbUZVavc+t+Q3VtrPU/A7zGmxMPrGoIyGLcWGhzziD+9R4VA== X-Received: by 2002:a17:902:e750:b0:176:b0fb:9683 with SMTP id p16-20020a170902e75000b00176b0fb9683mr34012585plf.71.1663106620537; Tue, 13 Sep 2022 15:03:40 -0700 (PDT) Received: from www.outflux.net (smtp.outflux.net. [198.145.64.163]) by smtp.gmail.com with ESMTPSA id e13-20020a17090301cd00b00177f4ef7970sm9124775plh.11.2022.09.13.15.03.39 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 13 Sep 2022 15:03:39 -0700 (PDT) Date: Tue, 13 Sep 2022 15:03:38 -0700 From: Kees Cook To: Jorge Merlino Cc: Alexander Viro , Eric Biederman , Ingo Molnar , Peter Zijlstra , Juri Lelli , Vincent Guittot , Dietmar Eggemann , Steven Rostedt , Ben Segall , Mel Gorman , Daniel Bristot de Oliveira , Valentin Schneider , linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] Fix race condition when exec'ing setuid files Message-ID: <202209131456.76A13BC5E4@keescook> References: <20220910211215.140270-1-jorge.merlino@canonical.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20220910211215.140270-1-jorge.merlino@canonical.com> ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1663106621; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=NYBaVLRrXbe6H9m02gqcPbG6Tfzo/qSbO9657IGVEGU=; b=7rThZHtO3Jk74m8RezWZvDWlo1F9/qirfwozr3vnLV5Mp4xUu4clWh8R6ED28tj+3vPEin 6qU/C594hqCFwvedbl+qxtSWiQW0QnXwiHyK5De90zO3+TcoNm0Wel7g8cbcDQuXu5WFgl xRrncFmbBWCptES8iZVicAMSoE4RjGE= ARC-Authentication-Results: i=1; imf14.hostedemail.com; dkim=pass header.d=chromium.org header.s=google header.b=KuISZK38; spf=pass (imf14.hostedemail.com: domain of keescook@chromium.org designates 209.85.214.178 as permitted sender) smtp.mailfrom=keescook@chromium.org; dmarc=pass (policy=none) header.from=chromium.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1663106621; a=rsa-sha256; cv=none; b=WB9tIVVo9Pfq1lt1sR1XTF4WHd7F3BLFKgLvpPC75SQNQcBLydN03rJJf5Wa9lqLmVfGGW sIkYQ53pIVRohtJzBOAwjOanhziCAeMwzx3L7T8guDBGSwgyKaIqrY+dZvFNHgvCvtF3MS n0WJjbB7HyzQYiPrUmbM2DIckH9M4+Y= Authentication-Results: imf14.hostedemail.com; dkim=pass header.d=chromium.org header.s=google header.b=KuISZK38; spf=pass (imf14.hostedemail.com: domain of keescook@chromium.org designates 209.85.214.178 as permitted sender) smtp.mailfrom=keescook@chromium.org; dmarc=pass (policy=none) header.from=chromium.org X-Rspamd-Server: rspam08 X-Rspam-User: X-Stat-Signature: rkos3injwcum67dyb4e7wmq1t3m6x6w5 X-Rspamd-Queue-Id: 937E31000A8 X-HE-Tag: 1663106621-168256 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: On Sat, Sep 10, 2022 at 06:12:14PM -0300, Jorge Merlino wrote: > This patch fixes a race condition in check_unsafe_exec when a heavily > threaded program tries to exec a setuid file. check_unsafe_exec counts the > number of threads sharing the same fs_struct and compares it to the total > number of users of the fs_struct by looking at its users counter. If there > are more users than process threads using it the setuid exec fails with > LSM_UNSAFE_SHARE. The problem is that, during the kernel_clone code > execution, the fs_struct users counter is incremented before the new thread > is added to the thread_group list. So there is a race when the counter has > been incremented but the thread is not visible to the while_each_tread loop > in check_unsafe_exec. Thanks for reporting this and for having a reproducer! It looks like this is "failing safe", in the sense that the bug causes an exec of a setuid binary to not actually change the euid. Is that an accurate understanding here? > This patch sort of fixes this by setting a process flag to the parent > process during the time this race is possible. Thus, if a process is > forking, it counts an extra user fo the fs_struct as the counter might be > incremented before the thread is visible. But this is not great as this > could generate the opposite problem as there may be an external process > sharing the fs_struct that is masked by some thread that is being counted > twice. I submit this patch just as an idea but mainly I want to introduce > this issue and see if someone comes up with a better solution. I'll want to spend some more time studying this race, but yes, it looks like it should get fixed. I'm curious, though, how did you find this problem? It seems quite unusual to have a high-load heavily threaded process decide to exec. -Kees -- Kees Cook