Linux filesystem development
 help / color / mirror / Atom feed
From: Al Viro <viro@zeniv.linux.org.uk>
To: Ahmet Eray Karadag <eraykrdg1@gmail.com>
Cc: akpm@linux-foundation.org, linux-kernel@vger.kernel.org,
	linux-fsdevel@vger.kernel.org, skhan@linuxfoundation.org,
	david.hunter.linux@gmail.com,
	syzbot+1c70732df5fd4f0e4fbb@syzkaller.appspotmail.com
Subject: Re: [PATCH] adfs: fix memory leak in sb->s_fs_info
Date: Sun, 14 Dec 2025 01:32:49 +0000	[thread overview]
Message-ID: <20251214013249.GI1712166@ZenIV> (raw)
In-Reply-To: <20251213233621.151496-2-eraykrdg1@gmail.com>

On Sun, Dec 14, 2025 at 02:36:22AM +0300, Ahmet Eray Karadag wrote:
> Syzbot reported a memory leak in adfs during the mount process. The issue
> arises because the ownership of the allocated (struct adfs_sb_info) is
> transferred from the filesystem context to the superblock via sget_fc().
> This function sets fc->s_fs_info to NULL after the transfer.
> 
> The ADFS filesystem previously used the default kill_block_super for
> superblock destruction. This helper performs generic cleanup but does not
> free the private sb->s_fs_info data. Since fc->s_fs_info is set to
> NULL during the transfer, the standard context cleanup (adfs_free_fc)
> also skips freeing this memory. As a result, if the superblock is
> destroyed, the allocated struct adfs_sb_info is leaked.
> 
> Fix this by implementing a custom .kill_sb callback (adfs_kill_sb)
> that explicitly frees sb->s_fs_info before invoking the generic
> kill_block_super.

I hate dealing with humans in the way one would deal with a chatbot, but...


Question: if that thing is leaking all the time, why hadn't that been caught
earlier?

Question: does it really leak all the time?  How would one check that?

Question: if it does not leak in each and every case, presumably the damn thing
does get freed at some point; where would that be?

Question: would we, by any chance, run into a double-free with that "fix"?


Please, do yourself a favour and find answers to the questions above.
They are fairly trivial and it is the kind of exercise one has to do every
time when dealing with something of that sort.

  reply	other threads:[~2025-12-14  1:32 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-12-13 23:36 [PATCH] adfs: fix memory leak in sb->s_fs_info Ahmet Eray Karadag
2025-12-14  1:32 ` Al Viro [this message]
2025-12-14  2:02   ` Al Viro
2025-12-14  2:27     ` Al Viro
2025-12-14  2:58       ` Ahmet Eray Karadag
2025-12-15  0:22 ` [PATCH v2] " Ahmet Eray Karadag
2025-12-15  1:55   ` Al Viro
2025-12-15  3:14 ` [PATCH v3] " Ahmet Eray Karadag

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=20251214013249.GI1712166@ZenIV \
    --to=viro@zeniv.linux.org.uk \
    --cc=akpm@linux-foundation.org \
    --cc=david.hunter.linux@gmail.com \
    --cc=eraykrdg1@gmail.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=skhan@linuxfoundation.org \
    --cc=syzbot+1c70732df5fd4f0e4fbb@syzkaller.appspotmail.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox