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 3E896C433F5 for ; Tue, 26 Oct 2021 01:11:20 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id AE7BD60F02 for ; Tue, 26 Oct 2021 01:11:19 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org AE7BD60F02 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=linux-foundation.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=kvack.org Received: by kanga.kvack.org (Postfix) id 2C136940008; Mon, 25 Oct 2021 21:11:19 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 26F5D940007; Mon, 25 Oct 2021 21:11:19 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 15F41940008; Mon, 25 Oct 2021 21:11:19 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0075.hostedemail.com [216.40.44.75]) by kanga.kvack.org (Postfix) with ESMTP id 06DE4940007 for ; Mon, 25 Oct 2021 21:11:19 -0400 (EDT) Received: from smtpin34.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay05.hostedemail.com (Postfix) with ESMTP id B2E49183C16B3 for ; Tue, 26 Oct 2021 01:11:18 +0000 (UTC) X-FDA: 78736810236.34.3EE2B58 Received: from mail-lf1-f44.google.com (mail-lf1-f44.google.com [209.85.167.44]) by imf07.hostedemail.com (Postfix) with ESMTP id 56D891000099 for ; Tue, 26 Oct 2021 01:11:18 +0000 (UTC) Received: by mail-lf1-f44.google.com with SMTP id bq11so13813104lfb.10 for ; Mon, 25 Oct 2021 18:11:18 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux-foundation.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=rtWXwfuaH6YO89IeWMAaSmOX7k04XWxVVJQflOLKKPU=; b=DjFGmggF2BzXZ3m0L7IJhIQ6i1iK4kxkRXO1HpAV7yknmCHxWUZBIB7IOldZXa1w5Q 1GHozxd5zQsANLVpe5xqWWzm4PK35l3/hXuDbzTDZHtJQLkYzgBAsNRhBt4Ts920bt8T vkDTc5JaWSDAKi3dB/TDj6IDLUWMB0bE7alKM= 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=rtWXwfuaH6YO89IeWMAaSmOX7k04XWxVVJQflOLKKPU=; b=3Y4JDDA0JgClf2L8FjoAn5OizlyX2hCSAzijFdossPBHvJGyfQaPFb+ZlGzkEJIATm 0AFc1KYLVVNAX0AwHmv/bPZIGwjRS/HKJFeVEwkMrLCLryAAwPBC8dDQbahHJo7HBkkK PYzyc5+L0FO3e/CWWQLyuhOA0fpY5cxEHFinwe9VprZjZVYBNVuBzEgflt/2kEYVKTNf 2XWEta5pGA/+iwYzhRPiRdX5eQgakZ2nDsajqODqSif2S6CFJOwQqjVuDHEA1RkJi6db QUCQSn47eapwGUnObJ00g7EhiS73Fme8pOEHPVBSMHUR3cBPlotgLyZxA1kJQFI1PNh5 HhvA== X-Gm-Message-State: AOAM533UH3fHVQBFdwL0STXy7SaJPU5/MrTS+zlTEyPfG7+iLeV5tedZ 5v9Bcqm6mSOLQH1pYHeyG5Obl91DBeWx5g== X-Google-Smtp-Source: ABdhPJxGR4J6t7VtzmtQklh5G+c1GItcyBEffYdWGM4B0EzkuNSkYzZHRUM0VOtt565CRgbYUGTPgw== X-Received: by 2002:a05:6512:150a:: with SMTP id bq10mr20076061lfb.68.1635210675569; Mon, 25 Oct 2021 18:11:15 -0700 (PDT) Received: from mail-lj1-f178.google.com (mail-lj1-f178.google.com. [209.85.208.178]) by smtp.gmail.com with ESMTPSA id z13sm630915lfd.143.2021.10.25.18.11.14 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 25 Oct 2021 18:11:15 -0700 (PDT) Received: by mail-lj1-f178.google.com with SMTP id u5so15554526ljo.8 for ; Mon, 25 Oct 2021 18:11:14 -0700 (PDT) X-Received: by 2002:a2e:bc24:: with SMTP id b36mr22545470ljf.95.1635210674455; Mon, 25 Oct 2021 18:11:14 -0700 (PDT) MIME-Version: 1.0 References: <20211025181634.3889666-1-willy@infradead.org> <202110251225.D01841AE67@keescook> <202110251402.ADFA4D41BF@keescook> <202110251438.1762406A5@keescook> <202110251706.BBEE1428@keescook> In-Reply-To: <202110251706.BBEE1428@keescook> From: Linus Torvalds Date: Mon, 25 Oct 2021 18:10:58 -0700 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH] secretmem: Prevent secretmem_users from wrapping to zero To: Kees Cook Cc: Matthew Wilcox , Linux Kernel Mailing List , Linux-MM , Jordy Zomer , James Bottomley , Mike Rapoport , Andrew Morton Content-Type: text/plain; charset="UTF-8" X-Rspamd-Server: rspam03 X-Rspamd-Queue-Id: 56D891000099 X-Stat-Signature: pify1hec8ixk4ghtep3k9of85zr577w6 Authentication-Results: imf07.hostedemail.com; dkim=pass header.d=linux-foundation.org header.s=google header.b=DjFGmggF; dmarc=none; spf=pass (imf07.hostedemail.com: domain of torvalds@linuxfoundation.org designates 209.85.167.44 as permitted sender) smtp.mailfrom=torvalds@linuxfoundation.org X-HE-Tag: 1635210678-223890 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 Mon, Oct 25, 2021 at 5:18 PM Kees Cook wrote: > > Right, sure, but it's not a rare pattern. Well, for an actual reference count it certainly isn't a rare pattern, and zero _is_ special, because at zero, you are now in use-after-free territory. But that's kind of the issue here: that really isn't what 'secretmem_users' was ever about. Zero isn't some "now we're use-after-free" situation. Quite the reverse. Zero ends up being the safe thing. So with that kind of "just count number of existing users", where zero isn't special, then refcount_t doesn't make sense. And refcount_t is for non-core stuff that has a lot of random kernel users that you can't easily verify. In contrast, 'secretmem_users' had exactly two sites that modified it, and one that tested it. > But these places need to check for insane > conditions too ("we got a -1 back -- this means there's a bug but what > do we do?"). Same for atomic_inc(): "oh, we're at our limit, do > something", but what above discovering ourselves above the limit? So honestly, "above the limit" is often perfectly fine too. It can be fine for two very different reasons: (a) racy checks are often much simpler and faster, and perfectly safe when the limit is "far away from overflow". (b) limits can change And (a) isn't just about "avoid special atomics". It's about doing the limit check optimistically outside locking etc. And (b) wasn't an issue here (where the only real use was ltierally "are there any users at all"), but in most _proper_ use cases you will want to have some resource limit that might be set by MIS. And might be changed dynamically. So it's entirely possible that somebody sets the limit to something smaller than the current user (prep for shutdown, whatever), without it being an error at all. The limit is for future work, not for past work. Easily happens with things like rlimits etc. > There's nothing about using the atomic_t primitives that enforces these > kinds of checks. (And there likely shouldn't be for atomic_t -- it's a > plain type.) But we likely need something that fills in this API gap > between atomic_t and refcount_t. I dispute the "need". This isn't as common as you claim. Most resource counting _is_ for "free when no longer used". And on the other end, you have the users that don't want refcount_t because they can't live with the limitations of that interface, like the page counts etc, that do it properly. So I think in 99% of all situations, the proper fix is to embed an "atomic_t" in the type it protects, and then have the helper functions that actually do it properly. Like we do for "get_page()" and friends. The "new type" isn't about the reference counting, it's about the data itself, and the atomic_t is just a part of it. Could we do something new type that warns on the "decrement past zero" and "overflow on increment"? Sure. But since they by _definition_ aren't about data lifetimes, they probably don't need saturation - you want the _warning_, but they aren't protecting data, since they aren't refcounts. Or could we have something even fancier, that is an actual defined range, and "overflow" is not "overflow in 32 bits", but "becomes bigger than X")? That gets more complex because now you'd have to encode the range in the type somehow. You could do it with actual static types (generate typedef names and code), or you could do it with types that have a more dynamic pointer to ranges (kind of like the sysfs interfaces do) or have the ranges embedded in the data structure itself. But honestly, the complexity downside seems to just dwarf the upside. Linus