diff mbox series

[kvmtool] riscv: Pass correct size to snprintf()

Message ID 20241104192120.75841-1-bjorn@kernel.org
State Accepted
Headers show
Series [kvmtool] riscv: Pass correct size to snprintf() | expand

Commit Message

Björn Töpel Nov. 4, 2024, 7:21 p.m. UTC
From: Björn Töpel <bjorn@rivosinc.com>

The snprintf() function does not get the correct size argument passed,
when the FDT ISA string is built. Instead of adjusting the size for
each extension, the full size is passed for every iteration. Doing so
will make __snprinf_chk() bail out on glibc.

Adjust size for each iteration.

Fixes: 8aff29e1dafe ("riscv: Append ISA extensions to the device tree")
Signed-off-by: Björn Töpel <bjorn@rivosinc.com>
---
 riscv/fdt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


base-commit: 3040b298156e4e2a82b27ac8db5bd63a72b3785b

Comments

Andrew Jones Nov. 5, 2024, 9:14 a.m. UTC | #1
On Mon, Nov 04, 2024 at 08:21:19PM +0100, Björn Töpel wrote:
> From: Björn Töpel <bjorn@rivosinc.com>
> 
> The snprintf() function does not get the correct size argument passed,
> when the FDT ISA string is built. Instead of adjusting the size for
> each extension, the full size is passed for every iteration. Doing so
> will make __snprinf_chk() bail out on glibc.
> 
> Adjust size for each iteration.
> 
> Fixes: 8aff29e1dafe ("riscv: Append ISA extensions to the device tree")
> Signed-off-by: Björn Töpel <bjorn@rivosinc.com>
> ---
>  riscv/fdt.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/riscv/fdt.c b/riscv/fdt.c
> index 8189601f46de..85c8f95604f6 100644
> --- a/riscv/fdt.c
> +++ b/riscv/fdt.c
> @@ -157,7 +157,7 @@ static void generate_cpu_nodes(void *fdt, struct kvm *kvm)
>  					   isa_info_arr[i].name);
>  				break;
>  			}
> -			pos += snprintf(cpu_isa + pos, CPU_ISA_MAX_LEN, "_%s",
> +			pos += snprintf(cpu_isa + pos, CPU_ISA_MAX_LEN - pos, "_%s",

Just above this we confirm strlen(isa_info_arr[i].name) + pos + 1 is less
than CPU_ISA_MAX_LEN, which means we should be able to use anything for
size which is greater than or equal to strlen(isa_info_arr[i].name) + 1,
as snprintf won't write more anyway. But, it's understandable that
__snprinf_chk doesn't know that.

>  					isa_info_arr[i].name);
>  		}
>  		cpu_isa[pos] = '\0';

Not part of this patch, but part of the context, so I'll comment on it
anyway, this '\0' assignment could be removed. It looks like it's a left
over from commit 7c9aac003925 ("riscv: Generate FDT at runtime for
Guest/VM") which could have been removed with commit 8aff29e1dafe ("riscv:
Append ISA extensions to the device tree")

Anyway,

Reviewed-by: Andrew Jones <ajones@ventanamicro.com>

Thanks,
drew
Will Deacon Nov. 11, 2024, 5:10 p.m. UTC | #2
On Mon, 04 Nov 2024 20:21:19 +0100, Björn Töpel wrote:
> The snprintf() function does not get the correct size argument passed,
> when the FDT ISA string is built. Instead of adjusting the size for
> each extension, the full size is passed for every iteration. Doing so
> will make __snprinf_chk() bail out on glibc.
> 
> Adjust size for each iteration.
> 
> [...]

Applied to kvmtool (master), thanks!

[1/1] riscv: Pass correct size to snprintf()
      https://git.kernel.org/will/kvmtool/c/574bd7b432ec

Cheers,
diff mbox series

Patch

diff --git a/riscv/fdt.c b/riscv/fdt.c
index 8189601f46de..85c8f95604f6 100644
--- a/riscv/fdt.c
+++ b/riscv/fdt.c
@@ -157,7 +157,7 @@  static void generate_cpu_nodes(void *fdt, struct kvm *kvm)
 					   isa_info_arr[i].name);
 				break;
 			}
-			pos += snprintf(cpu_isa + pos, CPU_ISA_MAX_LEN, "_%s",
+			pos += snprintf(cpu_isa + pos, CPU_ISA_MAX_LEN - pos, "_%s",
 					isa_info_arr[i].name);
 		}
 		cpu_isa[pos] = '\0';