From: Chao Yu <[email protected]>
In create_unique_id(), kmalloc(, GFP_KERNEL) can fail due to
out-of-memory, if it fails, return errno correctly rather than
triggering panic via BUG_ON();
kernel BUG at mm/slub.c:5893!
Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
Call trace:
sysfs_slab_add+0x258/0x260 mm/slub.c:5973
__kmem_cache_create+0x60/0x118 mm/slub.c:4899
create_cache mm/slab_common.c:229 [inline]
kmem_cache_create_usercopy+0x19c/0x31c mm/slab_common.c:335
kmem_cache_create+0x1c/0x28 mm/slab_common.c:390
f2fs_kmem_cache_create fs/f2fs/f2fs.h:2766 [inline]
f2fs_init_xattr_caches+0x78/0xb4 fs/f2fs/xattr.c:808
f2fs_fill_super+0x1050/0x1e0c fs/f2fs/super.c:4149
mount_bdev+0x1b8/0x210 fs/super.c:1400
f2fs_mount+0x44/0x58 fs/f2fs/super.c:4512
legacy_get_tree+0x30/0x74 fs/fs_context.c:610
vfs_get_tree+0x40/0x140 fs/super.c:1530
do_new_mount+0x1dc/0x4e4 fs/namespace.c:3040
path_mount+0x358/0x914 fs/namespace.c:3370
do_mount fs/namespace.c:3383 [inline]
__do_sys_mount fs/namespace.c:3591 [inline]
__se_sys_mount fs/namespace.c:3568 [inline]
__arm64_sys_mount+0x2f8/0x408 fs/namespace.c:3568
Cc: <[email protected]>
Reported-by: [email protected]
Signed-off-by: Chao Yu <[email protected]>
---
mm/slub.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/mm/slub.c b/mm/slub.c
index 862dbd9af4f5..e6f3727b9ad2 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -5890,7 +5890,8 @@ static char *create_unique_id(struct kmem_cache *s)
char *name = kmalloc(ID_STR_LENGTH, GFP_KERNEL);
char *p = name;
- BUG_ON(!name);
+ if (!name)
+ return ERR_PTR(-ENOMEM);
*p++ = ':';
/*
@@ -5948,6 +5949,8 @@ static int sysfs_slab_add(struct kmem_cache *s)
* for the symlinks.
*/
name = create_unique_id(s);
+ if (IS_ERR(name))
+ return PTR_ERR(name);
}
s->kobj.kset = kset;
--
2.25.1
> On Aug 30, 2022, at 22:10, Chao Yu <[email protected]> wrote:
>
> From: Chao Yu <[email protected]>
>
> In create_unique_id(), kmalloc(, GFP_KERNEL) can fail due to
> out-of-memory, if it fails, return errno correctly rather than
> triggering panic via BUG_ON();
I tend to agree with you. A mount operation shouldn’t panic the
kernel.
>
> kernel BUG at mm/slub.c:5893!
> Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
>
> Call trace:
> sysfs_slab_add+0x258/0x260 mm/slub.c:5973
> __kmem_cache_create+0x60/0x118 mm/slub.c:4899
> create_cache mm/slab_common.c:229 [inline]
> kmem_cache_create_usercopy+0x19c/0x31c mm/slab_common.c:335
> kmem_cache_create+0x1c/0x28 mm/slab_common.c:390
> f2fs_kmem_cache_create fs/f2fs/f2fs.h:2766 [inline]
> f2fs_init_xattr_caches+0x78/0xb4 fs/f2fs/xattr.c:808
> f2fs_fill_super+0x1050/0x1e0c fs/f2fs/super.c:4149
> mount_bdev+0x1b8/0x210 fs/super.c:1400
> f2fs_mount+0x44/0x58 fs/f2fs/super.c:4512
> legacy_get_tree+0x30/0x74 fs/fs_context.c:610
> vfs_get_tree+0x40/0x140 fs/super.c:1530
> do_new_mount+0x1dc/0x4e4 fs/namespace.c:3040
> path_mount+0x358/0x914 fs/namespace.c:3370
> do_mount fs/namespace.c:3383 [inline]
> __do_sys_mount fs/namespace.c:3591 [inline]
> __se_sys_mount fs/namespace.c:3568 [inline]
> __arm64_sys_mount+0x2f8/0x408 fs/namespace.c:3568
>
> Cc: <[email protected]>
> Reported-by: [email protected]
> Signed-off-by: Chao Yu <[email protected]>
Reviewed-by: Muchun Song <[email protected]>
Thanks.
On Tue, Aug 30, 2022 at 10:10:09PM +0800, Chao Yu wrote:
> From: Chao Yu <[email protected]>
>
> In create_unique_id(), kmalloc(, GFP_KERNEL) can fail due to
> out-of-memory, if it fails, return errno correctly rather than
> triggering panic via BUG_ON();
>
> kernel BUG at mm/slub.c:5893!
> Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
>
> Call trace:
> sysfs_slab_add+0x258/0x260 mm/slub.c:5973
> __kmem_cache_create+0x60/0x118 mm/slub.c:4899
> create_cache mm/slab_common.c:229 [inline]
> kmem_cache_create_usercopy+0x19c/0x31c mm/slab_common.c:335
> kmem_cache_create+0x1c/0x28 mm/slab_common.c:390
> f2fs_kmem_cache_create fs/f2fs/f2fs.h:2766 [inline]
> f2fs_init_xattr_caches+0x78/0xb4 fs/f2fs/xattr.c:808
> f2fs_fill_super+0x1050/0x1e0c fs/f2fs/super.c:4149
> mount_bdev+0x1b8/0x210 fs/super.c:1400
> f2fs_mount+0x44/0x58 fs/f2fs/super.c:4512
> legacy_get_tree+0x30/0x74 fs/fs_context.c:610
> vfs_get_tree+0x40/0x140 fs/super.c:1530
> do_new_mount+0x1dc/0x4e4 fs/namespace.c:3040
> path_mount+0x358/0x914 fs/namespace.c:3370
> do_mount fs/namespace.c:3383 [inline]
> __do_sys_mount fs/namespace.c:3591 [inline]
> __se_sys_mount fs/namespace.c:3568 [inline]
> __arm64_sys_mount+0x2f8/0x408 fs/namespace.c:3568
>
> Cc: <[email protected]>
> Reported-by: [email protected]
Fixes: 81819f0fc8285 ("SLUB core")
> Signed-off-by: Chao Yu <[email protected]>
> ---
> mm/slub.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/mm/slub.c b/mm/slub.c
> index 862dbd9af4f5..e6f3727b9ad2 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -5890,7 +5890,8 @@ static char *create_unique_id(struct kmem_cache *s)
> char *name = kmalloc(ID_STR_LENGTH, GFP_KERNEL);
> char *p = name;
>
> - BUG_ON(!name);
> + if (!name)
> + return ERR_PTR(-ENOMEM);
>
> *p++ = ':';
> /*
> @@ -5948,6 +5949,8 @@ static int sysfs_slab_add(struct kmem_cache *s)
> * for the symlinks.
> */
> name = create_unique_id(s);
> + if (IS_ERR(name))
> + return PTR_ERR(name);
> }
>
> s->kobj.kset = kset;
> --
> 2.25.1
>
>
Looks good to me.
Reviewed-by: Hyeonggon Yoo <[email protected]>
Thanks!
--
Thanks,
Hyeonggon
On Tue, 30 Aug 2022, Chao Yu wrote:
> From: Chao Yu <[email protected]>
>
> In create_unique_id(), kmalloc(, GFP_KERNEL) can fail due to
> out-of-memory, if it fails, return errno correctly rather than
> triggering panic via BUG_ON();
>
> kernel BUG at mm/slub.c:5893!
> Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
>
> Call trace:
> sysfs_slab_add+0x258/0x260 mm/slub.c:5973
> __kmem_cache_create+0x60/0x118 mm/slub.c:4899
> create_cache mm/slab_common.c:229 [inline]
> kmem_cache_create_usercopy+0x19c/0x31c mm/slab_common.c:335
> kmem_cache_create+0x1c/0x28 mm/slab_common.c:390
> f2fs_kmem_cache_create fs/f2fs/f2fs.h:2766 [inline]
> f2fs_init_xattr_caches+0x78/0xb4 fs/f2fs/xattr.c:808
> f2fs_fill_super+0x1050/0x1e0c fs/f2fs/super.c:4149
> mount_bdev+0x1b8/0x210 fs/super.c:1400
> f2fs_mount+0x44/0x58 fs/f2fs/super.c:4512
> legacy_get_tree+0x30/0x74 fs/fs_context.c:610
> vfs_get_tree+0x40/0x140 fs/super.c:1530
> do_new_mount+0x1dc/0x4e4 fs/namespace.c:3040
> path_mount+0x358/0x914 fs/namespace.c:3370
> do_mount fs/namespace.c:3383 [inline]
> __do_sys_mount fs/namespace.c:3591 [inline]
> __se_sys_mount fs/namespace.c:3568 [inline]
> __arm64_sys_mount+0x2f8/0x408 fs/namespace.c:3568
>
> Cc: <[email protected]>
> Reported-by: [email protected]
> Signed-off-by: Chao Yu <[email protected]>
Acked-by: David Rientjes <[email protected]>
On 8/31/22 05:09, Muchun Song wrote:
>
>
>> On Aug 30, 2022, at 22:10, Chao Yu <[email protected]> wrote:
Please use scripts/get_maintainer.pl next time, I could have missed this.
>> From: Chao Yu <[email protected]>
>>
>> In create_unique_id(), kmalloc(, GFP_KERNEL) can fail due to
>> out-of-memory, if it fails, return errno correctly rather than
>> triggering panic via BUG_ON();
>
> I tend to agree with you. A mount operation shouldn’t panic the
> kernel.
Hmm kmalloc(64) shouldn't normally due that due to the the underlying page
allocation falling into the "too small to fail" category, wonder if
syzkaller was doing anything special here?
But yeah we should get rid of all BUG_ONs eventually, just not sure if
stable@ is needed here.
>>
>> kernel BUG at mm/slub.c:5893!
>> Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
>>
>> Call trace:
>> sysfs_slab_add+0x258/0x260 mm/slub.c:5973
>> __kmem_cache_create+0x60/0x118 mm/slub.c:4899
>> create_cache mm/slab_common.c:229 [inline]
>> kmem_cache_create_usercopy+0x19c/0x31c mm/slab_common.c:335
>> kmem_cache_create+0x1c/0x28 mm/slab_common.c:390
>> f2fs_kmem_cache_create fs/f2fs/f2fs.h:2766 [inline]
>> f2fs_init_xattr_caches+0x78/0xb4 fs/f2fs/xattr.c:808
>> f2fs_fill_super+0x1050/0x1e0c fs/f2fs/super.c:4149
>> mount_bdev+0x1b8/0x210 fs/super.c:1400
>> f2fs_mount+0x44/0x58 fs/f2fs/super.c:4512
>> legacy_get_tree+0x30/0x74 fs/fs_context.c:610
>> vfs_get_tree+0x40/0x140 fs/super.c:1530
>> do_new_mount+0x1dc/0x4e4 fs/namespace.c:3040
>> path_mount+0x358/0x914 fs/namespace.c:3370
>> do_mount fs/namespace.c:3383 [inline]
>> __do_sys_mount fs/namespace.c:3591 [inline]
>> __se_sys_mount fs/namespace.c:3568 [inline]
>> __arm64_sys_mount+0x2f8/0x408 fs/namespace.c:3568
>>
>> Cc: <[email protected]>
>> Reported-by: [email protected]
>> Signed-off-by: Chao Yu <[email protected]>
>
> Reviewed-by: Muchun Song <[email protected]>
>
> Thanks.
>
>
Le 30/08/2022 à 16:10, Chao Yu a écrit :
> From: Chao Yu <[email protected]>
>
> In create_unique_id(), kmalloc(, GFP_KERNEL) can fail due to
> out-of-memory, if it fails, return errno correctly rather than
> triggering panic via BUG_ON();
>
> kernel BUG at mm/slub.c:5893!
> Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
>
> Call trace:
> sysfs_slab_add+0x258/0x260 mm/slub.c:5973
> __kmem_cache_create+0x60/0x118 mm/slub.c:4899
> create_cache mm/slab_common.c:229 [inline]
> kmem_cache_create_usercopy+0x19c/0x31c mm/slab_common.c:335
> kmem_cache_create+0x1c/0x28 mm/slab_common.c:390
> f2fs_kmem_cache_create fs/f2fs/f2fs.h:2766 [inline]
> f2fs_init_xattr_caches+0x78/0xb4 fs/f2fs/xattr.c:808
> f2fs_fill_super+0x1050/0x1e0c fs/f2fs/super.c:4149
> mount_bdev+0x1b8/0x210 fs/super.c:1400
> f2fs_mount+0x44/0x58 fs/f2fs/super.c:4512
> legacy_get_tree+0x30/0x74 fs/fs_context.c:610
> vfs_get_tree+0x40/0x140 fs/super.c:1530
> do_new_mount+0x1dc/0x4e4 fs/namespace.c:3040
> path_mount+0x358/0x914 fs/namespace.c:3370
> do_mount fs/namespace.c:3383 [inline]
> __do_sys_mount fs/namespace.c:3591 [inline]
> __se_sys_mount fs/namespace.c:3568 [inline]
> __arm64_sys_mount+0x2f8/0x408 fs/namespace.c:3568
>
> Cc: <[email protected]>
> Reported-by: [email protected]
> Signed-off-by: Chao Yu <[email protected]>
> ---
> mm/slub.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/mm/slub.c b/mm/slub.c
> index 862dbd9af4f5..e6f3727b9ad2 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -5890,7 +5890,8 @@ static char *create_unique_id(struct kmem_cache *s)
> char *name = kmalloc(ID_STR_LENGTH, GFP_KERNEL);
Hi,
looks that ID_STR_LENGTH could even be reduced to 32 or 16.
The 2nd BUG_ON at the end of the function could certainly be just
removed as well or remplaced by a:
if (p > name + ID_STR_LENGTH - 1) {
kfree(name);
return -E<something>;
}
Just my 2c,
CJ
> char *p = name;
>
> - BUG_ON(!name);
> + if (!name)
> + return ERR_PTR(-ENOMEM);
>
> *p++ = ':';
> /*
> @@ -5948,6 +5949,8 @@ static int sysfs_slab_add(struct kmem_cache *s)
> * for the symlinks.
> */
> name = create_unique_id(s);
> + if (IS_ERR(name))
> + return PTR_ERR(name);
> }
>
> s->kobj.kset = kset;
On Thu, Sep 08, 2022 at 11:25:08PM +0200, Vlastimil Babka (SUSE) wrote:
> > I tend to agree with you. A mount operation shouldn’t panic the
> > kernel.
>
> Hmm kmalloc(64) shouldn't normally due that due to the the underlying page
> allocation falling into the "too small to fail" category, wonder if
> syzkaller was doing anything special here?
Here's the repro:
https://syzkaller.appspot.com/x/repro.c?x=17cd7fa3080000
you can see it does:
fd = open("/proc/thread-self/fail-nth", O_RDWR);
if (fd == -1)
exit(1);
char buf[16];
sprintf(buf, "%d", nth);
if (write(fd, buf, strlen(buf)) != (ssize_t)strlen(buf))
so this is the kind of stupid nitpicky bug that we shouldn't be
reporting, let alone fixing, IMO.
On 9/9/22 22:06, Matthew Wilcox wrote:
> On Thu, Sep 08, 2022 at 11:25:08PM +0200, Vlastimil Babka (SUSE) wrote:
>> > I tend to agree with you. A mount operation shouldn’t panic the
>> > kernel.
>>
>> Hmm kmalloc(64) shouldn't normally due that due to the the underlying page
>> allocation falling into the "too small to fail" category, wonder if
>> syzkaller was doing anything special here?
>
> Here's the repro:
>
> https://syzkaller.appspot.com/x/repro.c?x=17cd7fa3080000
>
> you can see it does:
>
> fd = open("/proc/thread-self/fail-nth", O_RDWR);
> if (fd == -1)
> exit(1);
> char buf[16];
> sprintf(buf, "%d", nth);
> if (write(fd, buf, strlen(buf)) != (ssize_t)strlen(buf))
>
> so this is the kind of stupid nitpicky bug that we shouldn't be
> reporting, let alone fixing, IMO.
Ah, thanks.
Well I'm ok with eventually removing all such BUG_ONs including what
Christophe Jaillet suggested, but it certainly isn't urgent nor deserves Cc:
stable then.
On 2022/9/9 5:25, Vlastimil Babka (SUSE) wrote:
> On 8/31/22 05:09, Muchun Song wrote:
>>
>>
>>> On Aug 30, 2022, at 22:10, Chao Yu <[email protected]> wrote:
>
> Please use scripts/get_maintainer.pl next time, I could have missed this.
Oh, my bad, will Cc all maintainers next time.
Thanks,
>
>>> From: Chao Yu <[email protected]>
>>>
>>> In create_unique_id(), kmalloc(, GFP_KERNEL) can fail due to
>>> out-of-memory, if it fails, return errno correctly rather than
>>> triggering panic via BUG_ON();
>>
>> I tend to agree with you. A mount operation shouldn’t panic the
>> kernel.
>
> Hmm kmalloc(64) shouldn't normally due that due to the the underlying page
> allocation falling into the "too small to fail" category, wonder if
> syzkaller was doing anything special here?
>
> But yeah we should get rid of all BUG_ONs eventually, just not sure if
> stable@ is needed here.
>
>>>
>>> kernel BUG at mm/slub.c:5893!
>>> Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
>>>
>>> Call trace:
>>> sysfs_slab_add+0x258/0x260 mm/slub.c:5973
>>> __kmem_cache_create+0x60/0x118 mm/slub.c:4899
>>> create_cache mm/slab_common.c:229 [inline]
>>> kmem_cache_create_usercopy+0x19c/0x31c mm/slab_common.c:335
>>> kmem_cache_create+0x1c/0x28 mm/slab_common.c:390
>>> f2fs_kmem_cache_create fs/f2fs/f2fs.h:2766 [inline]
>>> f2fs_init_xattr_caches+0x78/0xb4 fs/f2fs/xattr.c:808
>>> f2fs_fill_super+0x1050/0x1e0c fs/f2fs/super.c:4149
>>> mount_bdev+0x1b8/0x210 fs/super.c:1400
>>> f2fs_mount+0x44/0x58 fs/f2fs/super.c:4512
>>> legacy_get_tree+0x30/0x74 fs/fs_context.c:610
>>> vfs_get_tree+0x40/0x140 fs/super.c:1530
>>> do_new_mount+0x1dc/0x4e4 fs/namespace.c:3040
>>> path_mount+0x358/0x914 fs/namespace.c:3370
>>> do_mount fs/namespace.c:3383 [inline]
>>> __do_sys_mount fs/namespace.c:3591 [inline]
>>> __se_sys_mount fs/namespace.c:3568 [inline]
>>> __arm64_sys_mount+0x2f8/0x408 fs/namespace.c:3568
>>>
>>> Cc: <[email protected]>
>>> Reported-by: [email protected]
>>> Signed-off-by: Chao Yu <[email protected]>
>>
>> Reviewed-by: Muchun Song <[email protected]>
>>
>> Thanks.
>>
>>
>
On 2022/9/10 0:47, Christophe JAILLET wrote:
> Le 30/08/2022 à 16:10, Chao Yu a écrit :
>> From: Chao Yu <[email protected]>
>>
>> In create_unique_id(), kmalloc(, GFP_KERNEL) can fail due to
>> out-of-memory, if it fails, return errno correctly rather than
>> triggering panic via BUG_ON();
>>
>> kernel BUG at mm/slub.c:5893!
>> Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
>>
>> Call trace:
>> sysfs_slab_add+0x258/0x260 mm/slub.c:5973
>> __kmem_cache_create+0x60/0x118 mm/slub.c:4899
>> create_cache mm/slab_common.c:229 [inline]
>> kmem_cache_create_usercopy+0x19c/0x31c mm/slab_common.c:335
>> kmem_cache_create+0x1c/0x28 mm/slab_common.c:390
>> f2fs_kmem_cache_create fs/f2fs/f2fs.h:2766 [inline]
>> f2fs_init_xattr_caches+0x78/0xb4 fs/f2fs/xattr.c:808
>> f2fs_fill_super+0x1050/0x1e0c fs/f2fs/super.c:4149
>> mount_bdev+0x1b8/0x210 fs/super.c:1400
>> f2fs_mount+0x44/0x58 fs/f2fs/super.c:4512
>> legacy_get_tree+0x30/0x74 fs/fs_context.c:610
>> vfs_get_tree+0x40/0x140 fs/super.c:1530
>> do_new_mount+0x1dc/0x4e4 fs/namespace.c:3040
>> path_mount+0x358/0x914 fs/namespace.c:3370
>> do_mount fs/namespace.c:3383 [inline]
>> __do_sys_mount fs/namespace.c:3591 [inline]
>> __se_sys_mount fs/namespace.c:3568 [inline]
>> __arm64_sys_mount+0x2f8/0x408 fs/namespace.c:3568
>>
>> Cc: <[email protected]>
>> Reported-by: [email protected]
>> Signed-off-by: Chao Yu <[email protected]>
>> ---
>> mm/slub.c | 5 ++++-
>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/mm/slub.c b/mm/slub.c
>> index 862dbd9af4f5..e6f3727b9ad2 100644
>> --- a/mm/slub.c
>> +++ b/mm/slub.c
>> @@ -5890,7 +5890,8 @@ static char *create_unique_id(struct kmem_cache *s)
>> char *name = kmalloc(ID_STR_LENGTH, GFP_KERNEL);
>
> Hi,
>
> looks that ID_STR_LENGTH could even be reduced to 32 or 16.
>
> The 2nd BUG_ON at the end of the function could certainly be just removed as well or remplaced by a:
> if (p > name + ID_STR_LENGTH - 1) {
> kfree(name);
> return -E<something>;
> }
Hi Christophe, Vlastimil,
Should I include this in v3? or may be in another patch?
Thanks,
>
> Just my 2c,
>
> CJ
>
>> char *p = name;
>> - BUG_ON(!name);
>> + if (!name)
>> + return ERR_PTR(-ENOMEM);
>> *p++ = ':';
>> /*
>> @@ -5948,6 +5949,8 @@ static int sysfs_slab_add(struct kmem_cache *s)
>> * for the symlinks.
>> */
>> name = create_unique_id(s);
>> + if (IS_ERR(name))
>> + return PTR_ERR(name);
>> }
>> s->kobj.kset = kset;
>
Le 13/09/2022 à 05:42, Chao Yu a écrit :
> On 2022/9/10 0:47, Christophe JAILLET wrote:
>> Le 30/08/2022 à 16:10, Chao Yu a écrit :
>>> From: Chao Yu <[email protected]>
>>>
>>> In create_unique_id(), kmalloc(, GFP_KERNEL) can fail due to
>>> out-of-memory, if it fails, return errno correctly rather than
>>> triggering panic via BUG_ON();
>>>
>>> kernel BUG at mm/slub.c:5893!
>>> Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
>>>
>>> Call trace:
[...]
>>>
>>> Cc: <[email protected]>
>>> Reported-by: [email protected]
>>> Signed-off-by: Chao Yu <[email protected]>
>>> ---
>>> mm/slub.c | 5 ++++-
>>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/mm/slub.c b/mm/slub.c
>>> index 862dbd9af4f5..e6f3727b9ad2 100644
>>> --- a/mm/slub.c
>>> +++ b/mm/slub.c
>>> @@ -5890,7 +5890,8 @@ static char *create_unique_id(struct
>>> kmem_cache *s)
>>> char *name = kmalloc(ID_STR_LENGTH, GFP_KERNEL);
>>
>> Hi,
>>
>> looks that ID_STR_LENGTH could even be reduced to 32 or 16.
>>
>> The 2nd BUG_ON at the end of the function could certainly be just
>> removed as well or remplaced by a:
>> if (p > name + ID_STR_LENGTH - 1) {
>> kfree(name);
>> return -E<something>;
>> }
>
> Hi Christophe, Vlastimil,
>
> Should I include this in v3? or may be in another patch?
Hi,
My own preference would be for 3 patches.
Yours, as-is.
It fixes a specific issue spotted by syzbot.
Another one for removing a BUG_ON() (that, IIUC can't happen!)
Mostly a clean-up or a good practice in order to remove BUG_ON() from
the kernel we it can be handled another way.
Eventually a 3rd one for reducing ID_STR_LENGTH.
I guess that it is safe to reduce it to 32 or 16, but the impact on RL
would be so small, that I wonder if it worth proposing it.
Just my 2c,
CJ
>
> Thanks,
>
>>
>> Just my 2c,
>>
>> CJ
>>
>>> char *p = name;
>>> - BUG_ON(!name);
>>> + if (!name)
>>> + return ERR_PTR(-ENOMEM);
>>> *p++ = ':';
>>> /*
>>> @@ -5948,6 +5949,8 @@ static int sysfs_slab_add(struct kmem_cache *s)
>>> * for the symlinks.
>>> */
>>> name = create_unique_id(s);
>>> + if (IS_ERR(name))
>>> + return PTR_ERR(name);
>>> }
>>> s->kobj.kset = kset;
>>
On 9/13/22 07:26, Marion & Christophe JAILLET wrote:
>
> Le 13/09/2022 à 05:42, Chao Yu a écrit :
>> On 2022/9/10 0:47, Christophe JAILLET wrote:
>>> Le 30/08/2022 à 16:10, Chao Yu a écrit :
>>>> From: Chao Yu <[email protected]>
>>>>
>>>> In create_unique_id(), kmalloc(, GFP_KERNEL) can fail due to
>>>> out-of-memory, if it fails, return errno correctly rather than
>>>> triggering panic via BUG_ON();
>>>>
>>>> kernel BUG at mm/slub.c:5893!
>>>> Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
>>>>
>>>> Call trace:
> [...]
>>>>
>>>> Cc: <[email protected]>
>>>> Reported-by: [email protected]
>>>> Signed-off-by: Chao Yu <[email protected]>
>>>> ---
>>>> mm/slub.c | 5 ++++-
>>>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/mm/slub.c b/mm/slub.c
>>>> index 862dbd9af4f5..e6f3727b9ad2 100644
>>>> --- a/mm/slub.c
>>>> +++ b/mm/slub.c
>>>> @@ -5890,7 +5890,8 @@ static char *create_unique_id(struct kmem_cache *s)
>>>> char *name = kmalloc(ID_STR_LENGTH, GFP_KERNEL);
>>>
>>> Hi,
>>>
>>> looks that ID_STR_LENGTH could even be reduced to 32 or 16.
>>>
>>> The 2nd BUG_ON at the end of the function could certainly be just removed
>>> as well or remplaced by a:
>>> if (p > name + ID_STR_LENGTH - 1) {
>>> kfree(name);
>>> return -E<something>;
>>> }
>>
>> Hi Christophe, Vlastimil,
>>
>> Should I include this in v3? or may be in another patch?
>
> Hi,
>
> My own preference would be for 3 patches.
>
> Yours, as-is.
> It fixes a specific issue spotted by syzbot.
Yeah and it's already in git.
> Another one for removing a BUG_ON() (that, IIUC can't happen!)
> Mostly a clean-up or a good practice in order to remove BUG_ON() from the
> kernel we it can be handled another way.
>
> Eventually a 3rd one for reducing ID_STR_LENGTH.
> I guess that it is safe to reduce it to 32 or 16, but the impact on RL would
> be so small, that I wonder if it worth proposing it.
Agree. Doing 2+3 in the same patch would be OK with me too.