All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kees Cook <keescook@chromium.org>
To: Al Viro <viro@zeniv.linux.org.uk>
Cc: Christian Brauner <brauner@kernel.org>, Jan Kara <jack@suse.cz>,
	linux-fsdevel@vger.kernel.org,
	Zack Rusin <zack.rusin@broadcom.com>,
	Broadcom internal kernel review list
	<bcm-kernel-feedback-list@broadcom.com>,
	 Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
	Maxime Ripard <mripard@kernel.org>,
	Thomas Zimmermann <tzimmermann@suse.de>,
	David Airlie <airlied@gmail.com>, Daniel Vetter <daniel@ffwll.ch>,
	Jani Nikula <jani.nikula@linux.intel.com>,
	Joonas Lahtinen <joonas.lahtinen@linux.intel.com>,
	Rodrigo Vivi <rodrigo.vivi@intel.com>,
	Tvrtko Ursulin <tursulin@ursulin.net>,
	Andi Shyti <andi.shyti@linux.intel.com>,
	Lucas De Marchi <lucas.demarchi@intel.com>,
	Matt Atwood <matthew.s.atwood@intel.com>,
	Matthew Auld <matthew.auld@intel.com>,
	Nirmoy Das <nirmoy.das@intel.com>,
	Jonathan Cavitt <jonathan.cavitt@intel.com>,
	Will Deacon <will@kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Boqun Feng <boqun.feng@gmail.com>,
	Mark Rutland <mark.rutland@arm.com>,
	Kent Overstreet <kent.overstreet@linux.dev>,
	Masahiro Yamada <masahiroy@kernel.org>,
	Nathan Chancellor <nathan@kernel.org>,
	Nicolas Schier <nicolas@fjasle.eu>,
	Andrew Morton <akpm@linux-foundation.org>,
	linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org,
	intel-gfx@lists.freedesktop.org, linux-kbuild@vger.kernel.org,
	linux-hardening@vger.kernel.org
Subject: Re: [PATCH 5/5] fs: Convert struct file::f_count to refcount_long_t
Date: Thu, 2 May 2024 15:52:21 -0700	[thread overview]
Message-ID: <202405021548.040579B1C@keescook> (raw)
In-Reply-To: <20240502224250.GM2118490@ZenIV>

On Thu, May 02, 2024 at 11:42:50PM +0100, Al Viro wrote:
> On Thu, May 02, 2024 at 03:33:40PM -0700, Kees Cook wrote:
> > Underflow of f_count needs to be more carefully detected than it
> > currently is. The results of get_file() should be checked, but the
> > first step is detection. Redefine f_count from atomic_long_t to
> > refcount_long_t.
> 
> 	It is used on fairly hot paths.  What's more, it's not
> at all obvious what the hell would right semantics be.

I think we've put performance concerns between refcount_t and atomic_t
to rest long ago. If there is a real workload where it's a problem,
let's find it! :)

As for semantics, what do you mean? Detecting dec-below-zero means we
catch underflow, and detected inc-from-zero means we catch resurrection
attempts. In both cases we avoid double-free, but we have already lost
to a potential dangling reference to a freed struct file. But just
letting f_count go bad seems dangerous.

-- 
Kees Cook

  reply	other threads:[~2024-05-02 22:52 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-05-02 22:33 [PATCH 0/5] fs: Do not allow get_file() to resurrect 0 f_count Kees Cook
2024-05-02 22:33 ` [PATCH 1/5] " Kees Cook
2024-05-02 22:53   ` Jann Horn
2024-05-02 23:03     ` Kees Cook
2024-05-03  9:02       ` Christian Brauner
2024-05-06 10:41         ` Hillf Danton
2024-05-02 22:33 ` [PATCH 2/5] drm/vmwgfx: Do not directly manipulate file->f_count Kees Cook
2024-05-02 22:33 ` [PATCH 3/5] drm/i915: " Kees Cook
2024-05-02 22:33 ` [PATCH 4/5] refcount: Introduce refcount_long_t and APIs Kees Cook
2024-05-06  8:04   ` kernel test robot
2024-05-02 22:33 ` [PATCH 5/5] fs: Convert struct file::f_count to refcount_long_t Kees Cook
2024-05-02 22:42   ` Al Viro
2024-05-02 22:52     ` Kees Cook [this message]
2024-05-02 23:12       ` Al Viro
2024-05-02 23:21         ` Kees Cook
2024-05-02 23:41           ` Al Viro
2024-05-03  0:10             ` Kees Cook
2024-05-03  0:14               ` Al Viro
2024-05-03  0:41                 ` Kees Cook
2024-05-03  9:37                   ` Christian Brauner
2024-05-03 10:36                     ` Peter Zijlstra
2024-05-03 11:35                       ` Christian Brauner
2024-05-02 23:08 ` ✗ Fi.CI.CHECKPATCH: warning for fs: Do not allow get_file() to resurrect 0 f_count Patchwork
2024-05-02 23:08 ` ✗ Fi.CI.SPARSE: " Patchwork
2024-05-02 23:15 ` ✓ Fi.CI.BAT: success " Patchwork
2024-05-03  2:28 ` ✓ Fi.CI.IGT: " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=202405021548.040579B1C@keescook \
    --to=keescook@chromium.org \
    --cc=airlied@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=andi.shyti@linux.intel.com \
    --cc=bcm-kernel-feedback-list@broadcom.com \
    --cc=boqun.feng@gmail.com \
    --cc=brauner@kernel.org \
    --cc=daniel@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=jack@suse.cz \
    --cc=jani.nikula@linux.intel.com \
    --cc=jonathan.cavitt@intel.com \
    --cc=joonas.lahtinen@linux.intel.com \
    --cc=kent.overstreet@linux.dev \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-hardening@vger.kernel.org \
    --cc=linux-kbuild@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lucas.demarchi@intel.com \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=mark.rutland@arm.com \
    --cc=masahiroy@kernel.org \
    --cc=matthew.auld@intel.com \
    --cc=matthew.s.atwood@intel.com \
    --cc=mripard@kernel.org \
    --cc=nathan@kernel.org \
    --cc=nicolas@fjasle.eu \
    --cc=nirmoy.das@intel.com \
    --cc=peterz@infradead.org \
    --cc=rodrigo.vivi@intel.com \
    --cc=tursulin@ursulin.net \
    --cc=tzimmermann@suse.de \
    --cc=viro@zeniv.linux.org.uk \
    --cc=will@kernel.org \
    --cc=zack.rusin@broadcom.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.