| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20260204173029.GL3183987@ZenIV>
Date: Wed, 4 Feb 2026 17:30:29 +0000
From: Al Viro <viro@...iv.linux.org.uk>
To: Viacheslav Dubeyko <Slava.Dubeyko@....com>
Cc: "shardulsb08@...il.com" <shardulsb08@...il.com>,
"jack@...e.cz" <jack@...e.cz>,
"frank.li@...o.com" <frank.li@...o.com>,
"brauner@...nel.org" <brauner@...nel.org>,
"linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>,
"slava@...eyko.com" <slava@...eyko.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"shardul.b@...ricsoftware.com" <shardul.b@...ricsoftware.com>,
"glaubitz@...sik.fu-berlin.de" <glaubitz@...sik.fu-berlin.de>,
"janak@...ricsoftware.com" <janak@...ricsoftware.com>,
"syzbot+99f6ed51479b86ac4c41@...kaller.appspotmail.com" <syzbot+99f6ed51479b86ac4c41@...kaller.appspotmail.com>
Subject: Re: [PATCH v2] hfsplus: fix s_fs_info leak on mount setup failure
On Tue, Feb 03, 2026 at 11:35:18PM +0000, Viacheslav Dubeyko wrote:
> On Tue, 2026-02-03 at 04:38 +0000, Al Viro wrote:
> > On Mon, Feb 02, 2026 at 05:53:57PM +0000, Viacheslav Dubeyko wrote:
> > > > out_unload_nls:
> > > > - unload_nls(sbi->nls);
> > ^^^^^^^^^^^^^^^^^^^^
> > > > unload_nls(nls);
> > > > - kfree(sbi);
> >
> > > The patch [1] fixes the issue and it in HFS/HFS+ tree already.
> >
> > AFAICS, [1] lacks this removal of unload_nls() on failure exit.
> > IOW, the variant in your tree does unload_nls(sbi->nls) twice...
>
> Yeah, I think you are right here.
While we are at it, this
kfree(sbi->s_vhdr_buf);
kfree(sbi->s_backup_vhdr_buf);
might as well go into ->kill_sb(). That would result in the (untested)
delta below and IMO it's easier to follow that way...
diff --git a/fs/hfsplus/super.c b/fs/hfsplus/super.c
index 592d8fbb748c..6ce4d121e446 100644
--- a/fs/hfsplus/super.c
+++ b/fs/hfsplus/super.c
@@ -348,8 +348,6 @@ static void hfsplus_put_super(struct super_block *sb)
hfs_btree_close(sbi->attr_tree);
hfs_btree_close(sbi->cat_tree);
hfs_btree_close(sbi->ext_tree);
- kfree(sbi->s_vhdr_buf);
- kfree(sbi->s_backup_vhdr_buf);
hfs_dbg("finished\n");
}
@@ -471,7 +469,7 @@ static int hfsplus_fill_super(struct super_block *sb, struct fs_context *fc)
if (be16_to_cpu(vhdr->version) < HFSPLUS_MIN_VERSION ||
be16_to_cpu(vhdr->version) > HFSPLUS_CURRENT_VERSION) {
pr_err("wrong filesystem version\n");
- goto out_free_vhdr;
+ goto out_unload_nls;
}
sbi->total_blocks = be32_to_cpu(vhdr->total_blocks);
sbi->free_blocks = be32_to_cpu(vhdr->free_blocks);
@@ -495,7 +493,7 @@ static int hfsplus_fill_super(struct super_block *sb, struct fs_context *fc)
if ((last_fs_block > (sector_t)(~0ULL) >> (sbi->alloc_blksz_shift - 9)) ||
(last_fs_page > (pgoff_t)(~0ULL))) {
pr_err("filesystem size too large\n");
- goto out_free_vhdr;
+ goto out_unload_nls;
}
/* Set up operations so we can load metadata */
@@ -522,7 +520,7 @@ static int hfsplus_fill_super(struct super_block *sb, struct fs_context *fc)
sbi->ext_tree = hfs_btree_open(sb, HFSPLUS_EXT_CNID);
if (!sbi->ext_tree) {
pr_err("failed to load extents file\n");
- goto out_free_vhdr;
+ goto out_unload_nls;
}
sbi->cat_tree = hfs_btree_open(sb, HFSPLUS_CAT_CNID);
if (!sbi->cat_tree) {
@@ -648,9 +646,6 @@ static int hfsplus_fill_super(struct super_block *sb, struct fs_context *fc)
hfs_btree_close(sbi->cat_tree);
out_close_ext_tree:
hfs_btree_close(sbi->ext_tree);
-out_free_vhdr:
- kfree(sbi->s_vhdr_buf);
- kfree(sbi->s_backup_vhdr_buf);
out_unload_nls:
unload_nls(nls);
return err;
@@ -716,6 +711,8 @@ static void hfsplus_kill_super(struct super_block *sb)
struct hfsplus_sb_info *sbi = HFSPLUS_SB(sb);
kill_block_super(sb);
+ kfree(sbi->s_vhdr_buf);
+ kfree(sbi->s_backup_vhdr_buf);
call_rcu(&sbi->rcu, delayed_free);
}
diff --git a/fs/hfsplus/wrapper.c b/fs/hfsplus/wrapper.c
index 30cf4fe78b3d..8e4dcc83af30 100644
--- a/fs/hfsplus/wrapper.c
+++ b/fs/hfsplus/wrapper.c
@@ -139,32 +139,29 @@ int hfsplus_read_wrapper(struct super_block *sb)
u32 blocksize;
int error = 0;
- error = -EINVAL;
blocksize = sb_min_blocksize(sb, HFSPLUS_SECTOR_SIZE);
if (!blocksize)
- goto out;
+ return -EINVAL;
sbi->min_io_size = blocksize;
if (hfsplus_get_last_session(sb, &part_start, &part_size))
- goto out;
+ return -EINVAL;
- error = -ENOMEM;
sbi->s_vhdr_buf = kmalloc(hfsplus_min_io_size(sb), GFP_KERNEL);
if (!sbi->s_vhdr_buf)
- goto out;
+ return -ENOMEM;
sbi->s_backup_vhdr_buf = kmalloc(hfsplus_min_io_size(sb), GFP_KERNEL);
if (!sbi->s_backup_vhdr_buf)
- goto out_free_vhdr;
+ return -ENOMEM;
reread:
error = hfsplus_submit_bio(sb, part_start + HFSPLUS_VOLHEAD_SECTOR,
sbi->s_vhdr_buf, (void **)&sbi->s_vhdr,
REQ_OP_READ);
if (error)
- goto out_free_backup_vhdr;
+ return error;
- error = -EINVAL;
switch (sbi->s_vhdr->signature) {
case cpu_to_be16(HFSPLUS_VOLHEAD_SIGX):
set_bit(HFSPLUS_SB_HFSX, &sbi->flags);
@@ -194,12 +191,11 @@ int hfsplus_read_wrapper(struct super_block *sb)
sbi->s_backup_vhdr_buf,
(void **)&sbi->s_backup_vhdr, REQ_OP_READ);
if (error)
- goto out_free_backup_vhdr;
+ return error;
- error = -EINVAL;
if (sbi->s_backup_vhdr->signature != sbi->s_vhdr->signature) {
pr_warn("invalid secondary volume header\n");
- goto out_free_backup_vhdr;
+ return -EINVAL;
}
blocksize = be32_to_cpu(sbi->s_vhdr->blocksize);
@@ -221,7 +217,7 @@ int hfsplus_read_wrapper(struct super_block *sb)
if (sb_set_blocksize(sb, blocksize) != blocksize) {
pr_err("unable to set blocksize to %u!\n", blocksize);
- goto out_free_backup_vhdr;
+ return -EINVAL;
}
sbi->blockoffset =
@@ -230,11 +226,4 @@ int hfsplus_read_wrapper(struct super_block *sb)
sbi->sect_count = part_size;
sbi->fs_shift = sbi->alloc_blksz_shift - sb->s_blocksize_bits;
return 0;
-
-out_free_backup_vhdr:
- kfree(sbi->s_backup_vhdr_buf);
-out_free_vhdr:
- kfree(sbi->s_vhdr_buf);
-out:
- return error;
}
Powered by blists - more mailing lists