[PATCH] hfs/hfsplus: fix slab-out-of-bounds in hfs_bnode_read_key

[PATCH] hfs/hfsplus: fix slab-out-of-bounds in hfs_bnode_read_key

[Vasiliy Kovalev]

Syzbot reported an issue in hfs subsystem:

BUG: KASAN: slab-out-of-bounds in memcpy_from_page include/linux/highmem.h:423 [inline]
BUG: KASAN: slab-out-of-bounds in hfs_bnode_read fs/hfs/bnode.c:35 [inline]
BUG: KASAN: slab-out-of-bounds in hfs_bnode_read_key+0x314/0x450 fs/hfs/bnode.c:70
Write of size 94 at addr ffff8880123cd100 by task syz-executor237/5102

Call Trace:
 <TASK>
 __dump_stack lib/dump_stack.c:94 [inline]
 dump_stack_lvl+0x241/0x360 lib/dump_stack.c:120
 print_address_description mm/kasan/report.c:377 [inline]
 print_report+0x169/0x550 mm/kasan/report.c:488
 kasan_report+0x143/0x180 mm/kasan/report.c:601
 kasan_check_range+0x282/0x290 mm/kasan/generic.c:189
 __asan_memcpy+0x40/0x70 mm/kasan/shadow.c:106
 memcpy_from_page include/linux/highmem.h:423 [inline]
 hfs_bnode_read fs/hfs/bnode.c:35 [inline]
 hfs_bnode_read_key+0x314/0x450 fs/hfs/bnode.c:70
 hfs_brec_insert+0x7f3/0xbd0 fs/hfs/brec.c:159
 hfs_cat_create+0x41d/0xa50 fs/hfs/catalog.c:118
 hfs_mkdir+0x6c/0xe0 fs/hfs/dir.c:232
 vfs_mkdir+0x2f9/0x4f0 fs/namei.c:4257
 do_mkdirat+0x264/0x3a0 fs/namei.c:4280
 __do_sys_mkdir fs/namei.c:4300 [inline]
 __se_sys_mkdir fs/namei.c:4298 [inline]
 __x64_sys_mkdir+0x6c/0x80 fs/namei.c:4298
 do_syscall_x64 arch/x86/entry/common.c:52 [inline]
 do_syscall_64+0xf3/0x230 arch/x86/entry/common.c:83
 entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7fbdd6057a99

Add a check for key length in hfs_bnode_read_key to prevent
out-of-bounds memory access. If the key length is invalid, the
key buffer is cleared, improving stability and reliability.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Reported-by: syzbot+5f3a973ed3dfb85a6683@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=5f3a973ed3dfb85a6683
Cc: stable@vger.kernel.org
Signed-off-by: Vasiliy Kovalev <kovalev@altlinux.org>
---
 fs/hfs/bnode.c     | 6 ++++++
 fs/hfsplus/bnode.c | 6 ++++++
 2 files changed, 12 insertions(+)

diff --git a/fs/hfs/bnode.c b/fs/hfs/bnode.c
index 6add6ebfef8967..cb823a8a6ba960 100644
--- a/fs/hfs/bnode.c
+++ b/fs/hfs/bnode.c
@@ -67,6 +67,12 @@ void hfs_bnode_read_key(struct hfs_bnode *node, void *key, int off)
 	else
 		key_len = tree->max_key_len + 1;
 
+	if (key_len > sizeof(hfs_btree_key) || key_len < 1) {
+		memset(key, 0, sizeof(hfs_btree_key));
+		pr_err("hfs: Invalid key length: %d\n", key_len);
+		return;
+	}
+
 	hfs_bnode_read(node, key, off, key_len);
 }
 
diff --git a/fs/hfsplus/bnode.c b/fs/hfsplus/bnode.c
index 87974d5e679156..079ea80534f7de 100644
--- a/fs/hfsplus/bnode.c
+++ b/fs/hfsplus/bnode.c
@@ -67,6 +67,12 @@ void hfs_bnode_read_key(struct hfs_bnode *node, void *key, int off)
 	else
 		key_len = tree->max_key_len + 2;
 
+	if (key_len > sizeof(hfsplus_btree_key) || key_len < 1) {
+		memset(key, 0, sizeof(hfsplus_btree_key));
+		pr_err("hfsplus: Invalid key length: %d\n", key_len);
+		return;
+	}
+
 	hfs_bnode_read(node, key, off, key_len);
 }
 
-- 
2.33.8

Re: [PATCH] hfs/hfsplus: fix slab-out-of-bounds in hfs_bnode_read_key

[Salvatore Bonaccorso]

Hi

On Sat, Oct 19, 2024 at 10:13:03PM +0300, Vasiliy Kovalev wrote:
> Syzbot reported an issue in hfs subsystem:
> 
> BUG: KASAN: slab-out-of-bounds in memcpy_from_page include/linux/highmem.h:423 [inline]
> BUG: KASAN: slab-out-of-bounds in hfs_bnode_read fs/hfs/bnode.c:35 [inline]
> BUG: KASAN: slab-out-of-bounds in hfs_bnode_read_key+0x314/0x450 fs/hfs/bnode.c:70
> Write of size 94 at addr ffff8880123cd100 by task syz-executor237/5102
> 
> Call Trace:
>  <TASK>
>  __dump_stack lib/dump_stack.c:94 [inline]
>  dump_stack_lvl+0x241/0x360 lib/dump_stack.c:120
>  print_address_description mm/kasan/report.c:377 [inline]
>  print_report+0x169/0x550 mm/kasan/report.c:488
>  kasan_report+0x143/0x180 mm/kasan/report.c:601
>  kasan_check_range+0x282/0x290 mm/kasan/generic.c:189
>  __asan_memcpy+0x40/0x70 mm/kasan/shadow.c:106
>  memcpy_from_page include/linux/highmem.h:423 [inline]
>  hfs_bnode_read fs/hfs/bnode.c:35 [inline]
>  hfs_bnode_read_key+0x314/0x450 fs/hfs/bnode.c:70
>  hfs_brec_insert+0x7f3/0xbd0 fs/hfs/brec.c:159
>  hfs_cat_create+0x41d/0xa50 fs/hfs/catalog.c:118
>  hfs_mkdir+0x6c/0xe0 fs/hfs/dir.c:232
>  vfs_mkdir+0x2f9/0x4f0 fs/namei.c:4257
>  do_mkdirat+0x264/0x3a0 fs/namei.c:4280
>  __do_sys_mkdir fs/namei.c:4300 [inline]
>  __se_sys_mkdir fs/namei.c:4298 [inline]
>  __x64_sys_mkdir+0x6c/0x80 fs/namei.c:4298
>  do_syscall_x64 arch/x86/entry/common.c:52 [inline]
>  do_syscall_64+0xf3/0x230 arch/x86/entry/common.c:83
>  entry_SYSCALL_64_after_hwframe+0x77/0x7f
> RIP: 0033:0x7fbdd6057a99
> 
> Add a check for key length in hfs_bnode_read_key to prevent
> out-of-bounds memory access. If the key length is invalid, the
> key buffer is cleared, improving stability and reliability.
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Reported-by: syzbot+5f3a973ed3dfb85a6683@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=5f3a973ed3dfb85a6683
> Cc: stable@vger.kernel.org
> Signed-off-by: Vasiliy Kovalev <kovalev@altlinux.org>
> ---
>  fs/hfs/bnode.c     | 6 ++++++
>  fs/hfsplus/bnode.c | 6 ++++++
>  2 files changed, 12 insertions(+)
> 
> diff --git a/fs/hfs/bnode.c b/fs/hfs/bnode.c
> index 6add6ebfef8967..cb823a8a6ba960 100644
> --- a/fs/hfs/bnode.c
> +++ b/fs/hfs/bnode.c
> @@ -67,6 +67,12 @@ void hfs_bnode_read_key(struct hfs_bnode *node, void *key, int off)
>  	else
>  		key_len = tree->max_key_len + 1;
>  
> +	if (key_len > sizeof(hfs_btree_key) || key_len < 1) {
> +		memset(key, 0, sizeof(hfs_btree_key));
> +		pr_err("hfs: Invalid key length: %d\n", key_len);
> +		return;
> +	}
> +
>  	hfs_bnode_read(node, key, off, key_len);
>  }
>  
> diff --git a/fs/hfsplus/bnode.c b/fs/hfsplus/bnode.c
> index 87974d5e679156..079ea80534f7de 100644
> --- a/fs/hfsplus/bnode.c
> +++ b/fs/hfsplus/bnode.c
> @@ -67,6 +67,12 @@ void hfs_bnode_read_key(struct hfs_bnode *node, void *key, int off)
>  	else
>  		key_len = tree->max_key_len + 2;
>  
> +	if (key_len > sizeof(hfsplus_btree_key) || key_len < 1) {
> +		memset(key, 0, sizeof(hfsplus_btree_key));
> +		pr_err("hfsplus: Invalid key length: %d\n", key_len);
> +		return;
> +	}
> +
>  	hfs_bnode_read(node, key, off, key_len);
>  }
>  
> -- 
> 2.33.8

I do realize that the HFS filesystem is "Orphan". But in the light of
the disclosure in
https://ssd-disclosure.com/ssd-advisory-linux-kernel-hfsplus-slab-out-of-bounds-write/
is there still something which needs to be done here?

Does the above needs to be picked in mainline and then propagated to
the supported stable versions?

Regards,
Salvatore

Re: [PATCH] hfs/hfsplus: fix slab-out-of-bounds in hfs_bnode_read_key

[Cengiz Can]

On 20-03-25 20:30:15, Salvatore Bonaccorso wrote:
> Hi
> 

Hello Salvatore,

> On Sat, Oct 19, 2024 at 10:13:03PM +0300, Vasiliy Kovalev wrote:
> > Syzbot reported an issue in hfs subsystem:
> > 
> > BUG: KASAN: slab-out-of-bounds in memcpy_from_page include/linux/highmem.h:423 [inline]
> > BUG: KASAN: slab-out-of-bounds in hfs_bnode_read fs/hfs/bnode.c:35 [inline]
> > BUG: KASAN: slab-out-of-bounds in hfs_bnode_read_key+0x314/0x450 fs/hfs/bnode.c:70
> > Write of size 94 at addr ffff8880123cd100 by task syz-executor237/5102
> > 
> > Call Trace:
> >  <TASK>
> >  __dump_stack lib/dump_stack.c:94 [inline]
> >  dump_stack_lvl+0x241/0x360 lib/dump_stack.c:120
> >  print_address_description mm/kasan/report.c:377 [inline]
> >  print_report+0x169/0x550 mm/kasan/report.c:488
> >  kasan_report+0x143/0x180 mm/kasan/report.c:601
> >  kasan_check_range+0x282/0x290 mm/kasan/generic.c:189
> >  __asan_memcpy+0x40/0x70 mm/kasan/shadow.c:106
> >  memcpy_from_page include/linux/highmem.h:423 [inline]
> >  hfs_bnode_read fs/hfs/bnode.c:35 [inline]
> >  hfs_bnode_read_key+0x314/0x450 fs/hfs/bnode.c:70
> >  hfs_brec_insert+0x7f3/0xbd0 fs/hfs/brec.c:159
> >  hfs_cat_create+0x41d/0xa50 fs/hfs/catalog.c:118
> >  hfs_mkdir+0x6c/0xe0 fs/hfs/dir.c:232
> >  vfs_mkdir+0x2f9/0x4f0 fs/namei.c:4257
> >  do_mkdirat+0x264/0x3a0 fs/namei.c:4280
> >  __do_sys_mkdir fs/namei.c:4300 [inline]
> >  __se_sys_mkdir fs/namei.c:4298 [inline]
> >  __x64_sys_mkdir+0x6c/0x80 fs/namei.c:4298
> >  do_syscall_x64 arch/x86/entry/common.c:52 [inline]
> >  do_syscall_64+0xf3/0x230 arch/x86/entry/common.c:83
> >  entry_SYSCALL_64_after_hwframe+0x77/0x7f
> > RIP: 0033:0x7fbdd6057a99
> > 
> > Add a check for key length in hfs_bnode_read_key to prevent
> > out-of-bounds memory access. If the key length is invalid, the
> > key buffer is cleared, improving stability and reliability.
> > 
> > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> > Reported-by: syzbot+5f3a973ed3dfb85a6683@syzkaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=5f3a973ed3dfb85a6683
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Vasiliy Kovalev <kovalev@altlinux.org>
> > ---
> >  fs/hfs/bnode.c     | 6 ++++++
> >  fs/hfsplus/bnode.c | 6 ++++++
> >  2 files changed, 12 insertions(+)
> > 
> > diff --git a/fs/hfs/bnode.c b/fs/hfs/bnode.c
> > index 6add6ebfef8967..cb823a8a6ba960 100644
> > --- a/fs/hfs/bnode.c
> > +++ b/fs/hfs/bnode.c
> > @@ -67,6 +67,12 @@ void hfs_bnode_read_key(struct hfs_bnode *node, void *key, int off)
> >  	else
> >  		key_len = tree->max_key_len + 1;
> >  
> > +	if (key_len > sizeof(hfs_btree_key) || key_len < 1) {
> > +		memset(key, 0, sizeof(hfs_btree_key));
> > +		pr_err("hfs: Invalid key length: %d\n", key_len);
> > +		return;
> > +	}
> > +
> >  	hfs_bnode_read(node, key, off, key_len);
> >  }

Simpler the better. 

Our fix was released back in February. (There are other issues in our attempt I
admit).

https://git.launchpad.net/~ubuntu-kernel/ubuntu/+source/linux/+git/jammy/commit/?id=2e8d8dffa2e0b5291522548309ec70428be7cf5a

If someone can pick this submission, I will be happy to replace our version.

> >  
> > diff --git a/fs/hfsplus/bnode.c b/fs/hfsplus/bnode.c
> > index 87974d5e679156..079ea80534f7de 100644
> > --- a/fs/hfsplus/bnode.c
> > +++ b/fs/hfsplus/bnode.c
> > @@ -67,6 +67,12 @@ void hfs_bnode_read_key(struct hfs_bnode *node, void *key, int off)
> >  	else
> >  		key_len = tree->max_key_len + 2;
> >  
> > +	if (key_len > sizeof(hfsplus_btree_key) || key_len < 1) {
> > +		memset(key, 0, sizeof(hfsplus_btree_key));
> > +		pr_err("hfsplus: Invalid key length: %d\n", key_len);
> > +		return;
> > +	}
> > +
> >  	hfs_bnode_read(node, key, off, key_len);
> >  }
> >  
> > -- 
> > 2.33.8

Reviewed-by: Cengiz Can <cengiz.can@canonical.com>

> 
> I do realize that the HFS filesystem is "Orphan". But in the light of
> the disclosure in
> https://ssd-disclosure.com/ssd-advisory-linux-kernel-hfsplus-slab-out-of-bounds-write/
> is there still something which needs to be done here?
> 
> Does the above needs to be picked in mainline and then propagated to
> the supported stable versions?
> 
> Regards,
> Salvatore
> 

Re: [PATCH] hfs/hfsplus: fix slab-out-of-bounds in hfs_bnode_read_key

[Greg KH]

On Mon, Mar 24, 2025 at 07:14:07PM +0300, Cengiz Can wrote:
> On 20-03-25 20:30:15, Salvatore Bonaccorso wrote:
> > Hi
> > 
> 
> Hello Salvatore,
> 
> > On Sat, Oct 19, 2024 at 10:13:03PM +0300, Vasiliy Kovalev wrote:
> > > Syzbot reported an issue in hfs subsystem:
> > > 
> > > BUG: KASAN: slab-out-of-bounds in memcpy_from_page include/linux/highmem.h:423 [inline]
> > > BUG: KASAN: slab-out-of-bounds in hfs_bnode_read fs/hfs/bnode.c:35 [inline]
> > > BUG: KASAN: slab-out-of-bounds in hfs_bnode_read_key+0x314/0x450 fs/hfs/bnode.c:70
> > > Write of size 94 at addr ffff8880123cd100 by task syz-executor237/5102
> > > 
> > > Call Trace:
> > >  <TASK>
> > >  __dump_stack lib/dump_stack.c:94 [inline]
> > >  dump_stack_lvl+0x241/0x360 lib/dump_stack.c:120
> > >  print_address_description mm/kasan/report.c:377 [inline]
> > >  print_report+0x169/0x550 mm/kasan/report.c:488
> > >  kasan_report+0x143/0x180 mm/kasan/report.c:601
> > >  kasan_check_range+0x282/0x290 mm/kasan/generic.c:189
> > >  __asan_memcpy+0x40/0x70 mm/kasan/shadow.c:106
> > >  memcpy_from_page include/linux/highmem.h:423 [inline]
> > >  hfs_bnode_read fs/hfs/bnode.c:35 [inline]
> > >  hfs_bnode_read_key+0x314/0x450 fs/hfs/bnode.c:70
> > >  hfs_brec_insert+0x7f3/0xbd0 fs/hfs/brec.c:159
> > >  hfs_cat_create+0x41d/0xa50 fs/hfs/catalog.c:118
> > >  hfs_mkdir+0x6c/0xe0 fs/hfs/dir.c:232
> > >  vfs_mkdir+0x2f9/0x4f0 fs/namei.c:4257
> > >  do_mkdirat+0x264/0x3a0 fs/namei.c:4280
> > >  __do_sys_mkdir fs/namei.c:4300 [inline]
> > >  __se_sys_mkdir fs/namei.c:4298 [inline]
> > >  __x64_sys_mkdir+0x6c/0x80 fs/namei.c:4298
> > >  do_syscall_x64 arch/x86/entry/common.c:52 [inline]
> > >  do_syscall_64+0xf3/0x230 arch/x86/entry/common.c:83
> > >  entry_SYSCALL_64_after_hwframe+0x77/0x7f
> > > RIP: 0033:0x7fbdd6057a99
> > > 
> > > Add a check for key length in hfs_bnode_read_key to prevent
> > > out-of-bounds memory access. If the key length is invalid, the
> > > key buffer is cleared, improving stability and reliability.
> > > 
> > > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> > > Reported-by: syzbot+5f3a973ed3dfb85a6683@syzkaller.appspotmail.com
> > > Closes: https://syzkaller.appspot.com/bug?extid=5f3a973ed3dfb85a6683
> > > Cc: stable@vger.kernel.org
> > > Signed-off-by: Vasiliy Kovalev <kovalev@altlinux.org>
> > > ---
> > >  fs/hfs/bnode.c     | 6 ++++++
> > >  fs/hfsplus/bnode.c | 6 ++++++
> > >  2 files changed, 12 insertions(+)
> > > 
> > > diff --git a/fs/hfs/bnode.c b/fs/hfs/bnode.c
> > > index 6add6ebfef8967..cb823a8a6ba960 100644
> > > --- a/fs/hfs/bnode.c
> > > +++ b/fs/hfs/bnode.c
> > > @@ -67,6 +67,12 @@ void hfs_bnode_read_key(struct hfs_bnode *node, void *key, int off)
> > >  	else
> > >  		key_len = tree->max_key_len + 1;
> > >  
> > > +	if (key_len > sizeof(hfs_btree_key) || key_len < 1) {
> > > +		memset(key, 0, sizeof(hfs_btree_key));
> > > +		pr_err("hfs: Invalid key length: %d\n", key_len);
> > > +		return;
> > > +	}
> > > +
> > >  	hfs_bnode_read(node, key, off, key_len);
> > >  }
> 
> Simpler the better. 
> 
> Our fix was released back in February. (There are other issues in our attempt I
> admit).
> 
> https://git.launchpad.net/~ubuntu-kernel/ubuntu/+source/linux/+git/jammy/commit/?id=2e8d8dffa2e0b5291522548309ec70428be7cf5a
> 
> If someone can pick this submission, I will be happy to replace our version.

any specific reason why you didn't submit this upstream?  Or did that
happen and it somehow not get picked up?

And why assign a CVE for an issue that is in the mainline kernel, last I
checked, Canonical was NOT allowed to do that.

Please work to revoke that CVE and ask for one properly.

thanks,

greg k-h

Re: [PATCH] hfs/hfsplus: fix slab-out-of-bounds in hfs_bnode_read_key

[Cengiz Can]

On 24-03-25 09:17:05, Greg KH wrote:
> On Mon, Mar 24, 2025 at 07:14:07PM +0300, Cengiz Can wrote:
> > On 20-03-25 20:30:15, Salvatore Bonaccorso wrote:
> > > Hi
> > > 
> > 
> > Hello Salvatore,
> > 
> > > On Sat, Oct 19, 2024 at 10:13:03PM +0300, Vasiliy Kovalev wrote:
> > > > Syzbot reported an issue in hfs subsystem:
> > > > 
> > > > BUG: KASAN: slab-out-of-bounds in memcpy_from_page include/linux/highmem.h:423 [inline]
> > > > BUG: KASAN: slab-out-of-bounds in hfs_bnode_read fs/hfs/bnode.c:35 [inline]
> > > > BUG: KASAN: slab-out-of-bounds in hfs_bnode_read_key+0x314/0x450 fs/hfs/bnode.c:70
> > > > Write of size 94 at addr ffff8880123cd100 by task syz-executor237/5102
> > > > 
> > > > Call Trace:
> > > >  <TASK>
> > > >  __dump_stack lib/dump_stack.c:94 [inline]
> > > >  dump_stack_lvl+0x241/0x360 lib/dump_stack.c:120
> > > >  print_address_description mm/kasan/report.c:377 [inline]
> > > >  print_report+0x169/0x550 mm/kasan/report.c:488
> > > >  kasan_report+0x143/0x180 mm/kasan/report.c:601
> > > >  kasan_check_range+0x282/0x290 mm/kasan/generic.c:189
> > > >  __asan_memcpy+0x40/0x70 mm/kasan/shadow.c:106
> > > >  memcpy_from_page include/linux/highmem.h:423 [inline]
> > > >  hfs_bnode_read fs/hfs/bnode.c:35 [inline]
> > > >  hfs_bnode_read_key+0x314/0x450 fs/hfs/bnode.c:70
> > > >  hfs_brec_insert+0x7f3/0xbd0 fs/hfs/brec.c:159
> > > >  hfs_cat_create+0x41d/0xa50 fs/hfs/catalog.c:118
> > > >  hfs_mkdir+0x6c/0xe0 fs/hfs/dir.c:232
> > > >  vfs_mkdir+0x2f9/0x4f0 fs/namei.c:4257
> > > >  do_mkdirat+0x264/0x3a0 fs/namei.c:4280
> > > >  __do_sys_mkdir fs/namei.c:4300 [inline]
> > > >  __se_sys_mkdir fs/namei.c:4298 [inline]
> > > >  __x64_sys_mkdir+0x6c/0x80 fs/namei.c:4298
> > > >  do_syscall_x64 arch/x86/entry/common.c:52 [inline]
> > > >  do_syscall_64+0xf3/0x230 arch/x86/entry/common.c:83
> > > >  entry_SYSCALL_64_after_hwframe+0x77/0x7f
> > > > RIP: 0033:0x7fbdd6057a99
> > > > 
> > > > Add a check for key length in hfs_bnode_read_key to prevent
> > > > out-of-bounds memory access. If the key length is invalid, the
> > > > key buffer is cleared, improving stability and reliability.
> > > > 
> > > > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> > > > Reported-by: syzbot+5f3a973ed3dfb85a6683@syzkaller.appspotmail.com
> > > > Closes: https://syzkaller.appspot.com/bug?extid=5f3a973ed3dfb85a6683
> > > > Cc: stable@vger.kernel.org
> > > > Signed-off-by: Vasiliy Kovalev <kovalev@altlinux.org>
> > > > ---
> > > >  fs/hfs/bnode.c     | 6 ++++++
> > > >  fs/hfsplus/bnode.c | 6 ++++++
> > > >  2 files changed, 12 insertions(+)
> > > > 
> > > > diff --git a/fs/hfs/bnode.c b/fs/hfs/bnode.c
> > > > index 6add6ebfef8967..cb823a8a6ba960 100644
> > > > --- a/fs/hfs/bnode.c
> > > > +++ b/fs/hfs/bnode.c
> > > > @@ -67,6 +67,12 @@ void hfs_bnode_read_key(struct hfs_bnode *node, void *key, int off)
> > > >  	else
> > > >  		key_len = tree->max_key_len + 1;
> > > >  
> > > > +	if (key_len > sizeof(hfs_btree_key) || key_len < 1) {
> > > > +		memset(key, 0, sizeof(hfs_btree_key));
> > > > +		pr_err("hfs: Invalid key length: %d\n", key_len);
> > > > +		return;
> > > > +	}
> > > > +
> > > >  	hfs_bnode_read(node, key, off, key_len);
> > > >  }
> > 
> > Simpler the better. 
> > 
> > Our fix was released back in February. (There are other issues in our attempt I
> > admit).
> > 
> > https://git.launchpad.net/~ubuntu-kernel/ubuntu/+source/linux/+git/jammy/commit/?id=2e8d8dffa2e0b5291522548309ec70428be7cf5a
> > 
> > If someone can pick this submission, I will be happy to replace our version.
> 
> any specific reason why you didn't submit this upstream?  Or did that
> happen and it somehow not get picked up?

It was mentioned by the researchers that previous attempts were unanswered. I
didn't question the validity of that statement.

I received excerpts from a private email communication indicating that the
HFSPlus filesystem currently has no maintainers, and that at least one of the
decision-makers does not consider filesystem corruption flaws to be particularly
sensitive.

Re-sharing this publicly on linux-fsdevel probably won't get picked up and would
definitely put Ubuntu users at risk, as we were the only ones shipping the
'enabling' policy lines at org.freedesktop.udisks2.filesystem-mount. 

So I proceeded with downstream fix and we released the fix before announcement
date.

> 
> And why assign a CVE for an issue that is in the mainline kernel, last I
> checked, Canonical was NOT allowed to do that.

This was not ideal, you're right. It should be assigned by kernel.org.

> 
> Please work to revoke that CVE and ask for one properly.

Will do. Thanks.

In the meantime, can we get this fix applied?

> 
> thanks,
> 
> greg k-h

Re: [PATCH] hfs/hfsplus: fix slab-out-of-bounds in hfs_bnode_read_key

[Greg KH]

On Mon, Mar 24, 2025 at 09:43:18PM +0300, Cengiz Can wrote:
> In the meantime, can we get this fix applied?

Please work with the filesystem maintainers to do so.

thanks,

greg k-h

Re: [PATCH] hfs/hfsplus: fix slab-out-of-bounds in hfs_bnode_read_key

[Cengiz Can]

On 24-03-25 11:53:51, Greg KH wrote:
> On Mon, Mar 24, 2025 at 09:43:18PM +0300, Cengiz Can wrote:
> > In the meantime, can we get this fix applied?
> 
> Please work with the filesystem maintainers to do so.

Hello Christian, hello Alexander

Can you help us with this?

Thanks in advance!

> 
> thanks,
> 
> greg k-h

Re: [PATCH] hfs/hfsplus: fix slab-out-of-bounds in hfs_bnode_read_key

[Greg KH]

On Sun, Apr 06, 2025 at 07:07:57PM +0300, Cengiz Can wrote:
> On 24-03-25 11:53:51, Greg KH wrote:
> > On Mon, Mar 24, 2025 at 09:43:18PM +0300, Cengiz Can wrote:
> > > In the meantime, can we get this fix applied?
> > 
> > Please work with the filesystem maintainers to do so.
> 
> Hello Christian, hello Alexander
> 
> Can you help us with this?

What is "this"?  There is no context here at all, you know better!

Please submit "this" properly, like you all well know how to do, in
order to get this resolved as soon as possible as this is considered
an un-fixed CVE that you all are completely responsible for.

greg k-h

Re: [PATCH] hfs/hfsplus: fix slab-out-of-bounds in hfs_bnode_read_key

[Christian Brauner]

On Sun, Apr 06, 2025 at 07:07:57PM +0300, Cengiz Can wrote:
> On 24-03-25 11:53:51, Greg KH wrote:
> > On Mon, Mar 24, 2025 at 09:43:18PM +0300, Cengiz Can wrote:
> > > In the meantime, can we get this fix applied?
> > 
> > Please work with the filesystem maintainers to do so.
> 
> Hello Christian, hello Alexander
> 
> Can you help us with this?
> 
> Thanks in advance!

Filesystem bugs due to corrupt images are not considered a CVE for any
filesystem that is only mountable by CAP_SYS_ADMIN in the initial user
namespace. That includes delegated mounting.

Now, quoting from [1]:

"So, for the record, the Linux kernel in general only allows mounts for
those with CAP_SYS_ADMIN, however, it is true that desktop and even
server environments allow regular non-privileged users to mount and
automount filesystems.

In particular, both the latest Ubuntu Desktop and Server versions come
with default polkit rules that allow users with an active local session
to create loop devices and mount a range of block filesystems commonly
found on USB flash drives with udisks2. Inspecting
/usr/share/polkit-1/actions/org.freedesktop.UDisks2.policy shows:"

So what this saying is:

A distribution is shipping tooling that allows unprivileged users to mount
arbitrary filesystems including hpfsplus. Or to rephrase this: A
distribution is allowing unprivileged users to mount orphaned
filesystems. Congratulations on the brave decision to play Russian
Roulette with a fully-loaded gun.

The VFS doesn't allow mounting arbitrary filesystems by unprivileged
users. Every FS_REQUIRES_DEV filesystem requires global CAP_SYS_ADMIN
privileged at which point you can also do sudo rm -rf --no-preserve-root /
or a million other destructive things.

The blogpost is aware that the VFS maintainers don't accept CVEs like
this. Yet a CVE was still filed against the upstream kernel. IOW,
someone abused the fact that a distro chose to allow mounting arbitrary
filesystems including orphaned ones by unprivileged user as an argument
to gain a kernel CVE.

Revoke that CVE against the upstream kernel. This is a CVE against a
distro. There's zero reason for us to hurry with any fix.

