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 mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id A2B6FC433EF for ; Thu, 21 Oct 2021 09:00:23 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 39B0960E9C for ; Thu, 21 Oct 2021 09:00:23 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 39B0960E9C Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=kvack.org Received: by kanga.kvack.org (Postfix) id B98906B0071; Thu, 21 Oct 2021 05:00:22 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id B47AF6B0072; Thu, 21 Oct 2021 05:00:22 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id A363F900002; Thu, 21 Oct 2021 05:00:22 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0038.hostedemail.com [216.40.44.38]) by kanga.kvack.org (Postfix) with ESMTP id 95E516B0071 for ; Thu, 21 Oct 2021 05:00:22 -0400 (EDT) Received: from smtpin05.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with ESMTP id 577302FDF6 for ; Thu, 21 Oct 2021 09:00:22 +0000 (UTC) X-FDA: 78719848284.05.7B7EAE2 Received: from mail-oi1-f173.google.com (mail-oi1-f173.google.com [209.85.167.173]) by imf29.hostedemail.com (Postfix) with ESMTP id 273EB9000270 for ; Thu, 21 Oct 2021 09:00:19 +0000 (UTC) Received: by mail-oi1-f173.google.com with SMTP id g125so33083oif.9 for ; Thu, 21 Oct 2021 02:00:21 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=RxeQAhKq35QFKlvh0wF1hKrofkjgeU944O74OCk+Hsc=; b=AIY0ZvgmHs1i2Jjewb/l7F+u1TVlacdEKavBVM+t4cp0J8AdxfOoodcM9OiO6xIxwU g0wct/DAnD8STZH0RivKu5czP7QryMpzdZsdoa/hpqB08G1sVrOqNBGq+Jg1Zdwij3eO h3pGGihW4Vj1g5pKuypqmC7n8MAGlnJCSMAOl+H3xp8qR5ZrY6dGWsu3/sRtUrm/PIkT SnDTRETq6vw03AWK7c2uFkTAxDAnc+J4r+dvBfhBt7M9s9/+8dlFfuxS+xtguKC4Zc99 DCCG+6rMPLO1fIpcnnhwq68dvrCFaobDR/9weVjd5b0opaRTKjhBsR4kNJRINREATGwB kWfg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=RxeQAhKq35QFKlvh0wF1hKrofkjgeU944O74OCk+Hsc=; b=PKOCUM0DFaD3hzwKHImBFbQmTlSjzoQIu1OnSFmXfi2ibzQYn4CKqxkiO61NHDh0l0 CIxhRLK3vseLVKwa2bA/zaQUlX1vvDSRu8uffXVOKI82GilX9n1W7iEgrilb+UHzh6mX ir02wXUcQbSmeMWS4LoleG1W068CSYEcIc2KYwlZlKotXxdLOZEFVfKnhvBdBeUQ54eM 1b79QceSIC1RBCVNiqBH42cawhJKaP+e8uXeN6xuA0jQ2uuvOViw9gVD8s5+AYsWbYD2 MSfZR47Imiu7fxAl+3RhkNcEq9dUjeEYHcVZ4nOYVoDjGT8XMHdygKZ45pJ4dLlRHOD7 JKfA== X-Gm-Message-State: AOAM533moGQmrR64ks4rZfJ8Y1/rK7s4ZNUn8xstmmPR6r1qiwweCtxQ uQ1YpkZpp/WP5d70W+buN42oTOgYddkTTCwRxxFXFA== X-Google-Smtp-Source: ABdhPJw0EqXC972LTR3d9LQ+cFNpenvaE/C+SPIaBoTi8mq6VP1mHeAwXqLj90EHQe+j5liHQBl16VdrBUimnz7kddE= X-Received: by 2002:a05:6808:ec9:: with SMTP id q9mr3594527oiv.160.1634806820945; Thu, 21 Oct 2021 02:00:20 -0700 (PDT) MIME-Version: 1.0 References: <20210820043339.2151352-1-jordy@pwning.systems> <202108192227.8BE02F1C@keescook> In-Reply-To: From: Dmitry Vyukov Date: Thu, 21 Oct 2021 11:00:09 +0200 Message-ID: Subject: Re: [PATCH] mm/secretmem: use refcount_t instead of atomic_t To: Mike Rapoport Cc: Kees Cook , Jordy Zomer , linux-kernel@vger.kernel.org, Andrew Morton , linux-mm@kvack.org, James Bottomley Content-Type: text/plain; charset="UTF-8" X-Stat-Signature: zap6tqynitdsax6oajrsy5sauno3fg1o X-Rspamd-Server: rspam01 X-Rspamd-Queue-Id: 273EB9000270 Authentication-Results: imf29.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=AIY0Zvgm; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf29.hostedemail.com: domain of dvyukov@google.com designates 209.85.167.173 as permitted sender) smtp.mailfrom=dvyukov@google.com X-HE-Tag: 1634806819-785104 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 Tue, 24 Aug 2021 at 16:06, Mike Rapoport wrote: > > On Thu, Aug 19, 2021 at 10:33:49PM -0700, Kees Cook wrote: > > On Fri, Aug 20, 2021 at 06:33:38AM +0200, Jordy Zomer wrote: > > > When a secret memory region is active, memfd_secret disables > > > hibernation. One of the goals is to keep the secret data from being > > > written to persistent-storage. > > > > > > It accomplishes this by maintaining a reference count to > > > `secretmem_users`. Once this reference is held your system can not be > > > hibernated due to the check in `hibernation_available()`. However, > > > because `secretmem_users` is of type `atomic_t`, reference counter > > > overflows are possible. > > > > It's an unlikely condition to hit given max-open-fds, etc, but there's > > no reason to leave this weakness. Changing this to refcount_t is easy > > and better than using atomic_t. > > > > Reviewed-by: Kees Cook > > > > > As you can see there's an `atomic_inc` for each `memfd` that is opened > > > in the `memfd_secret` syscall. If a local attacker succeeds to open 2^32 > > > memfd's, the counter will wrap around to 0. This implies that you may > > > hibernate again, even though there are still regions of this secret > > > memory, thereby bypassing the security check. > > > > IMO, this hibernation check is also buggy, since it looks to be > > vulnerable to ToCToU: processes aren't frozen when > > hibernation_available() checks secretmem_users(), so a process could add > > one and fill it before the process freezer stops it. > > > > And of course, there's still the ptrace hole[1], which is think is quite > > serious as it renders the entire defense moot. > > I thought about what can be done here and could not come up with anything > better that prevent PTRACE on a process with secretmem, but this seems to > me too much from usability vs security POV. > > Protecting against root is always hard and secretmem anyway does not > provide 100% guarantee by itself but rather makes an accidental data leak > or non-target attack much harder. > > To be effective it also presumes that other hardening features are turned > on by the system administrator on production systems, so it's not > unrealistic to rely on ptrace being disabled. Hi, The issue existed before this change, but I think refcount_inc needs to be done before fd_install. After fd_install finishes, the fd can be used by userspace and we can have secret data in memory before the refcount_inc. A straightforward mis-use where a user will predict the returned fd in another thread before the syscall returns and will use it to store secret data is somewhat dubious because such a user just shoots themself in the foot. But a more interesting mis-used would be to close the predicted fd and decrement the refcount before the corresponding refcount_inc, this way one can briefly drop the refcount to zero while there are other users of secretmem.