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]

quoted

Reported-by: Abaci <redacted>
Signed-off-by: Michael Wang <redacted>
---

 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;
  }

quoted

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