Skip to contentSkip to conversation

riscv: Fix a NULL pointer dereference in machine_kexec_prepare

PATCH Superseded Tao Liu 5 participants 10 messages Jul 1 – Jul 3

Subsystems (5)
Kernel janitors RISC-V Live update kexec handover (KHO) +1 more subsystems

v3 of 5 Superseded Latest is v5 →

lore
Share & tools
lore

Tao Liu

author
#0 lore raw

PATCH v3 1/1 riscv: Fix a NULL pointer dereference in machine_kexec_prepare

A NULL pointer dereference issue is noticed in riscv's machine_kexec_prepare, where image->segment[i].buf might be NULL and copied unchecked.

The NULL buf comes from security/integrity/ima/ima_kexec.c:
ima_add_kexec_buffer(), where kbuf is added by kexec_add_buffer(), but kbuf.buffer is NULL.

Fix this by simply adding a check before copy.

Fixes b7fb4d78a6ad ("RISC-V: use memcpy for kexec_file mode")
Acked-by Baoquan He <[email protected]>
Acked-by Pratyush Yadav <[email protected]>
Signed-off-by Tao Liu <[email protected]>

---

v3 -> v2: Add fixes tag; Replace "reference" to "dereference".
link to v2: https://lore.kernel.org/linux-riscv/[email protected]/
link to v1: https://lore.kernel.org/linux-riscv/[email protected]/

---
 arch/riscv/kernel/machine_kexec.c | 7 +++++++
 1 file changed, 7 insertions(+)
arch/riscv/kernel/machine_kexec.c +7 -0
index 2306ce3e5f22..afc68f6a4aa1 100644
@@ -41,6 +41,13 @@ machine_kexec_prepare(struct kimage *image)
 		if (image->segment[i].memsz <= sizeof(fdt))
 			continue;
 
+		/*
+		 * Some segments (e.g. IMA) reserve space but have no buffer
+		 * loaded yet. Skip them as they cannot contain an FDT.
+		 */
+		if (image->segment[i].buf == NULL)
+			continue;
+
 		if (image->file_mode)
 			memcpy(&fdt, image->segment[i].buf, sizeof(fdt));
 		else if (copy_from_user(&fdt, image->segment[i].buf, sizeof(fdt)))
Signature · 1 line
2.54.0
Reviewed-by on v3 1/1
#1 lore raw
Quoted 7 lines · Tao Liu

A NULL pointer dereference issue is noticed in riscv's machine_kexec_prepare, where image->segment[i].buf might be NULL and copied unchecked.

The NULL buf comes from security/integrity/ima/ima_kexec.c:
ima_add_kexec_buffer(), where kbuf is added by kexec_add_buffer(), but kbuf.buffer is NULL.

Quoted 6 lines

Fix this by simply adding a check before copy.

Fixes b7fb4d78a6ad ("RISC-V: use memcpy for kexec_file mode")
Acked-by Baoquan He <[email protected]>
Acked-by Pratyush Yadav <[email protected]>
Signed-off-by Tao Liu <[email protected]>

It would be better to add the log of the dereference calltrace. Otherwise,

Reviewed-by Nutty Liu <[email protected]>

Thanks,
Nutty

Quoted 28 lines

---

v3 -> v2: Add fixes tag; Replace "reference" to "dereference".
link to v2: https://lore.kernel.org/linux-riscv/[email protected]/
link to v1: https://lore.kernel.org/linux-riscv/[email protected]/

---
  arch/riscv/kernel/machine_kexec.c | 7 +++++++
  1 file changed, 7 insertions(+)
arch/riscv/kernel/machine_kexec.c +7 -0
index 2306ce3e5f22..afc68f6a4aa1 100644
@@ -41,6 +41,13 @@ machine_kexec_prepare(struct kimage *image)
  		if (image->segment[i].memsz <= sizeof(fdt))
  			continue;
 
+		/*
+		 * Some segments (e.g. IMA) reserve space but have no buffer
+		 * loaded yet. Skip them as they cannot contain an FDT.
+		 */
+		if (image->segment[i].buf == NULL)
+			continue;
+
  		if (image->file_mode)
  			memcpy(&fdt, image->segment[i].buf, sizeof(fdt));
  		else if (copy_from_user(&fdt, image->segment[i].buf, sizeof(fdt)))
maintainer
#2 lore raw
In reply to Tao Liu
Quoted 6 lines · Tao Liu

A NULL pointer dereference issue is noticed in riscv's machine_kexec_prepare, where image->segment[i].buf might be NULL and copied unchecked.

The NULL buf comes from security/integrity/ima/ima_kexec.c:
ima_add_kexec_buffer(), where kbuf is added by kexec_add_buffer(), but kbuf.buffer is NULL

This should have a proper call sequence. Now the root cause is obfuscated.

Quoted 22 lines

Fix this by simply adding a check before copy.

Fixes b7fb4d78a6ad ("RISC-V: use memcpy for kexec_file mode")
Acked-by Baoquan He <[email protected]>
Acked-by Pratyush Yadav <[email protected]>
Signed-off-by Tao Liu <[email protected]>

