Messages in this thread | | | Subject | Re: [PATCH] net: fix NULL pointer reference in cipso_v4_doi_free | From | 王贇 <> | Date | Mon, 30 Aug 2021 18:20:19 +0800 |
| |
Hi, Paul
I'm sorry for missing this mail since my stupid filter rules...
Will send a new one soon as you suggested :-)
Regards, Michael Wang
On 2021/8/27 上午8:09, Paul Moore wrote: [snip] >> >> Reported-by: Abaci <abaci@linux.alibaba.com> >> Signed-off-by: Michael Wang <yun.wang@linux.alibaba.com> >> --- >> >> net/ipv4/cipso_ipv4.c | 18 ++++++++++-------- >> 1 file changed, 10 insertions(+), 8 deletions(-) > > Thanks for the problem report. It's hard to say for certain due to > the abbreviated backtrace without line number information, but it > looks like the problem you are describing is happening when the > allocation for doi_def->map.std fails near the top of > netlbl_cipsov4_add_std() which causes the function to jump the > add_std_failure target which ends up calling cipso_v4_doi_free(). > > doi_def = kmalloc(sizeof(*doi_def), GFP_KERNEL); > if (doi_def == NULL) > return -ENOMEM; > doi_def->map.std = kzalloc(sizeof(*doi_def->map.std), GFP_KERNEL); > if (doi_def->map.std == NULL) { > ret_val = -ENOMEM; > goto add_std_failure; > } > ... > add_std_failure: > cipso_v4_doi_free(doi_def); > > Since the doi_def allocation is not zero'd out, it is possible that > the doi_def->type value could have a value of CIPSO_V4_MAP_TRANS when > the doi_def->map.std allocation fails, causing the NULL pointer deref > in cipso_v4_doi_free(). As this is the only case where we would see a > problem like this, I suggest a better solution would be to change the > if-block following the doi_def->map.std allocation to something like > this: > > doi_def->map.std = kzalloc(sizeof(*doi_def->map.std), GFP_KERNEL); > if (doi_def->map.std == NULL) { > kfree(doi_def); > return -ENOMEM; > } > >> diff --git a/net/ipv4/cipso_ipv4.c b/net/ipv4/cipso_ipv4.c >> index 099259f..7fbd0b5 100644 >> --- a/net/ipv4/cipso_ipv4.c >> +++ b/net/ipv4/cipso_ipv4.c >> @@ -465,14 +465,16 @@ void cipso_v4_doi_free(struct cipso_v4_doi *doi_def) >> if (!doi_def) >> return; >> >> - switch (doi_def->type) { >> - case CIPSO_V4_MAP_TRANS: >> - kfree(doi_def->map.std->lvl.cipso); >> - kfree(doi_def->map.std->lvl.local); >> - kfree(doi_def->map.std->cat.cipso); >> - kfree(doi_def->map.std->cat.local); >> - kfree(doi_def->map.std); >> - break; >> + if (doi_def->map.std) { >> + switch (doi_def->type) { >> + case CIPSO_V4_MAP_TRANS: >> + kfree(doi_def->map.std->lvl.cipso); >> + kfree(doi_def->map.std->lvl.local); >> + kfree(doi_def->map.std->cat.cipso); >> + kfree(doi_def->map.std->cat.local); >> + kfree(doi_def->map.std); >> + break; >> + } >> } >> kfree(doi_def); >> } >> -- >> 1.8.3.1 >> > >
| |