[1]: https://ssd-disclosure.com/ssd-advisory-linux-kernel-hfsplus-slab-out-of-bounds-write/

Re: [PATCH] hfs/hfsplus: fix slab-out-of-bounds in hfs_bnode_read_key

[Christian Brauner]

On Mon, Apr 07, 2025 at 12:59:18PM +0200, Christian Brauner wrote:
> On Sun, Apr 06, 2025 at 07:07:57PM +0300, Cengiz Can wrote:
> > On 24-03-25 11:53:51, Greg KH wrote:
> > > On Mon, Mar 24, 2025 at 09:43:18PM +0300, Cengiz Can wrote:
> > > > In the meantime, can we get this fix applied?
> > > 
> > > Please work with the filesystem maintainers to do so.
> > 
> > Hello Christian, hello Alexander
> > 
> > Can you help us with this?
> > 
> > Thanks in advance!
> 
> Filesystem bugs due to corrupt images are not considered a CVE for any
> filesystem that is only mountable by CAP_SYS_ADMIN in the initial user
> namespace. That includes delegated mounting.
> 
> Now, quoting from [1]:
> 
> "So, for the record, the Linux kernel in general only allows mounts for
> those with CAP_SYS_ADMIN, however, it is true that desktop and even
> server environments allow regular non-privileged users to mount and
> automount filesystems.
> 
> In particular, both the latest Ubuntu Desktop and Server versions come
> with default polkit rules that allow users with an active local session
> to create loop devices and mount a range of block filesystems commonly
> found on USB flash drives with udisks2. Inspecting
> /usr/share/polkit-1/actions/org.freedesktop.UDisks2.policy shows:"
> 
> So what this saying is:
> 
> A distribution is shipping tooling that allows unprivileged users to mount
> arbitrary filesystems including hpfsplus. Or to rephrase this: A
> distribution is allowing unprivileged users to mount orphaned
> filesystems. Congratulations on the brave decision to play Russian
> Roulette with a fully-loaded gun.
> 
> The VFS doesn't allow mounting arbitrary filesystems by unprivileged
> users. Every FS_REQUIRES_DEV filesystem requires global CAP_SYS_ADMIN
> privileged at which point you can also do sudo rm -rf --no-preserve-root /
> or a million other destructive things.
> 
> The blogpost is aware that the VFS maintainers don't accept CVEs like
> this. Yet a CVE was still filed against the upstream kernel. IOW,
> someone abused the fact that a distro chose to allow mounting arbitrary
> filesystems including orphaned ones by unprivileged user as an argument
> to gain a kernel CVE.
> 
> Revoke that CVE against the upstream kernel. This is a CVE against a
> distro. There's zero reason for us to hurry with any fix.

Before that gets misinterpreted: This is not intended to either
implicitly or explicitly imply that patch pickup is dependend on the
revocation of this CVE.

Since this isn't a valid CVE there's no reason to hurry-up merging this
into mainline within the next 24 hours. It'll get there whenever the
next fixes pr is ready.

> 
> [1]: https://ssd-disclosure.com/ssd-advisory-linux-kernel-hfsplus-slab-out-of-bounds-write/

Re: [PATCH] hfs/hfsplus: fix slab-out-of-bounds in hfs_bnode_read_key

[Attila Szasz]

Christian,

It was Greg who moved this CVE under the kernel.org CNA territory:

https://lore.kernel.org/lkml/2025032402-jam-immovable-2d57@gregkh/

This thread was kicked into gear by Salvatore from Debian, who asked 
whether there was a mainline fix. There wasn’t one upstream, I think, 
primarily due to your assessment.

Meanwhile, the distros wanted to protect their users and fix that gaping 
64k heap buffer overflow with a one-liner boundary check. Canonical did. 
I believe they wanted to help Debian do the same.

They assigned a CVE -with the ID you are seeing- for Canonical Ubuntu Linux:

https://github.com/CVEProject/cvelist/commit/a56d5efc25a561c94ccf296fceaab2a01dc4bc01

It seems Debian initially dropped the config option altogether — a 
reasonable decision I personally agree with — but later reverted the 
change after someone from SuSE pointed out that it’s still required for 
PowerPC, PPC64, and apparently some Apple hardware:
https://salsa.debian.org/kernel-team/linux/-/commit/180f39f01cb9175dc77e8a5e27b78b5d1537752e#note_598347

Still, I think the distros were just trying to do their jobs. Something 
that it seems we might both agree on.

On 4/7/25 19:15, Christian Brauner wrote:
> On Mon, Apr 07, 2025 at 12:59:18PM +0200, Christian Brauner wrote:
>> On Sun, Apr 06, 2025 at 07:07:57PM +0300, Cengiz Can wrote:
>>> On 24-03-25 11:53:51, Greg KH wrote:
>>>> On Mon, Mar 24, 2025 at 09:43:18PM +0300, Cengiz Can wrote:
>>>>> In the meantime, can we get this fix applied?
>>>> Please work with the filesystem maintainers to do so.
>>> Hello Christian, hello Alexander
>>>
>>> Can you help us with this?
>>>
>>> Thanks in advance!
>> Filesystem bugs due to corrupt images are not considered a CVE for any
>> filesystem that is only mountable by CAP_SYS_ADMIN in the initial user
>> namespace. That includes delegated mounting.
>>
>> Now, quoting from [1]:
>>
>> "So, for the record, the Linux kernel in general only allows mounts for
>> those with CAP_SYS_ADMIN, however, it is true that desktop and even
>> server environments allow regular non-privileged users to mount and
>> automount filesystems.
>>
>> In particular, both the latest Ubuntu Desktop and Server versions come
>> with default polkit rules that allow users with an active local session
>> to create loop devices and mount a range of block filesystems commonly
>> found on USB flash drives with udisks2. Inspecting
>> /usr/share/polkit-1/actions/org.freedesktop.UDisks2.policy shows:"
>>
>> So what this saying is:
>>
>> A distribution is shipping tooling that allows unprivileged users to mount
>> arbitrary filesystems including hpfsplus. Or to rephrase this: A
>> distribution is allowing unprivileged users to mount orphaned
>> filesystems. Congratulations on the brave decision to play Russian
>> Roulette with a fully-loaded gun.
>>
>> The VFS doesn't allow mounting arbitrary filesystems by unprivileged
>> users. Every FS_REQUIRES_DEV filesystem requires global CAP_SYS_ADMIN
>> privileged at which point you can also do sudo rm -rf --no-preserve-root /
>> or a million other destructive things.
>>
>> The blogpost is aware that the VFS maintainers don't accept CVEs like
>> this. Yet a CVE was still filed against the upstream kernel. IOW,
>> someone abused the fact that a distro chose to allow mounting arbitrary
>> filesystems including orphaned ones by unprivileged user as an argument
>> to gain a kernel CVE.
>>
>> Revoke that CVE against the upstream kernel. This is a CVE against a
>> distro. There's zero reason for us to hurry with any fix.
> Before that gets misinterpreted: This is not intended to either
> implicitly or explicitly imply that patch pickup is dependend on the
> revocation of this CVE.
>
> Since this isn't a valid CVE there's no reason to hurry-up merging this
> into mainline within the next 24 hours. It'll get there whenever the
> next fixes pr is ready.
>
>> [1]: https://ssd-disclosure.com/ssd-advisory-linux-kernel-hfsplus-slab-out-of-bounds-write/

Re: [PATCH] hfs/hfsplus: fix slab-out-of-bounds in hfs_bnode_read_key

[Darrick J. Wong]