---

v3 -> v2: Add fixes tag; Replace "reference" to "dereference".
link to v2: https://lore.kernel.org/linux-riscv/[email protected]/
link to v1: https://lore.kernel.org/linux-riscv/[email protected]/

---
 arch/riscv/kernel/machine_kexec.c | 7 +++++++
 1 file changed, 7 insertions(+)
arch/riscv/kernel/machine_kexec.c +0 -0
index 2306ce3e5f22..afc68f6a4aa1 100644

arch/riscv/kernel/machine_kexec.c machine_kexec_prepare(struct kimage *image) · line 47

Quoted 7 lines
arch/riscv/kernel/machine_kexec.c +4 -0
@@ -41,6 +41,13 @@ machine_kexec_prepare(struct kimage *image)
 		if (image->segment[i].memsz <= sizeof(fdt))
 			continue;
 
+		/*
+		 * Some segments (e.g. IMA) reserve space but have no buffer
+		 * loaded yet. Skip them as they cannot contain an FDT.
+		 */

This is destined to rot over time. It also adds up also potentially to the backporting effort while backporting to stable kernes. And most importantly. Please, don't document every other null check.

arch/riscv/kernel/machine_kexec.c machine_kexec_prepare(struct kimage *image) · line 48

Quoted 1 line
arch/riscv/kernel/machine_kexec.c +1 -0
+		if (image->segment[i].buf == NULL)

if (!image->segments[i].buf)

Quoted 9 lines
arch/riscv/kernel/machine_kexec.c +2 -0
+			continue;
+
 		if (image->file_mode)
 			memcpy(&fdt, image->segment[i].buf, sizeof(fdt));
 		else if (copy_from_user(&fdt, image->segment[i].buf, sizeof(fdt)))
Signature · 1 line
2.54.0

BR, Jarkko

author
#3 lore raw

Hi Jarkko,

Quoted 11 lines · Jarkko Sakkinen
Quoted 6 lines · Tao Liu

A NULL pointer dereference issue is noticed in riscv's machine_kexec_prepare, where image->segment[i].buf might be NULL and copied unchecked.

The NULL buf comes from security/integrity/ima/ima_kexec.c:
ima_add_kexec_buffer(), where kbuf is added by kexec_add_buffer(), but kbuf.buffer is NULL

This should have a proper call sequence. Now the root cause is obfuscated.

Sure, I will attach the stack trace in v4. Here is the one:

