libdwfl: Add some extra space to buffer to read kernel image header

Message ID 20240121195439.478132-1-mark@klomp.org
State Committed
Delegated to: Aaron Merey
Headers
Series libdwfl: Add some extra space to buffer to read kernel image header |

Commit Message

Mark Wielaard Jan. 21, 2024, 7:54 p.m. UTC
  GCC 14 notices we play some tricks with the array into which we try
to read the kernel image header.

image-header.c: In function ‘__libdw_image_header’:
image-header.c:77:18: error: array subscript -496 is outside array bounds of ‘char[96]’ [-Werror=array-bounds=]
   77 |           header = header_buffer - H_START;
      |                  ^
image-header.c:67:12: note: at offset -496 into object ‘header_buffer’ of size 96
   67 |       char header_buffer[H_READ_SIZE];
      |            ^~~~~~~~~~~~~

GCC is correct. The new header pointer is before the actually buffer we
want to read from. Later in the code we "correct" the address again by
adding the "offset" off the elements we want to read. Such pointer
arithmetic is technically invalid. Make it valid by making the buffer
a little bigger, so all pointer arithmetic stays inside the header_buffer.
This does waste 496 bytes on the stack at the front of the buffer that
is never used.

	* libdwfl/image-header.c (__libdw_image_header): Add H_START
	to header_buffer size and return

Signed-off-by: Mark Wielaard <mark@klomp.org>
---
 libdwfl/image-header.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
  

Comments

Mark Wielaard Jan. 30, 2024, 3:37 p.m. UTC | #1
Hi,

On Sun, 2024-01-21 at 20:54 +0100, Mark Wielaard wrote:
> GCC 14 notices we play some tricks with the array into which we try
> to read the kernel image header.
> 
> image-header.c: In function ‘__libdw_image_header’:
> image-header.c:77:18: error: array subscript -496 is outside array bounds of ‘char[96]’ [-Werror=array-bounds=]
>    77 |           header = header_buffer - H_START;
>       |                  ^
> image-header.c:67:12: note: at offset -496 into object ‘header_buffer’ of size 96
>    67 |       char header_buffer[H_READ_SIZE];
>       |            ^~~~~~~~~~~~~
> 
> GCC is correct. The new header pointer is before the actually buffer we
> want to read from. Later in the code we "correct" the address again by
> adding the "offset" off the elements we want to read. Such pointer
> arithmetic is technically invalid. Make it valid by making the buffer
> a little bigger, so all pointer arithmetic stays inside the header_buffer.
> This does waste 496 bytes on the stack at the front of the buffer that
> is never used.
> 
> 	* libdwfl/image-header.c (__libdw_image_header): Add H_START
> 	to header_buffer size and return

Pushed after briefly discussing with Aaron on irc.

Cheers,

Mark
  

Patch

diff --git a/libdwfl/image-header.c b/libdwfl/image-header.c
index c777cc84..03cd7a76 100644
--- a/libdwfl/image-header.c
+++ b/libdwfl/image-header.c
@@ -1,6 +1,6 @@ 
 /* Linux kernel image support for libdwfl.
    Copyright (C) 2009-2011 Red Hat, Inc.
-   Copyright (C) 2022 Mark J. Wielaard <mark@klomp.org>
+   Copyright (C) 2022, 2024 Mark J. Wielaard <mark@klomp.org>
    This file is part of elfutils.
 
    This file is free software; you can redistribute it and/or modify
@@ -64,17 +64,17 @@  __libdw_image_header (int fd, off_t *start_offset,
   if (likely (mapped_size > H_END))
     {
       const void *header = mapped;
-      char header_buffer[H_READ_SIZE];
+      char header_buffer[H_READ_SIZE + H_START];
       if (header == NULL)
 	{
-	  ssize_t n = pread_retry (fd, header_buffer, H_READ_SIZE,
+	  ssize_t n = pread_retry (fd, header_buffer + H_START, H_READ_SIZE,
 				   *start_offset + H_START);
 	  if (n < 0)
 	    return DWFL_E_ERRNO;
 	  if (n < H_READ_SIZE)
 	    return DWFL_E_BADELF;
 
-	  header = header_buffer - H_START;
+	  header = header_buffer;
 	}
 
       uint16_t magic1;