On Mon, Apr 07, 2025 at 12:59:18PM +0200, Christian Brauner wrote:
> On Sun, Apr 06, 2025 at 07:07:57PM +0300, Cengiz Can wrote:
> > On 24-03-25 11:53:51, Greg KH wrote:
> > > On Mon, Mar 24, 2025 at 09:43:18PM +0300, Cengiz Can wrote:
> > > > In the meantime, can we get this fix applied?
> > > 
> > > Please work with the filesystem maintainers to do so.
> > 
> > Hello Christian, hello Alexander
> > 
> > Can you help us with this?
> > 
> > Thanks in advance!
> 
> Filesystem bugs due to corrupt images are not considered a CVE for any
> filesystem that is only mountable by CAP_SYS_ADMIN in the initial user
> namespace. That includes delegated mounting.
> 
> Now, quoting from [1]:
> 
> "So, for the record, the Linux kernel in general only allows mounts for
> those with CAP_SYS_ADMIN, however, it is true that desktop and even
> server environments allow regular non-privileged users to mount and
> automount filesystems.
> 
> In particular, both the latest Ubuntu Desktop and Server versions come
> with default polkit rules that allow users with an active local session
> to create loop devices and mount a range of block filesystems commonly
> found on USB flash drives with udisks2. Inspecting
> /usr/share/polkit-1/actions/org.freedesktop.UDisks2.policy shows:"
> 
> So what this saying is:
> 
> A distribution is shipping tooling that allows unprivileged users to mount
> arbitrary filesystems including hpfsplus. Or to rephrase this: A
> distribution is allowing unprivileged users to mount orphaned
> filesystems. Congratulations on the brave decision to play Russian
> Roulette with a fully-loaded gun.

It's also the default policy on Debian 12 and RHEL9 that if you're
logged into the GUI, any program can run:

$ truncate -s 3g /tmp/a
$ mkfs.hfs /tmp/a
$ <write evil stuff on /tmp/a>
$ udisksctl loop-setup -f /tmp/a
$ udisksctl mount -b /dev/loopX

and the user never sees a prompt.  GNOME and KDE both display a
notification when the mount finishes, but by then it could be too late.
Someone should file a CVE against them too.

You can tighten this up by doing this:

# cat > /usr/share/polkit-1/rules.d/always-ask-mount.rules << ENDL
// don't allow mounting, reformatting, or loopdev creation without asking
polkit.addRule(function(action, subject) {
        if ((action.id == "org.freedesktop.udisks2.loop-setup" ||
             action.id == "org.freedesktop.udisks2.filesystem-mount" ||
             action.id == "org.freedesktop.udisks2.modify-device") &&
            subject.local == true) {
                return polkit.Result.AUTH_ADMIN_KEEP;
        }
});
ENDL

so at least you have to authenticate with an admin account.  We do love
our footguns, don't we?  At least it doesn't let you do that if you're
ssh'd in...

--D

> The VFS doesn't allow mounting arbitrary filesystems by unprivileged
> users. Every FS_REQUIRES_DEV filesystem requires global CAP_SYS_ADMIN
> privileged at which point you can also do sudo rm -rf --no-preserve-root /
> or a million other destructive things.
> 
> The blogpost is aware that the VFS maintainers don't accept CVEs like
> this. Yet a CVE was still filed against the upstream kernel. IOW,
> someone abused the fact that a distro chose to allow mounting arbitrary
> filesystems including orphaned ones by unprivileged user as an argument
> to gain a kernel CVE.
> 
> Revoke that CVE against the upstream kernel. This is a CVE against a
> distro. There's zero reason for us to hurry with any fix.
> 
> [1]: https://ssd-disclosure.com/ssd-advisory-linux-kernel-hfsplus-slab-out-of-bounds-write/
> 

Re: [PATCH] hfs/hfsplus: fix slab-out-of-bounds in hfs_bnode_read_key

[Richard Weinberger]

On Mon, Apr 7, 2025 at 9:08 PM Darrick J. Wong <djwong@kernel.org> wrote:
> It's also the default policy on Debian 12 and RHEL9 that if you're
> logged into the GUI, any program can run:
>
> $ truncate -s 3g /tmp/a
> $ mkfs.hfs /tmp/a
> $ <write evil stuff on /tmp/a>
> $ udisksctl loop-setup -f /tmp/a
> $ udisksctl mount -b /dev/loopX
>
> and the user never sees a prompt.  GNOME and KDE both display a
> notification when the mount finishes, but by then it could be too late.
> Someone should file a CVE against them too.

At least on SUSE orphaned and other problematic filesystem kernel modules
are blacklisted. I wonder why other distros didn't follow this approach.

> You can tighten this up by doing this:
>
> # cat > /usr/share/polkit-1/rules.d/always-ask-mount.rules << ENDL
> // don't allow mounting, reformatting, or loopdev creation without asking
> polkit.addRule(function(action, subject) {
>         if ((action.id == "org.freedesktop.udisks2.loop-setup" ||
>              action.id == "org.freedesktop.udisks2.filesystem-mount" ||
>              action.id == "org.freedesktop.udisks2.modify-device") &&
>             subject.local == true) {
>                 return polkit.Result.AUTH_ADMIN_KEEP;
>         }
> });
> ENDL

Thanks for sharing this!

> so at least you have to authenticate with an admin account.  We do love
> our footguns, don't we?  At least it doesn't let you do that if you're
> ssh'd in...

IMHO guestmount and other userspace filesystem implementations should
be the default
for such mounts.

//richard

Re: [PATCH] hfs/hfsplus: fix slab-out-of-bounds in hfs_bnode_read_key

[Darrick J. Wong]

On Tue, Apr 08, 2025 at 12:11:36PM +0200, Richard Weinberger wrote:
> On Mon, Apr 7, 2025 at 9:08 PM Darrick J. Wong <djwong@kernel.org> wrote:
> > It's also the default policy on Debian 12 and RHEL9 that if you're
> > logged into the GUI, any program can run:
> >
> > $ truncate -s 3g /tmp/a
> > $ mkfs.hfs /tmp/a
> > $ <write evil stuff on /tmp/a>
> > $ udisksctl loop-setup -f /tmp/a
> > $ udisksctl mount -b /dev/loopX
> >
> > and the user never sees a prompt.  GNOME and KDE both display a
> > notification when the mount finishes, but by then it could be too late.
> > Someone should file a CVE against them too.
> 
> At least on SUSE orphaned and other problematic filesystem kernel modules
> are blacklisted. I wonder why other distros didn't follow this approach.

Maximal flexibility, I'm assuming.  It's at least somewhat comforting
that RHEL doesn't enable HFS in Kconfig so it's a nonissue for them, but
some day it's going to be ext4/XFS/btrfs that creates a compromise
widget.

> > You can tighten this up by doing this:
> >
> > # cat > /usr/share/polkit-1/rules.d/always-ask-mount.rules << ENDL
> > // don't allow mounting, reformatting, or loopdev creation without asking
> > polkit.addRule(function(action, subject) {
> >         if ((action.id == "org.freedesktop.udisks2.loop-setup" ||
> >              action.id == "org.freedesktop.udisks2.filesystem-mount" ||
> >              action.id == "org.freedesktop.udisks2.modify-device") &&
> >             subject.local == true) {
> >                 return polkit.Result.AUTH_ADMIN_KEEP;
> >         }
> > });
> > ENDL
> 
> Thanks for sharing this!
> 
> > so at least you have to authenticate with an admin account.  We do love
> > our footguns, don't we?  At least it doesn't let you do that if you're
> > ssh'd in...
> 
> IMHO guestmount and other userspace filesystem implementations should
> be the default
> for such mounts.

Agree.  I don't know if they (udisks upstream) have any good way to
detect that a userspace filesystem driver is available for a given
filesystem.  Individual fuse drivers don't seem to have a naming
convention (fusefat, fuse2fs) though at least on Debian some of them
seem to end up as /sbin/mount.fuse.$FSTYPE.

