From: 王贇 <yun.wang@linux.alibaba.com>
To: Paul Moore <paul@paul-moore.com>
Cc: "David S. Miller" <davem@davemloft.net>,
Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>,
David Ahern <dsahern@kernel.org>,
Jakub Kicinski <kuba@kernel.org>,
netdev@vger.kernel.org, linux-security-module@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH] net: fix NULL pointer reference in cipso_v4_doi_free
Date: Mon, 30 Aug 2021 18:20:19 +0800 [thread overview]
Message-ID: <29171cda-0b6c-b6a9-0123-f356610d0ed4@linux.alibaba.com> (raw)
In-Reply-To: <CAHC9VhRJtU48Zt7dUEaTvKRoO+ODki75rS-hdJ0HPBrPRmCfxQ@mail.gmail.com>
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
>>
>
>
next prev parent reply other threads:[~2021-08-30 10:20 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-08-26 3:42 [PATCH] net: fix NULL pointer reference in cipso_v4_doi_free 王贇
2021-08-27 0:09 ` Paul Moore
2021-08-30 10:20 ` 王贇 [this message]
2021-08-30 10:14 ` 王贇
2021-08-30 10:28 ` [PATCH v2] " 王贇
2021-08-30 11:30 ` patchwork-bot+netdevbpf
2021-08-30 14:17 ` Paul Moore
2021-08-30 16:45 ` Jakub Kicinski
2021-08-30 16:50 ` Paul Moore
2021-08-31 2:41 ` 王贇
2021-08-31 13:48 ` Paul Moore
2021-09-01 1:51 ` 王贇
2021-09-01 9:30 ` David Miller
2021-09-01 9:41 ` 王贇
2021-09-01 10:45 ` David Miller
2021-09-02 3:04 ` 王贇
2021-09-01 2:18 ` [PATCH] Revert "net: fix NULL pointer reference in cipso_v4_doi_free" 王贇
2021-09-01 2:21 ` 王贇
2021-09-01 21:05 ` Paul Moore
2021-09-02 2:37 ` 王贇
2021-09-03 2:15 ` Paul Moore
2021-09-03 2:31 ` 王贇
2021-09-03 14:08 ` Paul Moore
2021-09-03 2:27 ` [PATCH] net: remove the unnecessary check in cipso_v4_doi_free 王贇
2021-09-03 14:08 ` Paul Moore
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=29171cda-0b6c-b6a9-0123-f356610d0ed4@linux.alibaba.com \
--to=yun.wang@linux.alibaba.com \
--cc=davem@davemloft.net \
--cc=dsahern@kernel.org \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-security-module@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=paul@paul-moore.com \
--cc=yoshfuji@linux-ipv6.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).