All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Howells <dhowells@redhat.com>
To: Max Kellermann <max.kellermann@ionos.com>
Cc: dhowells@redhat.com, Paulo Alcantara <pc@manguebit.org>,
	Christian Brauner <brauner@kernel.org>,
	netfs@lists.linux.dev, linux-fsdevel@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-stable@vger.kernel.org
Subject: Re: [PATCH] fs/netfs: fix reference leak
Date: Wed, 24 Sep 2025 16:39:26 +0100	[thread overview]
Message-ID: <755695.1758728366@warthog.procyon.org.uk> (raw)
In-Reply-To: <745741.1758727499@warthog.procyon.org.uk>

David Howells <dhowells@redhat.com> wrote:

> > ... and frees the allocation (without the "call_rcu" indirection).
> 
> Unfortunately, this isn't good.  The request has already been added to the
> proc list and is removed in netfs_deinit_request() by netfs_proc_del_rreq() -
> but that means that someone reading /proc/fs/netfs/requests can be looking at
> it as you free it.
> 
> You still need the call_rcu() - or you have to call synchronize_rcu().
> 
> I can change netfs_put_failed_request() to do the call_rcu() rather than
> mempool_free()/netfs_stat_d().

How about:

/*
 * Free a request (synchronously) that was just allocated but has failed before
 * it could be submitted.
 */
void netfs_put_failed_request(struct netfs_io_request *rreq)
{
	int r;

	/* New requests have two references (see netfs_alloc_request(), and
	 * this function is only allowed on new request objects
	 */
	if (!__refcount_sub_and_test(2, &rreq->ref, &r))
		WARN_ON_ONCE(1);

	trace_netfs_rreq_ref(rreq->debug_id, r, netfs_rreq_trace_put_failed);
	netfs_free_request(&rreq->cleanup_work);
}

David


  reply	other threads:[~2025-09-24 15:39 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-11 22:24 [PATCH] fs/netfs: fix reference leak Max Kellermann
2025-09-11 22:27 ` Fwd: " Max Kellermann
2025-09-12  5:36   ` Greg KH
2025-09-24 15:24 ` David Howells
2025-09-24 15:39   ` David Howells [this message]
2025-09-24 18:52     ` Max Kellermann
2025-09-24 19:10       ` David Howells

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=755695.1758728366@warthog.procyon.org.uk \
    --to=dhowells@redhat.com \
    --cc=brauner@kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-stable@vger.kernel.org \
    --cc=max.kellermann@ionos.com \
    --cc=netfs@lists.linux.dev \
    --cc=pc@manguebit.org \
    /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.