guestmount seems to boot the running kernel in qemu and use that?  So I
guess it's hard for guestmount itself even to tell you what formats it
supports?  I'm probably just ignorant on that issue.

--D

> //richard
> 

Re: [PATCH] hfs/hfsplus: fix slab-out-of-bounds in hfs_bnode_read_key

[Richard Weinberger]

----- Ursprüngliche Mail -----
> Von: "Darrick J. Wong" <djwong@kernel.org>
>> IMHO guestmount and other userspace filesystem implementations should
>> be the default
>> for such mounts.
> 
> Agree.  I don't know if they (udisks upstream) have any good way to
> detect that a userspace filesystem driver is available for a given
> filesystem.  Individual fuse drivers don't seem to have a naming
> convention (fusefat, fuse2fs) though at least on Debian some of them
> seem to end up as /sbin/mount.fuse.$FSTYPE.
> 
> guestmount seems to boot the running kernel in qemu and use that?  So I
> guess it's hard for guestmount itself even to tell you what formats it
> supports?  I'm probably just ignorant on that issue.

Yes, libguestfs runs a Linux kernel inside. It support qemu, UserModeLinux
and IIRC it had even a PoC for LKL (Linux Kernel Library).
That said, the performance is not optimal but I'm sure with reasonable
effort it could be improved at lot.
At least to work with odd filesystem in a satisfying way.

Thanks,
//richard

Re: [PATCH] hfs/hfsplus: fix slab-out-of-bounds in hfs_bnode_read_key

[Eric Sandeen]

On 4/8/25 5:11 AM, Richard Weinberger wrote:
> On Mon, Apr 7, 2025 at 9:08 PM Darrick J. Wong <djwong@kernel.org> wrote:
>> It's also the default policy on Debian 12 and RHEL9 that if you're
>> logged into the GUI, any program can run:
>>
>> $ truncate -s 3g /tmp/a
>> $ mkfs.hfs /tmp/a
>> $ <write evil stuff on /tmp/a>
>> $ udisksctl loop-setup -f /tmp/a
>> $ udisksctl mount -b /dev/loopX
>>
>> and the user never sees a prompt.  GNOME and KDE both display a
>> notification when the mount finishes, but by then it could be too late.
>> Someone should file a CVE against them too.
> 
> At least on SUSE orphaned and other problematic filesystem kernel modules
> are blacklisted. I wonder why other distros didn't follow this approach.

To be clear, RHEL9 ships a very limited set of filesystems, and as a result
does not ship any of these oddball/orphaned filesystems.

While I agree w/ Darrick that the silent automounter is a risk in general,
even for well-maintained filesystems, for distros like RHEL the attack surface
is much more limited because the most problematic filesystems aren't available.

Not saying it solves the problem completely, just saying it's not as egregious
as it might look from the original example.

Thanks,
-Eric

Re: [PATCH] hfs/hfsplus: fix slab-out-of-bounds in hfs_bnode_read_key

[Greg KH]

On Mon, Apr 07, 2025 at 12:59:18PM +0200, Christian Brauner wrote:
> On Sun, Apr 06, 2025 at 07:07:57PM +0300, Cengiz Can wrote:
> > On 24-03-25 11:53:51, Greg KH wrote:
> > > On Mon, Mar 24, 2025 at 09:43:18PM +0300, Cengiz Can wrote:
> > > > In the meantime, can we get this fix applied?
> > > 
> > > Please work with the filesystem maintainers to do so.
> > 
> > Hello Christian, hello Alexander
> > 
> > Can you help us with this?
> > 
> > Thanks in advance!
> 
> Filesystem bugs due to corrupt images are not considered a CVE for any
> filesystem that is only mountable by CAP_SYS_ADMIN in the initial user
> namespace. That includes delegated mounting.

Thank you for the concise summary of this.  We (i.e. the kernel CVE
team) will try to not assign CVEs going forward that can only be
triggered in this way.

> The blogpost is aware that the VFS maintainers don't accept CVEs like
> this. Yet a CVE was still filed against the upstream kernel. IOW,
> someone abused the fact that a distro chose to allow mounting arbitrary
> filesystems including orphaned ones by unprivileged user as an argument
> to gain a kernel CVE.

Yes, Canonical abused their role as a CNA and created this CVE without
going through the proper processes.  kernel.org is now in charge of this
CVE, and:

> Revoke that CVE against the upstream kernel. This is a CVE against a
> distro. There's zero reason for us to hurry with any fix.

I will go reject this now.

Note, there might be some older CVEs that we have accidentally assigned
that can only be triggered by hand-crafted filesystem images.  If anyone
wants to dig through the 5000+ different ones we have, we will be glad
to reject them as well.

thanks,

greg k-h

Re: [PATCH] hfs/hfsplus: fix slab-out-of-bounds in hfs_bnode_read_key

[Attila Szasz]

I’m not sure what you're doing with CVEs at the moment, but it doesn’t 
matter too much for me personally, though it can be a bit worrisome for 
others. Post-CRA, things will likely change again anyway.

Distros should probably consider guestmounting, as Richard suggested, or 
disabling the feature altogether once edge cases are worked out.

https://github.com/torvalds/linux/commit/25efb2ffdf991177e740b2f63e92b4ec7d310a92

Looking at other unpatched syzkaller reports and older commits, it seems 
plausible that the HFS+ driver was—and possibly still is—unstable in how 
it manipulates B-trees. One could potentially mount a valid, empty image 
and cause corruption through normal, unprivileged operations, leading to 
a memory corruption primitive that could root the box.

I’m not working on this right now, but it might be the case that 
syzkaller might uncover stuff similar to whatever was /*formerly 
referred to by*/ CVE-2025-0927. People tend to view these things as 
either abuse or contribution, depending on their sentiment toward 
information security, anyway.

Still, it might be worth keeping in mind for the distros and taking 
lessons from this to improve whatever implicit threat models the 
involved parties are working with.

Thanks for the fix upstream! It was probably a good call after all, all 
things considered.

On 4/8/25 10:03, Greg KH wrote:
> On Mon, Apr 07, 2025 at 12:59:18PM +0200, Christian Brauner wrote:
>> On Sun, Apr 06, 2025 at 07:07:57PM +0300, Cengiz Can wrote:
>>> On 24-03-25 11:53:51, Greg KH wrote:
>>>> On Mon, Mar 24, 2025 at 09:43:18PM +0300, Cengiz Can wrote:
>>>>> In the meantime, can we get this fix applied?
>>>> Please work with the filesystem maintainers to do so.
>>> Hello Christian, hello Alexander
>>>
>>> Can you help us with this?
>>>
>>> Thanks in advance!
>> Filesystem bugs due to corrupt images are not considered a CVE for any
>> filesystem that is only mountable by CAP_SYS_ADMIN in the initial user
>> namespace. That includes delegated mounting.
> Thank you for the concise summary of this.  We (i.e. the kernel CVE
> team) will try to not assign CVEs going forward that can only be
> triggered in this way.
>
>> The blogpost is aware that the VFS maintainers don't accept CVEs like
>> this. Yet a CVE was still filed against the upstream kernel. IOW,
>> someone abused the fact that a distro chose to allow mounting arbitrary
>> filesystems including orphaned ones by unprivileged user as an argument
>> to gain a kernel CVE.
> Yes, Canonical abused their role as a CNA and created this CVE without
> going through the proper processes.  kernel.org is now in charge of this
> CVE, and:
>
>> Revoke that CVE against the upstream kernel. This is a CVE against a
>> distro. There's zero reason for us to hurry with any fix.
> I will go reject this now.
>
> Note, there might be some older CVEs that we have accidentally assigned
> that can only be triggered by hand-crafted filesystem images.  If anyone
> wants to dig through the 5000+ different ones we have, we will be glad
> to reject them as well.
>
> thanks,
>
> greg k-h

