lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <c797bdde-b5a5-4b2b-9106-c1386a0e7516@easystack.cn>
Date: Tue, 19 Dec 2023 11:50:32 +0800
From: fuqiang wang <fuqiang.wang@...ystack.cn>
To: Yuntao Wang <ytcoode@...il.com>
Cc: bhe@...hat.com, dyoung@...hat.com, kexec@...ts.infradead.org,
 linux-kernel@...r.kernel.org, vgoyal@...hat.com
Subject: Re: [PATCH] kexec: avoid out of bounds in crash_exclude_mem_range()

在 2023/12/19 10:47, Yuntao Wang 写道:

> Hi fuqiang,
>
> Yesterday, I posted two patches that happen to address the bugs you an Baoquan
> are currently discussing here, I wasn't aware that you both were also working
> on fixing these issues.
>
> Baoquan suggested I talk to you about it.
>
> If you're interested, you can take a look at my patches and review them to see
> if there are any issues. If everything is fine, and if you're willing, you can
> also add a 'Reviewed-by' tag there.
>
> The following link is for the two patches I posted yesterday:
>
> https://lore.kernel.org/lkml/20231218081915.24120-3-ytcoode@gmail.com/t/#u
>
> Sincerely,
> Yuntao

Hi Yuntao,

I'm glad you've also noticed this issue. But I'm sorry, I want to solve this
problem myself because this is my first time posting a patch in the community,
and I cherish this opportunity very much.

I have carefully reviewed your patch. There is some changes where my views differ
from yours:
diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
index c92d88680dbf..3be46f4b441e 100644
--- a/arch/x86/kernel/crash.c
+++ b/arch/x86/kernel/crash.c
@@ -282,10 +282,6 @@ int crash_setup_memmap_entries(struct kimage *image, struct boot_params *params)
      struct crash_memmap_data cmd;
      struct crash_mem *cmem;

-    cmem = vzalloc(struct_size(cmem, ranges, 1));
-    if (!cmem)
-        return -ENOMEM;
-
      memset(&cmd, 0, sizeof(struct crash_memmap_data));
      cmd.params = params;

@@ -321,6 +317,11 @@ int crash_setup_memmap_entries(struct kimage *image, struct boot_params *params)
      }

      /* Exclude some ranges from crashk_res and add rest to memmap */
+    cmem = vzalloc(struct_size(cmem, ranges, 1));
+    if (!cmem)
+        return -ENOMEM;
+    cmem->max_nr_ranges = 1;
+
      ret = memmap_exclude_ranges(image, cmem, crashk_res.start, crashk_res.end);
      if (ret)
          goto out;

1. I don't feel very good that you have moved vzalloc() to in front of
memmap_exclude_ranges. Because if memory allocation fails, there is no need to
do anything else afterwards.

2. The cmem->max_nr_ranges should be set to 2. Because in
memmap_exclude_ranges, a cmem->ranges[] will be filled in and if a split occurs
later, another one will be added.

Thanks
fuqiang


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