| Message ID | 20220830141009.150075-1-chao@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mm/slub: fix to return errno if kmalloc() fails | expand |
> On Aug 30, 2022, at 22:10, Chao Yu <chao@kernel.org> wrote: > > From: Chao Yu <chao.yu@oppo.com> > > 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: <stable@kernel.org> > Reported-by: syzbot+81684812ea68216e08c5@syzkaller.appspotmail.com > Signed-off-by: Chao Yu <chao.yu@oppo.com> Reviewed-by: Muchun Song <songmuchun@bytedance.com> Thanks.
On Tue, Aug 30, 2022 at 10:10:09PM +0800, Chao Yu wrote: > From: Chao Yu <chao.yu@oppo.com> > > 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: <stable@kernel.org> > Reported-by: syzbot+81684812ea68216e08c5@syzkaller.appspotmail.com Fixes: 81819f0fc8285 ("SLUB core") > Signed-off-by: Chao Yu <chao.yu@oppo.com> > --- > 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 <42.hyeyoo@gmail.com> Thanks!
On Tue, 30 Aug 2022, Chao Yu wrote: > From: Chao Yu <chao.yu@oppo.com> > > 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: <stable@kernel.org> > Reported-by: syzbot+81684812ea68216e08c5@syzkaller.appspotmail.com > Signed-off-by: Chao Yu <chao.yu@oppo.com> Acked-by: David Rientjes <rientjes@google.com>
On 8/31/22 05:09, Muchun Song wrote: > > >> On Aug 30, 2022, at 22:10, Chao Yu <chao@kernel.org> wrote: Please use scripts/get_maintainer.pl next time, I could have missed this. >> From: Chao Yu <chao.yu@oppo.com> >> >> 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: <stable@kernel.org> >> Reported-by: syzbot+81684812ea68216e08c5@syzkaller.appspotmail.com >> Signed-off-by: Chao Yu <chao.yu@oppo.com> > > Reviewed-by: Muchun Song <songmuchun@bytedance.com> > > Thanks. > >
Le 30/08/2022 à 16:10, Chao Yu a écrit : > From: Chao Yu <chao.yu@oppo.com> > > 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: <stable@kernel.org> > Reported-by: syzbot+81684812ea68216e08c5@syzkaller.appspotmail.com > Signed-off-by: Chao Yu <chao.yu@oppo.com> > --- > 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 <chao@kernel.org> 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 <chao.yu@oppo.com> >>> >>> 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: <stable@kernel.org> >>> Reported-by: syzbot+81684812ea68216e08c5@syzkaller.appspotmail.com >>> Signed-off-by: Chao Yu <chao.yu@oppo.com> >> >> Reviewed-by: Muchun Song <songmuchun@bytedance.com> >> >> Thanks. >> >> >
On 2022/9/10 0:47, Christophe JAILLET wrote: > Le 30/08/2022 à 16:10, Chao Yu a écrit : >> From: Chao Yu <chao.yu@oppo.com> >> >> 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: <stable@kernel.org> >> Reported-by: syzbot+81684812ea68216e08c5@syzkaller.appspotmail.com >> Signed-off-by: Chao Yu <chao.yu@oppo.com> >> --- >> 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 <chao.yu@oppo.com> >>> >>> 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: <stable@kernel.org> >>> Reported-by: syzbot+81684812ea68216e08c5@syzkaller.appspotmail.com >>> Signed-off-by: Chao Yu <chao.yu@oppo.com> >>> --- >>> 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 <chao.yu@oppo.com> >>>> >>>> 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: <stable@kernel.org> >>>> Reported-by: syzbot+81684812ea68216e08c5@syzkaller.appspotmail.com >>>> Signed-off-by: Chao Yu <chao.yu@oppo.com> >>>> --- >>>> 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.
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;