Re: [PATCH] hfs/hfsplus: fix slab-out-of-bounds in hfs_bnode_read_key

[Attila Szasz]

hi, Attila here—the one who wrote the article and the PoC.

just for the record: this was a mistake then, and if one happens to 
discover another impactful bug in a potentially orphaned filesystem or 
another subsystem, it might EVEN get prioritized by upstream and stable 
over /panic_on_warn/ stuff next time, right? or am I missing something?

On 3/24/25 17:17, Greg KH wrote:
> On Mon, Mar 24, 2025 at 07:14:07PM +0300, Cengiz Can wrote:
>> On 20-03-25 20:30:15, Salvatore Bonaccorso wrote:
>>> Hi
>>>
>> Hello Salvatore,
>>
>>> On Sat, Oct 19, 2024 at 10:13:03PM +0300, Vasiliy Kovalev wrote:
>>>> Syzbot reported an issue in hfs subsystem:
>>>>
>>>> BUG: KASAN: slab-out-of-bounds in memcpy_from_page include/linux/highmem.h:423 [inline]
>>>> BUG: KASAN: slab-out-of-bounds in hfs_bnode_read fs/hfs/bnode.c:35 [inline]
>>>> BUG: KASAN: slab-out-of-bounds in hfs_bnode_read_key+0x314/0x450 fs/hfs/bnode.c:70
>>>> Write of size 94 at addr ffff8880123cd100 by task syz-executor237/5102
>>>>
>>>> Call Trace:
>>>>   <TASK>
>>>>   __dump_stack lib/dump_stack.c:94 [inline]
>>>>   dump_stack_lvl+0x241/0x360 lib/dump_stack.c:120
>>>>   print_address_description mm/kasan/report.c:377 [inline]
>>>>   print_report+0x169/0x550 mm/kasan/report.c:488
>>>>   kasan_report+0x143/0x180 mm/kasan/report.c:601
>>>>   kasan_check_range+0x282/0x290 mm/kasan/generic.c:189
>>>>   __asan_memcpy+0x40/0x70 mm/kasan/shadow.c:106
>>>>   memcpy_from_page include/linux/highmem.h:423 [inline]
>>>>   hfs_bnode_read fs/hfs/bnode.c:35 [inline]
>>>>   hfs_bnode_read_key+0x314/0x450 fs/hfs/bnode.c:70
>>>>   hfs_brec_insert+0x7f3/0xbd0 fs/hfs/brec.c:159
>>>>   hfs_cat_create+0x41d/0xa50 fs/hfs/catalog.c:118
>>>>   hfs_mkdir+0x6c/0xe0 fs/hfs/dir.c:232
>>>>   vfs_mkdir+0x2f9/0x4f0 fs/namei.c:4257
>>>>   do_mkdirat+0x264/0x3a0 fs/namei.c:4280
>>>>   __do_sys_mkdir fs/namei.c:4300 [inline]
>>>>   __se_sys_mkdir fs/namei.c:4298 [inline]
>>>>   __x64_sys_mkdir+0x6c/0x80 fs/namei.c:4298
>>>>   do_syscall_x64 arch/x86/entry/common.c:52 [inline]
>>>>   do_syscall_64+0xf3/0x230 arch/x86/entry/common.c:83
>>>>   entry_SYSCALL_64_after_hwframe+0x77/0x7f
>>>> RIP: 0033:0x7fbdd6057a99
>>>>
>>>> Add a check for key length in hfs_bnode_read_key to prevent
>>>> out-of-bounds memory access. If the key length is invalid, the
>>>> key buffer is cleared, improving stability and reliability.
>>>>
>>>> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
>>>> Reported-by:syzbot+5f3a973ed3dfb85a6683@syzkaller.appspotmail.com
>>>> Closes:https://syzkaller.appspot.com/bug?extid=5f3a973ed3dfb85a6683
>>>> Cc:stable@vger.kernel.org
>>>> Signed-off-by: Vasiliy Kovalev<kovalev@altlinux.org>
>>>> ---
>>>>   fs/hfs/bnode.c     | 6 ++++++
>>>>   fs/hfsplus/bnode.c | 6 ++++++
>>>>   2 files changed, 12 insertions(+)
>>>>
>>>> diff --git a/fs/hfs/bnode.c b/fs/hfs/bnode.c
>>>> index 6add6ebfef8967..cb823a8a6ba960 100644
>>>> --- a/fs/hfs/bnode.c
>>>> +++ b/fs/hfs/bnode.c
>>>> @@ -67,6 +67,12 @@ void hfs_bnode_read_key(struct hfs_bnode *node, void *key, int off)
>>>>   	else
>>>>   		key_len = tree->max_key_len + 1;
>>>>   
>>>> +	if (key_len > sizeof(hfs_btree_key) || key_len < 1) {
>>>> +		memset(key, 0, sizeof(hfs_btree_key));
>>>> +		pr_err("hfs: Invalid key length: %d\n", key_len);
>>>> +		return;
>>>> +	}
>>>> +
>>>>   	hfs_bnode_read(node, key, off, key_len);
>>>>   }
>> Simpler the better.
>>
>> Our fix was released back in February. (There are other issues in our attempt I
>> admit).
>>
>> https://git.launchpad.net/~ubuntu-kernel/ubuntu/+source/linux/+git/jammy/commit/?id=2e8d8dffa2e0b5291522548309ec70428be7cf5a
>>
>> If someone can pick this submission, I will be happy to replace our version.
> any specific reason why you didn't submit this upstream?  Or did that
> happen and it somehow not get picked up?
>
> And why assign a CVE for an issue that is in the mainline kernel, last I
> checked, Canonical was NOT allowed to do that.
>
> Please work to revoke that CVE and ask for one properly.
>
> thanks,
>
> greg k-h
>

Re: [PATCH] hfs/hfsplus: fix slab-out-of-bounds in hfs_bnode_read_key

[Christian Brauner]

On Sat, 19 Oct 2024 22:13:03 +0300, Vasiliy Kovalev wrote:
> Syzbot reported an issue in hfs subsystem:
> 
> BUG: KASAN: slab-out-of-bounds in memcpy_from_page include/linux/highmem.h:423 [inline]
> BUG: KASAN: slab-out-of-bounds in hfs_bnode_read fs/hfs/bnode.c:35 [inline]
> BUG: KASAN: slab-out-of-bounds in hfs_bnode_read_key+0x314/0x450 fs/hfs/bnode.c:70
> Write of size 94 at addr ffff8880123cd100 by task syz-executor237/5102
> 
> [...]

Stop-gap measure at best. I expect there to be plenty of other ways for
syzbot to trigger issues in hpfsplus.

---

Applied to the vfs.fixes branch of the vfs/vfs.git tree.
Patches in the vfs.fixes branch should appear in linux-next soon.

Please report any outstanding bugs that were missed during review in a
new review to the original patch series allowing us to drop it.

It's encouraged to provide Acked-bys and Reviewed-bys even though the
patch has now been applied. If possible patch trailers will be updated.

Note that commit hashes shown below are subject to change due to rebase,
trailer updates or similar. If in doubt, please check the listed branch.

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
branch: vfs.fixes

[1/1] hfs/hfsplus: fix slab-out-of-bounds in hfs_bnode_read_key
      https://git.kernel.org/vfs/vfs/c/bb5e07cb9277