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=-11.7 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,INCLUDES_CR_TRAILER,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=unavailable 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 E633EC4338F for ; Tue, 24 Aug 2021 14:06:01 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id C192160F92 for ; Tue, 24 Aug 2021 14:06:01 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S237696AbhHXOGl (ORCPT ); Tue, 24 Aug 2021 10:06:41 -0400 Received: from mail.kernel.org ([198.145.29.99]:34246 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S237669AbhHXOGj (ORCPT ); Tue, 24 Aug 2021 10:06:39 -0400 Received: by mail.kernel.org (Postfix) with ESMTPSA id 0C14161212; Tue, 24 Aug 2021 14:05:53 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1629813955; bh=t4e2cm3gb1n1OucC5GO6DcA/X+Kew8TVxJ8GdOTEUtM=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=C4e0oBi8AN2a8ghIAD0nL7gCg90aJHiLdwld94QkvRhotWfHuCUqu2IfNeXYM46ya uDc5AZQQ5mhtSO0S9QX43NSGfK7dLgdcu9QaNmHA8kWrSDNS1aGATh1DL5YfFtkiNX GuMiTO9IQhQDDFapRHgMyOhd4Q52wmODZ+E76fm5UtqFz3RlsXhkNnge6Uikz9BIFv 0TamGOIWOU44lRXUdICbgm7hKXmzXOy6/A1nvLL6kHEuZbvMre0yCo+pcgaw/mDdtX xW/s1TeMoAaNXIBtIAmF6Einl83plBj+ICSmO4pLQKYbdQA7UlpqVOBKIvUmK8lPTQ qOOxokX58P9Rw== Date: Tue, 24 Aug 2021 17:05:50 +0300 From: Mike Rapoport To: Kees Cook Cc: Jordy Zomer , linux-kernel@vger.kernel.org, Andrew Morton , linux-mm@kvack.org, James Bottomley Subject: Re: [PATCH] mm/secretmem: use refcount_t instead of atomic_t Message-ID: References: <20210820043339.2151352-1-jordy@pwning.systems> <202108192227.8BE02F1C@keescook> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <202108192227.8BE02F1C@keescook> Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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. -- Sincerely yours, Mike.