[   62.867540] kexec_file(Image): Loaded kernel at 0x80200000
bufsz=0x34ed800 memsz=0x35d0000
[   62.879983] Unable to handle kernel access to user memory without
uaccess routines at virtual address 0000000000000000
[   62.880736] Current kexec pgtable: 4K pagesize, 57-bit VAs,
pgdp=0x00000001062eb000
[   62.881185] [0000000000000000] pgd=00000000413b4401,
p4d=000000004151bc01, pud=00000000415b7801, pmd=0000000040af5801,
pte=0000000000000000
[   62.881969] Oops [#1]
[   62.882077] Modules linked in:
[   62.882717] CPU: 1 UID: 0 PID: 894 Comm: kexec Not tainted 7.1.1 #4
PREEMPT(lazy)
[   62.883037] Hardware name: QEMU QEMU Virtual Machine, BIOS
edk2-20260508-2.fc44 05/08/2026
[   62.883365] epc : __memcpy+0xd4/0xf8
[   62.883685]  ra : machine_kexec_prepare+0x8a/0x298
[   62.883914] epc : ffffffff81393ee8 ra : ffffffff800366ca sp :
ff20000004a83d10
[   62.884214]  gp : ffffffff83258db8 tp : ff6000008573db80 t0 :
ffffffff80033640
[   62.884433]  t1 : 2152ffffffffffc0 t2 : 0000000003000000 s0 :
ff20000004a83d80
[   62.884710]  s1 : 0000000000000000 a0 : ff20000004a83d10 a1 :
0000000000000000
[   62.884987]  a2 : 0000000000000028 a3 : 0000000000000028 a4 :
0000000000000000
[   62.885208]  a5 : 0000000000000000 a6 : 0000000104e33fff a7 :
0000000000000000
[   62.885486]  s2 : ff60000082a35800 s3 : 0000000000000000 s4 :
0000000000000010
[   62.885774]  s5 : 0000000000000028 s6 : ff20000004a83d10 s7 :
0000000000000005
[   62.886005]  s8 : 00000000000000c0 s9 : 000000000ac0d220 s10:
ffffffff835420e8
[   62.886218]  s11: 0000000000000000 t3 : ffffff800000007c t4 :
ff1c000002138d00
[   62.886515]  t5 : ffffffffffffffff t6 : ff20000004a83d10 ssp :
0000000000000000
[   62.886860] status: 0000000200000120 badaddr: 0000000000000000
cause: 000000000000000d
[   62.887162] [<ffffffff81393ee8>] __memcpy+0xd4/0xf8
[   62.887388] [<ffffffff801b253a>] __do_sys_kexec_file_load+0x1b2/0x338
[   62.887612] [<ffffffff801b26e4>] __riscv_sys_kexec_file_load+0x24/0x40
[   62.887855] [<ffffffff81395ea4>] do_trap_ecall_u+0x1a4/0x5a8
[   62.888134] [<ffffffff813a9eec>] handle_exception+0x16c/0x178
[   62.888445] Code: 7613 07f6 ca05 86b3 00c5 e7b3 01f5 8fd5 8b8d eb89
(4198) 0591
[   62.889223] ---[ end trace 0000000000000000 ]---

Segmentation fault kexec -l /boot/vmlinuz-7.1.1
--initrd=/boot/initramfs-7.1.1.img --reuse-cmdline

(gdb) p image->segment[0]
$3 = {{buf = 0x0, kbuf = 0x0}, bufsz = 0, mem = 10737586176, memsz = 4096}

The buf = 0x0 and bufsz = 0 comes from ima_add_kexec_buffer(), thought I'm not sure why it added a NULL segment, but it is no harm to add a NULL checker here in case any other scenarios similar as IMA.

Quoted 26 lines
Quoted 26 lines

Fix this by simply adding a check before copy.

Fixes b7fb4d78a6ad ("RISC-V: use memcpy for kexec_file mode")
Acked-by Baoquan He <[email protected]>
Acked-by Pratyush Yadav <[email protected]>
Signed-off-by Tao Liu <[email protected]>

---

v3 -> v2: Add fixes tag; Replace "reference" to "dereference".
link to v2: https://lore.kernel.org/linux-riscv/[email protected]/
link to v1: https://lore.kernel.org/linux-riscv/[email protected]/

---
 arch/riscv/kernel/machine_kexec.c | 7 +++++++
 1 file changed, 7 insertions(+)
arch/riscv/kernel/machine_kexec.c +0 -0
index 2306ce3e5f22..afc68f6a4aa1 100644
@@ -41,6 +41,13 @@ machine_kexec_prepare(struct kimage *image)
              if (image->segment[i].memsz <= sizeof(fdt))
                      continue;
Quoted 8 lines

arch/riscv/kernel/machine_kexec.c machine_kexec_prepare(struct kimage *image) · line 47

Quoted 4 lines
arch/riscv/kernel/machine_kexec.c +4 -0
+             /*
+              * Some segments (e.g. IMA) reserve space but have no buffer
+              * loaded yet. Skip them as they cannot contain an FDT.
+              */

This is destined to rot over time. It also adds up also potentially to the backporting effort while backporting to stable kernes. And most importantly. Please, don't document every other null check.

OK, will get rid of it.

Thanks,
Tao Liu

Quoted 17 lines

arch/riscv/kernel/machine_kexec.c machine_kexec_prepare(struct kimage *image) · line 48

Quoted 2 lines
arch/riscv/kernel/machine_kexec.c +1 -0
+             if (image->segment[i].buf == NULL)

if (!image->segments[i].buf)

Quoted 9 lines
arch/riscv/kernel/machine_kexec.c +2 -1
+                     continue;
+
              if (image->file_mode)
                      memcpy(&fdt, image->segment[i].buf, sizeof(fdt));
              else if (copy_from_user(&fdt, image->segment[i].buf, sizeof(fdt)))
--

2.54.0

BR, Jarkko

maintainer
#4 lore raw
Quoted 69 lines · Tao Liu

Hi Jarkko,

Quoted 11 lines · Jarkko Sakkinen
Quoted 6 lines · Tao Liu

A NULL pointer dereference issue is noticed in riscv's machine_kexec_prepare, where image->segment[i].buf might be NULL and copied unchecked.

The NULL buf comes from security/integrity/ima/ima_kexec.c:
ima_add_kexec_buffer(), where kbuf is added by kexec_add_buffer(), but kbuf.buffer is NULL

This should have a proper call sequence. Now the root cause is obfuscated.

Sure, I will attach the stack trace in v4. Here is the one:

[   62.867540] kexec_file(Image): Loaded kernel at 0x80200000
bufsz=0x34ed800 memsz=0x35d0000
[   62.879983] Unable to handle kernel access to user memory without
uaccess routines at virtual address 0000000000000000
[   62.880736] Current kexec pgtable: 4K pagesize, 57-bit VAs,
pgdp=0x00000001062eb000
[   62.881185] [0000000000000000] pgd=00000000413b4401,
p4d=000000004151bc01, pud=00000000415b7801, pmd=0000000040af5801,
pte=0000000000000000
[   62.881969] Oops [#1]
[   62.882077] Modules linked in:
[   62.882717] CPU: 1 UID: 0 PID: 894 Comm: kexec Not tainted 7.1.1 #4
PREEMPT(lazy)
[   62.883037] Hardware name: QEMU QEMU Virtual Machine, BIOS
edk2-20260508-2.fc44 05/08/2026
[   62.883365] epc : __memcpy+0xd4/0xf8
[   62.883685]  ra : machine_kexec_prepare+0x8a/0x298
[   62.883914] epc : ffffffff81393ee8 ra : ffffffff800366ca sp :
ff20000004a83d10
[   62.884214]  gp : ffffffff83258db8 tp : ff6000008573db80 t0 :
ffffffff80033640
[   62.884433]  t1 : 2152ffffffffffc0 t2 : 0000000003000000 s0 :
ff20000004a83d80
[   62.884710]  s1 : 0000000000000000 a0 : ff20000004a83d10 a1 :
0000000000000000
[   62.884987]  a2 : 0000000000000028 a3 : 0000000000000028 a4 :
0000000000000000
[   62.885208]  a5 : 0000000000000000 a6 : 0000000104e33fff a7 :
0000000000000000
[   62.885486]  s2 : ff60000082a35800 s3 : 0000000000000000 s4 :
0000000000000010
[   62.885774]  s5 : 0000000000000028 s6 : ff20000004a83d10 s7 :
0000000000000005
[   62.886005]  s8 : 00000000000000c0 s9 : 000000000ac0d220 s10:
ffffffff835420e8
[   62.886218]  s11: 0000000000000000 t3 : ffffff800000007c t4 :
ff1c000002138d00
[   62.886515]  t5 : ffffffffffffffff t6 : ff20000004a83d10 ssp :
0000000000000000
[   62.886860] status: 0000000200000120 badaddr: 0000000000000000
cause: 000000000000000d
[   62.887162] [<ffffffff81393ee8>] __memcpy+0xd4/0xf8
[   62.887388] [<ffffffff801b253a>] __do_sys_kexec_file_load+0x1b2/0x338
[   62.887612] [<ffffffff801b26e4>] __riscv_sys_kexec_file_load+0x24/0x40
[   62.887855] [<ffffffff81395ea4>] do_trap_ecall_u+0x1a4/0x5a8
[   62.888134] [<ffffffff813a9eec>] handle_exception+0x16c/0x178
[   62.888445] Code: 7613 07f6 ca05 86b3 00c5 e7b3 01f5 8fd5 8b8d eb89
(4198) 0591
[   62.889223] ---[ end trace 0000000000000000 ]---

Segmentation fault kexec -l /boot/vmlinuz-7.1.1
--initrd=/boot/initramfs-7.1.1.img --reuse-cmdline

Quoted 6 lines

(gdb) p image->segment[0]
$3 = {{buf = 0x0, kbuf = 0x0}, bufsz = 0, mem = 10737586176, memsz = 4096}

The buf = 0x0 and bufsz = 0 comes from ima_add_kexec_buffer(), thought I'm not sure why it added a NULL segment, but it is no harm to add a NULL checker here in case any other scenarios similar as IMA.

The mission oriented purpose of kernel's git log is to be a quick checklist for bisecting bugs for instance. You have a dump of details there for a transcript. Now you should do rationalize that because how can you otherwise trust your own code?

Here's one recent example from me:

https://lore.kernel.org/linux-integrity/[email protected]/

This is IMHO way more important part for a bug fix like this than the code change itself.

It's the "engineering part" of the equation.

Quoted 36 lines
Quoted 27 lines
Quoted 27 lines

Fix this by simply adding a check before copy.

Fixes b7fb4d78a6ad ("RISC-V: use memcpy for kexec_file mode")
Acked-by Baoquan He <[email protected]>
Acked-by Pratyush Yadav <[email protected]>
Signed-off-by Tao Liu <[email protected]>

---

v3 -> v2: Add fixes tag; Replace "reference" to "dereference".
link to v2: https://lore.kernel.org/linux-riscv/[email protected]/
link to v1: https://lore.kernel.org/linux-riscv/[email protected]/

---
 arch/riscv/kernel/machine_kexec.c | 7 +++++++
 1 file changed, 7 insertions(+)
arch/riscv/kernel/machine_kexec.c +0 -0
index 2306ce3e5f22..afc68f6a4aa1 100644
@@ -41,6 +41,13 @@ machine_kexec_prepare(struct kimage *image)
              if (image->segment[i].memsz <= sizeof(fdt))
                      continue;
Quoted 8 lines

arch/riscv/kernel/machine_kexec.c machine_kexec_prepare(struct kimage *image) · line 47

Quoted 4 lines
arch/riscv/kernel/machine_kexec.c +4 -0
+             /*
+              * Some segments (e.g. IMA) reserve space but have no buffer
+              * loaded yet. Skip them as they cannot contain an FDT.
+              */

This is destined to rot over time. It also adds up also potentially to the backporting effort while backporting to stable kernes. And most importantly. Please, don't document every other null check.

Quoted 1 line

OK, will get rid of it.

general rules of thumb i personally follow usually:

1. features/improvements: can be a bit more eager with comments
2. bugs: you really have to have rationale for having a commnet. E.g., mandatory SAFETY comment in linux-rust would obviously qualify.
3. both: if you add a comment are you sure it won't rot over time?

Quoted 22 lines

Thanks,
Tao Liu

Quoted 17 lines

arch/riscv/kernel/machine_kexec.c machine_kexec_prepare(struct kimage *image) · line 48

Quoted 2 lines
arch/riscv/kernel/machine_kexec.c +1 -0
+             if (image->segment[i].buf == NULL)

if (!image->segments[i].buf)

Quoted 9 lines
arch/riscv/kernel/machine_kexec.c +2 -1
+                     continue;
+
              if (image->file_mode)
                      memcpy(&fdt, image->segment[i].buf, sizeof(fdt));
              else if (copy_from_user(&fdt, image->segment[i].buf, sizeof(fdt)))
--

2.54.0

BR, Jarkko

BR, Jarkko

author
#5 lore raw

Hi Jarkko,

Quoted 128 lines · Jarkko Sakkinen
Quoted 69 lines · Tao Liu

Hi Jarkko,

Quoted 11 lines · Jarkko Sakkinen
Quoted 6 lines · Tao Liu

A NULL pointer dereference issue is noticed in riscv's machine_kexec_prepare, where image->segment[i].buf might be NULL and copied unchecked.

The NULL buf comes from security/integrity/ima/ima_kexec.c:
ima_add_kexec_buffer(), where kbuf is added by kexec_add_buffer(), but kbuf.buffer is NULL

This should have a proper call sequence. Now the root cause is obfuscated.

Sure, I will attach the stack trace in v4. Here is the one:

[   62.867540] kexec_file(Image): Loaded kernel at 0x80200000
bufsz=0x34ed800 memsz=0x35d0000
[   62.879983] Unable to handle kernel access to user memory without
uaccess routines at virtual address 0000000000000000
[   62.880736] Current kexec pgtable: 4K pagesize, 57-bit VAs,
pgdp=0x00000001062eb000
[   62.881185] [0000000000000000] pgd=00000000413b4401,
p4d=000000004151bc01, pud=00000000415b7801, pmd=0000000040af5801,
pte=0000000000000000
[   62.881969] Oops [#1]
[   62.882077] Modules linked in:
[   62.882717] CPU: 1 UID: 0 PID: 894 Comm: kexec Not tainted 7.1.1 #4
PREEMPT(lazy)
[   62.883037] Hardware name: QEMU QEMU Virtual Machine, BIOS
edk2-20260508-2.fc44 05/08/2026
[   62.883365] epc : __memcpy+0xd4/0xf8
[   62.883685]  ra : machine_kexec_prepare+0x8a/0x298
[   62.883914] epc : ffffffff81393ee8 ra : ffffffff800366ca sp :
ff20000004a83d10
[   62.884214]  gp : ffffffff83258db8 tp : ff6000008573db80 t0 :
ffffffff80033640
[   62.884433]  t1 : 2152ffffffffffc0 t2 : 0000000003000000 s0 :
ff20000004a83d80
[   62.884710]  s1 : 0000000000000000 a0 : ff20000004a83d10 a1 :
0000000000000000
[   62.884987]  a2 : 0000000000000028 a3 : 0000000000000028 a4 :
0000000000000000
[   62.885208]  a5 : 0000000000000000 a6 : 0000000104e33fff a7 :
0000000000000000
[   62.885486]  s2 : ff60000082a35800 s3 : 0000000000000000 s4 :
0000000000000010
[   62.885774]  s5 : 0000000000000028 s6 : ff20000004a83d10 s7 :
0000000000000005
[   62.886005]  s8 : 00000000000000c0 s9 : 000000000ac0d220 s10:
ffffffff835420e8
[   62.886218]  s11: 0000000000000000 t3 : ffffff800000007c t4 :
ff1c000002138d00
[   62.886515]  t5 : ffffffffffffffff t6 : ff20000004a83d10 ssp :
0000000000000000
[   62.886860] status: 0000000200000120 badaddr: 0000000000000000
cause: 000000000000000d
[   62.887162] [<ffffffff81393ee8>] __memcpy+0xd4/0xf8
[   62.887388] [<ffffffff801b253a>] __do_sys_kexec_file_load+0x1b2/0x338
[   62.887612] [<ffffffff801b26e4>] __riscv_sys_kexec_file_load+0x24/0x40
[   62.887855] [<ffffffff81395ea4>] do_trap_ecall_u+0x1a4/0x5a8
[   62.888134] [<ffffffff813a9eec>] handle_exception+0x16c/0x178
[   62.888445] Code: 7613 07f6 ca05 86b3 00c5 e7b3 01f5 8fd5 8b8d eb89
(4198) 0591
[   62.889223] ---[ end trace 0000000000000000 ]---

Segmentation fault kexec -l /boot/vmlinuz-7.1.1
--initrd=/boot/initramfs-7.1.1.img --reuse-cmdline

Quoted 6 lines

(gdb) p image->segment[0]
$3 = {{buf = 0x0, kbuf = 0x0}, bufsz = 0, mem = 10737586176, memsz = 4096}

The buf = 0x0 and bufsz = 0 comes from ima_add_kexec_buffer(), thought I'm not sure why it added a NULL segment, but it is no harm to add a NULL checker here in case any other scenarios similar as IMA.

The mission oriented purpose of kernel's git log is to be a quick checklist for bisecting bugs for instance. You have a dump of details there for a transcript. Now you should do rationalize that because how can you otherwise trust your own code?

Here's one recent example from me:

https://lore.kernel.org/linux-integrity/[email protected]/

This is IMHO way more important part for a bug fix like this than the code change itself.

It's the "engineering part" of the equation.

Quoted 36 lines
Quoted 35 lines
Quoted 24 lines

Fix this by simply adding a check before copy.

Fixes b7fb4d78a6ad ("RISC-V: use memcpy for kexec_file mode")
Acked-by Baoquan He <[email protected]>
Acked-by Pratyush Yadav <[email protected]>
Signed-off-by Tao Liu <[email protected]>

---

v3 -> v2: Add fixes tag; Replace "reference" to "dereference".
link to v2: https://lore.kernel.org/linux-riscv/[email protected]/
link to v1: https://lore.kernel.org/linux-riscv/[email protected]/

---
 arch/riscv/kernel/machine_kexec.c | 7 +++++++
 1 file changed, 7 insertions(+)
arch/riscv/kernel/machine_kexec.c +0 -0
index 2306ce3e5f22..afc68f6a4aa1 100644

arch/riscv/kernel/machine_kexec.c machine_kexec_prepare(struct kimage *image) · line 47

Quoted 7 lines
arch/riscv/kernel/machine_kexec.c +4 -0
@@ -41,6 +41,13 @@ machine_kexec_prepare(struct kimage *image)
              if (image->segment[i].memsz <= sizeof(fdt))
                      continue;
 
+             /*
+              * Some segments (e.g. IMA) reserve space but have no buffer
+              * loaded yet. Skip them as they cannot contain an FDT.
+              */

This is destined to rot over time. It also adds up also potentially to the backporting effort while backporting to stable kernes. And most importantly. Please, don't document every other null check.

Quoted 9 lines
Quoted 1 line

OK, will get rid of it.

general rules of thumb i personally follow usually:

1. features/improvements: can be a bit more eager with comments
2. bugs: you really have to have rationale for having a commnet. E.g., mandatory SAFETY comment in linux-rust would obviously qualify.
3. both: if you add a comment are you sure it won't rot over time?

Thank you very much for providing detailed guidance on code commenting and commit log drafting. I really appreciate your help with those!

Thanks,
Tao Liu

Quoted 25 lines
Quoted 22 lines

Thanks,
Tao Liu

Quoted 17 lines

arch/riscv/kernel/machine_kexec.c machine_kexec_prepare(struct kimage *image) · line 48

Quoted 2 lines
arch/riscv/kernel/machine_kexec.c +1 -0
+             if (image->segment[i].buf == NULL)

if (!image->segments[i].buf)

Quoted 9 lines
arch/riscv/kernel/machine_kexec.c +2 -1
+                     continue;
+
              if (image->file_mode)
                      memcpy(&fdt, image->segment[i].buf, sizeof(fdt));
              else if (copy_from_user(&fdt, image->segment[i].buf, sizeof(fdt)))
--

2.54.0

BR, Jarkko

BR, Jarkko

maintainer
#6 lore raw
In reply to Tao Liu
Quoted 69 lines · Tao Liu

Hi Jarkko,

Quoted 11 lines · Jarkko Sakkinen
Quoted 6 lines · Tao Liu

A NULL pointer dereference issue is noticed in riscv's machine_kexec_prepare, where image->segment[i].buf might be NULL and copied unchecked.

The NULL buf comes from security/integrity/ima/ima_kexec.c:
ima_add_kexec_buffer(), where kbuf is added by kexec_add_buffer(), but kbuf.buffer is NULL

This should have a proper call sequence. Now the root cause is obfuscated.

Sure, I will attach the stack trace in v4. Here is the one:

[   62.867540] kexec_file(Image): Loaded kernel at 0x80200000
bufsz=0x34ed800 memsz=0x35d0000
[   62.879983] Unable to handle kernel access to user memory without
uaccess routines at virtual address 0000000000000000
[   62.880736] Current kexec pgtable: 4K pagesize, 57-bit VAs,
pgdp=0x00000001062eb000
[   62.881185] [0000000000000000] pgd=00000000413b4401,
p4d=000000004151bc01, pud=00000000415b7801, pmd=0000000040af5801,
pte=0000000000000000
[   62.881969] Oops [#1]
[   62.882077] Modules linked in:
[   62.882717] CPU: 1 UID: 0 PID: 894 Comm: kexec Not tainted 7.1.1 #4
PREEMPT(lazy)
[   62.883037] Hardware name: QEMU QEMU Virtual Machine, BIOS
edk2-20260508-2.fc44 05/08/2026
[   62.883365] epc : __memcpy+0xd4/0xf8
[   62.883685]  ra : machine_kexec_prepare+0x8a/0x298
[   62.883914] epc : ffffffff81393ee8 ra : ffffffff800366ca sp :
ff20000004a83d10
[   62.884214]  gp : ffffffff83258db8 tp : ff6000008573db80 t0 :
ffffffff80033640
[   62.884433]  t1 : 2152ffffffffffc0 t2 : 0000000003000000 s0 :
ff20000004a83d80
[   62.884710]  s1 : 0000000000000000 a0 : ff20000004a83d10 a1 :
0000000000000000
[   62.884987]  a2 : 0000000000000028 a3 : 0000000000000028 a4 :
0000000000000000
[   62.885208]  a5 : 0000000000000000 a6 : 0000000104e33fff a7 :
0000000000000000
[   62.885486]  s2 : ff60000082a35800 s3 : 0000000000000000 s4 :
0000000000000010
[   62.885774]  s5 : 0000000000000028 s6 : ff20000004a83d10 s7 :
0000000000000005
[   62.886005]  s8 : 00000000000000c0 s9 : 000000000ac0d220 s10:
ffffffff835420e8
[   62.886218]  s11: 0000000000000000 t3 : ffffff800000007c t4 :
ff1c000002138d00
[   62.886515]  t5 : ffffffffffffffff t6 : ff20000004a83d10 ssp :
0000000000000000
[   62.886860] status: 0000000200000120 badaddr: 0000000000000000
cause: 000000000000000d
[   62.887162] [<ffffffff81393ee8>] __memcpy+0xd4/0xf8
[   62.887388] [<ffffffff801b253a>] __do_sys_kexec_file_load+0x1b2/0x338
[   62.887612] [<ffffffff801b26e4>] __riscv_sys_kexec_file_load+0x24/0x40
[   62.887855] [<ffffffff81395ea4>] do_trap_ecall_u+0x1a4/0x5a8
[   62.888134] [<ffffffff813a9eec>] handle_exception+0x16c/0x178
[   62.888445] Code: 7613 07f6 ca05 86b3 00c5 e7b3 01f5 8fd5 8b8d eb89
(4198) 0591
[   62.889223] ---[ end trace 0000000000000000 ]---

Segmentation fault kexec -l /boot/vmlinuz-7.1.1
--initrd=/boot/initramfs-7.1.1.img --reuse-cmdline

Quoted 6 lines

(gdb) p image->segment[0]
$3 = {{buf = 0x0, kbuf = 0x0}, bufsz = 0, mem = 10737586176, memsz = 4096}

The buf = 0x0 and bufsz = 0 comes from ima_add_kexec_buffer(), thought I'm not sure why it added a NULL segment, but it is no harm to add a NULL checker here in case any other scenarios similar as IMA.

From my reading of the IMA code, I think they do so because they don't want to add a buffer to the kimage just yet. This is because there can be IMA measurements between kexec load and kexec reboot. So they use kexec_add_buffer() to reserve the physical address space, then they vmap the destination pages in ima_kexec_post_load(), and then update the destination pages directly via a reboot notifier. So they do not have a buffer copied to the destination pages at the time of kexec_add_buffer().

Now all that is fairly convoluted and not very obvious, but I am not sure if there is a better way of doing this off the top of my head.

Quoted 9 lines
Quoted 9 lines
Quoted 9 lines

Fix this by simply adding a check before copy.

Fixes b7fb4d78a6ad ("RISC-V: use memcpy for kexec_file mode")
Acked-by Baoquan He <[email protected]>
Acked-by Pratyush Yadav <[email protected]>
Signed-off-by Tao Liu <[email protected]>

[...]

Signature · 2 lines
Regards,
Pratyush Yadav
author
#7 lore raw

Hi Pratyush,

Quoted 88 lines · Pratyush Yadav
Quoted 69 lines · Tao Liu

Hi Jarkko,

Quoted 11 lines · Jarkko Sakkinen
Quoted 6 lines · Tao Liu

A NULL pointer dereference issue is noticed in riscv's machine_kexec_prepare, where image->segment[i].buf might be NULL and copied unchecked.

The NULL buf comes from security/integrity/ima/ima_kexec.c:
ima_add_kexec_buffer(), where kbuf is added by kexec_add_buffer(), but kbuf.buffer is NULL

This should have a proper call sequence. Now the root cause is obfuscated.

Sure, I will attach the stack trace in v4. Here is the one:

[   62.867540] kexec_file(Image): Loaded kernel at 0x80200000
bufsz=0x34ed800 memsz=0x35d0000
[   62.879983] Unable to handle kernel access to user memory without
uaccess routines at virtual address 0000000000000000
[   62.880736] Current kexec pgtable: 4K pagesize, 57-bit VAs,
pgdp=0x00000001062eb000
[   62.881185] [0000000000000000] pgd=00000000413b4401,
p4d=000000004151bc01, pud=00000000415b7801, pmd=0000000040af5801,
pte=0000000000000000
[   62.881969] Oops [#1]
[   62.882077] Modules linked in:
[   62.882717] CPU: 1 UID: 0 PID: 894 Comm: kexec Not tainted 7.1.1 #4
PREEMPT(lazy)
[   62.883037] Hardware name: QEMU QEMU Virtual Machine, BIOS
edk2-20260508-2.fc44 05/08/2026
[   62.883365] epc : __memcpy+0xd4/0xf8
[   62.883685]  ra : machine_kexec_prepare+0x8a/0x298
[   62.883914] epc : ffffffff81393ee8 ra : ffffffff800366ca sp :
ff20000004a83d10
[   62.884214]  gp : ffffffff83258db8 tp : ff6000008573db80 t0 :
ffffffff80033640
[   62.884433]  t1 : 2152ffffffffffc0 t2 : 0000000003000000 s0 :
ff20000004a83d80
[   62.884710]  s1 : 0000000000000000 a0 : ff20000004a83d10 a1 :
0000000000000000
[   62.884987]  a2 : 0000000000000028 a3 : 0000000000000028 a4 :
0000000000000000
[   62.885208]  a5 : 0000000000000000 a6 : 0000000104e33fff a7 :
0000000000000000
[   62.885486]  s2 : ff60000082a35800 s3 : 0000000000000000 s4 :
0000000000000010
[   62.885774]  s5 : 0000000000000028 s6 : ff20000004a83d10 s7 :
0000000000000005
[   62.886005]  s8 : 00000000000000c0 s9 : 000000000ac0d220 s10:
ffffffff835420e8
[   62.886218]  s11: 0000000000000000 t3 : ffffff800000007c t4 :
ff1c000002138d00
[   62.886515]  t5 : ffffffffffffffff t6 : ff20000004a83d10 ssp :
0000000000000000
[   62.886860] status: 0000000200000120 badaddr: 0000000000000000
cause: 000000000000000d
[   62.887162] [<ffffffff81393ee8>] __memcpy+0xd4/0xf8
[   62.887388] [<ffffffff801b253a>] __do_sys_kexec_file_load+0x1b2/0x338
[   62.887612] [<ffffffff801b26e4>] __riscv_sys_kexec_file_load+0x24/0x40
[   62.887855] [<ffffffff81395ea4>] do_trap_ecall_u+0x1a4/0x5a8
[   62.888134] [<ffffffff813a9eec>] handle_exception+0x16c/0x178
[   62.888445] Code: 7613 07f6 ca05 86b3 00c5 e7b3 01f5 8fd5 8b8d eb89
(4198) 0591
[   62.889223] ---[ end trace 0000000000000000 ]---

Segmentation fault kexec -l /boot/vmlinuz-7.1.1
--initrd=/boot/initramfs-7.1.1.img --reuse-cmdline

Quoted 6 lines

(gdb) p image->segment[0]
$3 = {{buf = 0x0, kbuf = 0x0}, bufsz = 0, mem = 10737586176, memsz = 4096}

The buf = 0x0 and bufsz = 0 comes from ima_add_kexec_buffer(), thought I'm not sure why it added a NULL segment, but it is no harm to add a NULL checker here in case any other scenarios similar as IMA.

From my reading of the IMA code, I think they do so because they don't want to add a buffer to the kimage just yet. This is because there can be IMA measurements between kexec load and kexec reboot. So they use kexec_add_buffer() to reserve the physical address space, then they vmap the destination pages in ima_kexec_post_load(), and then update the destination pages directly via a reboot notifier. So they do not have a buffer copied to the destination pages at the time of kexec_add_buffer().

Quoted 2 lines

Now all that is fairly convoluted and not very obvious, but I am not sure if there is a better way of doing this off the top of my head.

Thank you very much for resolving my confusion! Now I understand it much better...

Thanks,
Tao Liu

Quoted 16 lines
Quoted 10 lines
Quoted 10 lines
Quoted 10 lines

Fix this by simply adding a check before copy.

Fixes b7fb4d78a6ad ("RISC-V: use memcpy for kexec_file mode")
Acked-by Baoquan He <[email protected]>
Acked-by Pratyush Yadav <[email protected]>
Signed-off-by Tao Liu <[email protected]>

[...]

--
Regards,
Pratyush Yadav

author
#9 lore raw

Hi Markus,

Quoted 8 lines · Markus Elfring

…

Quoted 1 line

Fix this by simply adding a check before copy.

a data copy attempt?

Quoted 8 lines
Quoted 1 line
Fixes b7fb4d78a6ad ("RISC-V: use memcpy for kexec_file mode")

See also once more:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/stable-kernel-rules.rst?h=v7.2-rc1#n34

Would a summary phrase like “Prevent null pointer dereference in machine_kexec_prepare()” be nicer?

Agreed, I will improve it in v4. Thanks again for your guidance!

Thanks,
Tao Liu

Quoted 4 lines

Regards,
Markus