[PATCH] hfsplus: fix ignored error return in hfsplus_delete_cat

Deepanshu Kartikey posted 1 patch 2 months ago
There is a newer version of this series
fs/hfsplus/catalog.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
[PATCH] hfsplus: fix ignored error return in hfsplus_delete_cat
Posted by Deepanshu Kartikey 2 months ago
hfsplus_delete_cat() calls hfsplus_delete_all_attrs() to remove all
extended attributes associated with a catalog entry, but silently
discards its return value. When the xattr deletion fails due to
filesystem corruption (as in a crafted image reported by syzbot),
hfsplus_delete_cat() returns 0 (success) to hfsplus_unlink().

hfsplus_unlink() then proceeds to call hfsplus_cat_write_inode() on
the inode in a corrupt, half-deleted state. Inside that function, if
HFSPLUS_IS_RSRC() is true but rsrc_inode was never properly set,
main_inode is assigned NULL. The subsequent dereference of
main_inode->i_nlink triggers a general protection fault, caught by
KASAN as a null-ptr-deref at offset 0x28.

Fix this by capturing the return value of hfsplus_delete_all_attrs()
and propagating genuine errors back to the caller. -ENOENT is
excluded since it signals normal loop termination (no more xattrs
left to delete) and is not an error condition.

Fixes: 324ef39a8a4f ("hfsplus: add support of manipulation by attributes file")
Reported-by: syzbot+c0ba772a362e70937dfb@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=c0ba772a362e70937dfb
Tested-by: syzbot+c0ba772a362e70937dfb@syzkaller.appspotmail.com
Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
---
 fs/hfsplus/catalog.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/fs/hfsplus/catalog.c b/fs/hfsplus/catalog.c
index 02c1eee4a4b8..adbaeabc06ab 100644
--- a/fs/hfsplus/catalog.c
+++ b/fs/hfsplus/catalog.c
@@ -421,8 +421,15 @@ int hfsplus_delete_cat(u32 cnid, struct inode *dir, const struct qstr *str)
 	hfsplus_mark_inode_dirty(dir, HFSPLUS_I_CAT_DIRTY);
 
 	if (type == HFSPLUS_FILE || type == HFSPLUS_FOLDER) {
-		if (HFSPLUS_SB(sb)->attr_tree)
-			hfsplus_delete_all_attrs(dir, cnid);
+		if (HFSPLUS_SB(sb)->attr_tree) {
+			int attr_err = hfsplus_delete_all_attrs(dir, cnid);
+
+			if (attr_err && attr_err != -ENOENT) {
+				pr_err("hfsplus: failed to delete xattrs for cnid %u: %d\n",
+				       cnid, attr_err);
+				err = attr_err;
+			}
+		}
 	}
 
 out:
-- 
2.43.0
Re: [PATCH] hfsplus: fix ignored error return in hfsplus_delete_cat
Posted by Deepanshu Kartikey 2 months ago
On Tue, Apr 14, 2026 at 6:43 AM Deepanshu Kartikey
<kartikey406@gmail.com> wrote:
>
> hfsplus_delete_cat() calls hfsplus_delete_all_attrs() to remove all
> extended attributes associated with a catalog entry, but silently
> discards its return value. When the xattr deletion fails due to
> filesystem corruption (as in a crafted image reported by syzbot),
> hfsplus_delete_cat() returns 0 (success) to hfsplus_unlink().
>
> hfsplus_unlink() then proceeds to call hfsplus_cat_write_inode() on
> the inode in a corrupt, half-deleted state. Inside that function, if
> HFSPLUS_IS_RSRC() is true but rsrc_inode was never properly set,
> main_inode is assigned NULL. The subsequent dereference of
> main_inode->i_nlink triggers a general protection fault, caught by
> KASAN as a null-ptr-deref at offset 0x28.
>
> Fix this by capturing the return value of hfsplus_delete_all_attrs()
> and propagating genuine errors back to the caller. -ENOENT is
> excluded since it signals normal loop termination (no more xattrs
> left to delete) and is not an error condition.
>
> Fixes: 324ef39a8a4f ("hfsplus: add support of manipulation by attributes file")
> Reported-by: syzbot+c0ba772a362e70937dfb@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=c0ba772a362e70937dfb
> Tested-by: syzbot+c0ba772a362e70937dfb@syzkaller.appspotmail.com
> Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
> ---
>  fs/hfsplus/catalog.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/fs/hfsplus/catalog.c b/fs/hfsplus/catalog.c
> index 02c1eee4a4b8..adbaeabc06ab 100644
> --- a/fs/hfsplus/catalog.c
> +++ b/fs/hfsplus/catalog.c
> @@ -421,8 +421,15 @@ int hfsplus_delete_cat(u32 cnid, struct inode *dir, const struct qstr *str)
>         hfsplus_mark_inode_dirty(dir, HFSPLUS_I_CAT_DIRTY);
>
>         if (type == HFSPLUS_FILE || type == HFSPLUS_FOLDER) {
> -               if (HFSPLUS_SB(sb)->attr_tree)
> -                       hfsplus_delete_all_attrs(dir, cnid);
> +               if (HFSPLUS_SB(sb)->attr_tree) {
> +                       int attr_err = hfsplus_delete_all_attrs(dir, cnid);
> +
> +                       if (attr_err && attr_err != -ENOENT) {
> +                               pr_err("hfsplus: failed to delete xattrs for cnid %u: %d\n",
> +                                      cnid, attr_err);
> +                               err = attr_err;
> +                       }
> +               }
>         }
>
>  out:
> --
> 2.43.0
>

Please disregard this patch and i am sending another patch with more
correct commit message

Thanks