lkml.org 
[lkml]   [2021]   [Aug]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH] net: fix NULL pointer reference in cipso_v4_doi_free
    From
    Date
    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
    >>
    >
    >

    \
     
     \ /
      Last update: 2021-08-30 12:21    [W:8.465 / U:0.040 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site