| Message ID | 20231130090953.2322490-1-chentao@kylinos.cn (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [v2] powerpc/mm: Fix null-pointer dereference in pgtable_cache_add | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/github-powerpc_ppctests | success | Successfully ran 8 jobs. |
| snowpatch_ozlabs/github-powerpc_selftests | success | Successfully ran 8 jobs. |
| snowpatch_ozlabs/github-powerpc_sparse | success | Successfully ran 4 jobs. |
| snowpatch_ozlabs/github-powerpc_clang | success | Successfully ran 6 jobs. |
| snowpatch_ozlabs/github-powerpc_kernel_qemu | success | Successfully ran 23 jobs. |
Kunwu Chan <chentao@kylinos.cn> writes: > kasprintf() returns a pointer to dynamically allocated memory > which can be NULL upon failure. Ensure the allocation was successful > by checking the pointer validity. > > Suggested-by: Christophe Leroy <christophe.leroy@csgroup.eu> > Suggested-by: Michael Ellerman <mpe@ellerman.id.au> > Signed-off-by: Kunwu Chan <chentao@kylinos.cn> > --- > v2: Use "panic" instead of "return" > --- > arch/powerpc/mm/init-common.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/arch/powerpc/mm/init-common.c b/arch/powerpc/mm/init-common.c > index 119ef491f797..9788950b33f5 100644 > --- a/arch/powerpc/mm/init-common.c > +++ b/arch/powerpc/mm/init-common.c > @@ -139,6 +139,8 @@ void pgtable_cache_add(unsigned int shift) > > align = max_t(unsigned long, align, minalign); > name = kasprintf(GFP_KERNEL, "pgtable-2^%d", shift); > + if (!name) > + panic("Failed to allocate memory for order %d", shift); > new = kmem_cache_create(name, table_size, align, 0, ctor(shift)); > if (!new) > panic("Could not allocate pgtable cache for order %d", shift); It would be nice to avoid two calls to panic. Can you reorganise the logic so that there's only one? Initialising new to NULL might help. cheers
Sure,i'll follow your suggestion in v3 patch:
1. set new to NULL
2. add a 'if' judgment before 'kmem_cache_create'
Thanks,
Kunwu
On 2023/12/1 18:17, Michael Ellerman wrote:
> avoid two calls to panic
diff --git a/arch/powerpc/mm/init-common.c b/arch/powerpc/mm/init-common.c index 119ef491f797..9788950b33f5 100644 --- a/arch/powerpc/mm/init-common.c +++ b/arch/powerpc/mm/init-common.c @@ -139,6 +139,8 @@ void pgtable_cache_add(unsigned int shift) align = max_t(unsigned long, align, minalign); name = kasprintf(GFP_KERNEL, "pgtable-2^%d", shift); + if (!name) + panic("Failed to allocate memory for order %d", shift); new = kmem_cache_create(name, table_size, align, 0, ctor(shift)); if (!new) panic("Could not allocate pgtable cache for order %d", shift);
kasprintf() returns a pointer to dynamically allocated memory which can be NULL upon failure. Ensure the allocation was successful by checking the pointer validity. Suggested-by: Christophe Leroy <christophe.leroy@csgroup.eu> Suggested-by: Michael Ellerman <mpe@ellerman.id.au> Signed-off-by: Kunwu Chan <chentao@kylinos.cn> --- v2: Use "panic" instead of "return" --- arch/powerpc/mm/init-common.c | 2 ++ 1 file changed, 2 insertions(+)