| Message ID | 20221206083657.3202856-1-ruanjinjie@huawei.com |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | of: overlay: fix null pointer dereferencing in find_dup_cset_node_entry() and find_dup_cset_prop() | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | warning | total: 0 errors, 1 warnings, 22 lines checked |
| robh/patch-applied | fail | build log |
On Tue, Dec 06, 2022 at 04:36:57PM +0800, ruanjinjie wrote: > when kmalloc() fail to allocate memory in kasprintf(), fn_1 or fn_2 will > be NULL, strcmp() will cause null pointer dereference. > > Fixes: 2fe0e8769df9 ("of: overlay: check prevents multiple fragments touching same property") > Signed-off-by: ruanjinjie <ruanjinjie@huawei.com> > --- > drivers/of/overlay.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c > index bd8ff4df723d..49c066b51148 100644 > --- a/drivers/of/overlay.c > +++ b/drivers/of/overlay.c > @@ -545,6 +545,11 @@ static int find_dup_cset_node_entry(struct overlay_changeset *ovcs, > > fn_1 = kasprintf(GFP_KERNEL, "%pOF", ce_1->np); > fn_2 = kasprintf(GFP_KERNEL, "%pOF", ce_2->np); > + if (!fn_1 || !fn_2) { > + kfree(fn_1); > + kfree(fn_2); > + return -ENOMEM; > + } > node_path_match = !strcmp(fn_1, fn_2); We don't actually care what the return code is, so just change this to: node_path_match = !fn_1 || !fn_2 || !strcmp(fn_1, fn_2); > kfree(fn_1); > kfree(fn_2); > @@ -580,6 +585,11 @@ static int find_dup_cset_prop(struct overlay_changeset *ovcs, > > fn_1 = kasprintf(GFP_KERNEL, "%pOF", ce_1->np); > fn_2 = kasprintf(GFP_KERNEL, "%pOF", ce_2->np); > + if (!fn_1 || !fn_2) { > + kfree(fn_1); > + kfree(fn_2); > + return -ENOMEM; > + } > node_path_match = !strcmp(fn_1, fn_2); > kfree(fn_1); > kfree(fn_2); > -- > 2.25.1 > >
On 2022/12/10 4:59, Rob Herring wrote: > On Tue, Dec 06, 2022 at 04:36:57PM +0800, ruanjinjie wrote: >> when kmalloc() fail to allocate memory in kasprintf(), fn_1 or fn_2 will >> be NULL, strcmp() will cause null pointer dereference. >> >> Fixes: 2fe0e8769df9 ("of: overlay: check prevents multiple fragments touching same property") >> Signed-off-by: ruanjinjie <ruanjinjie@huawei.com> >> --- >> drivers/of/overlay.c | 10 ++++++++++ >> 1 file changed, 10 insertions(+) >> >> diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c >> index bd8ff4df723d..49c066b51148 100644 >> --- a/drivers/of/overlay.c >> +++ b/drivers/of/overlay.c >> @@ -545,6 +545,11 @@ static int find_dup_cset_node_entry(struct overlay_changeset *ovcs, >> >> fn_1 = kasprintf(GFP_KERNEL, "%pOF", ce_1->np); >> fn_2 = kasprintf(GFP_KERNEL, "%pOF", ce_2->np); >> + if (!fn_1 || !fn_2) { >> + kfree(fn_1); >> + kfree(fn_2); >> + return -ENOMEM; >> + } >> node_path_match = !strcmp(fn_1, fn_2); > > We don't actually care what the return code is, so just change this to: > > node_path_match = !fn_1 || !fn_2 || !strcmp(fn_1, fn_2); Thank you very much!I'll give the v2 patch soon. > >> kfree(fn_1); >> kfree(fn_2); >> @@ -580,6 +585,11 @@ static int find_dup_cset_prop(struct overlay_changeset *ovcs, >> >> fn_1 = kasprintf(GFP_KERNEL, "%pOF", ce_1->np); >> fn_2 = kasprintf(GFP_KERNEL, "%pOF", ce_2->np); >> + if (!fn_1 || !fn_2) { >> + kfree(fn_1); >> + kfree(fn_2); >> + return -ENOMEM; >> + } >> node_path_match = !strcmp(fn_1, fn_2); >> kfree(fn_1); >> kfree(fn_2); >> -- >> 2.25.1 >> >> >
diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c index bd8ff4df723d..49c066b51148 100644 --- a/drivers/of/overlay.c +++ b/drivers/of/overlay.c @@ -545,6 +545,11 @@ static int find_dup_cset_node_entry(struct overlay_changeset *ovcs, fn_1 = kasprintf(GFP_KERNEL, "%pOF", ce_1->np); fn_2 = kasprintf(GFP_KERNEL, "%pOF", ce_2->np); + if (!fn_1 || !fn_2) { + kfree(fn_1); + kfree(fn_2); + return -ENOMEM; + } node_path_match = !strcmp(fn_1, fn_2); kfree(fn_1); kfree(fn_2); @@ -580,6 +585,11 @@ static int find_dup_cset_prop(struct overlay_changeset *ovcs, fn_1 = kasprintf(GFP_KERNEL, "%pOF", ce_1->np); fn_2 = kasprintf(GFP_KERNEL, "%pOF", ce_2->np); + if (!fn_1 || !fn_2) { + kfree(fn_1); + kfree(fn_2); + return -ENOMEM; + } node_path_match = !strcmp(fn_1, fn_2); kfree(fn_1); kfree(fn_2);
when kmalloc() fail to allocate memory in kasprintf(), fn_1 or fn_2 will be NULL, strcmp() will cause null pointer dereference. Fixes: 2fe0e8769df9 ("of: overlay: check prevents multiple fragments touching same property") Signed-off-by: ruanjinjie <ruanjinjie@huawei.com> --- drivers/of/overlay.c | 10 ++++++++++ 1 file changed, 10 insertions(+)