Hi Mark,
On Mon, Apr 13, 2026 at 8:40 AM Mark Wielaard <[email protected]> wrote:
>
> If d_align isn't explicitly set in elf_newdata it will default to
> zero. This confuses elf_update when more than one Elf_Data has been
> added to a (new) Elf_Section. Only the last will appear to be added.
> ELF_T_BYTE happens to be zero and so d_type did appear to be setup
> correctly. Also set it explicitly to show the defaults.
>
> The elf_newdata test happened to pass because it set d_align to 1
> explicitly. But would fail if it wouldn't set it. Add some extra
> checks.
>
> Also update the elf_newdata man page with the new defaults.
>
> * libelf/elf_newdata.c (elf_newdata): Explicitly set d_type
> and d_align.
> * tests/newdata.c (add_section_data): Check d_buf, d_size,
> d_type, d_align and d_version default values.
> * doc/elf_newdata.3: Document d_align defaults to one.
>
> Signed-off-by: Mark Wielaard <[email protected]>
LGTM.
Aaron
> ---
> doc/elf_newdata.3 | 5 ++---
> libelf/elf_newdata.c | 2 ++
> tests/newdata.c | 33 ++++++++++++++++++++++++++++++---
> 3 files changed, 34 insertions(+), 6 deletions(-)
>
> diff --git a/doc/elf_newdata.3 b/doc/elf_newdata.3
> index 0522bc15c235..1530dfb7fbcd 100644
> --- a/doc/elf_newdata.3
> +++ b/doc/elf_newdata.3
> @@ -42,12 +42,11 @@ set to
> .I d_version
> set to
> .BR EV_CURRENT ,
> -and
> .IR d_size ,
> .IR d_off ,
> -and
> +set to zero, and
> .IR d_align
> -set to zero.
> +set to one.
>
> .SH PARAMETERS
> .TP
> diff --git a/libelf/elf_newdata.c b/libelf/elf_newdata.c
> index 0063d599180f..b59b87ecfc0a 100644
> --- a/libelf/elf_newdata.c
> +++ b/libelf/elf_newdata.c
> @@ -118,6 +118,8 @@ elf_newdata (Elf_Scn *scn)
>
> /* Set the predefined values. */
> result->data.d.d_version = EV_CURRENT;
> + result->data.d.d_type = ELF_T_BYTE;
> + result->data.d.d_align = 1;
>
> result->data.s = scn;
>
> diff --git a/tests/newdata.c b/tests/newdata.c
> index fcf26acf8cb9..5b7da8d61b14 100644
> --- a/tests/newdata.c
> +++ b/tests/newdata.c
> @@ -54,11 +54,38 @@ add_section_data (Elf *elf, char *buf, size_t len)
> exit (1);
> }
>
> + if (data->d_buf != NULL)
> + {
> + printf ("newdata d_buf isn't NULL\n");
> + exit (1);
> + }
> +
> + if (data->d_size != 0)
> + {
> + printf ("newdata d_size isn't 0\n");
> + exit (1);
> + }
> +
> + if (data->d_type != ELF_T_BYTE)
> + {
> + printf ("newdata d_type isn't ELF_T_BYTE\n");
> + exit (1);
> + }
> +
> + if (data->d_align != 1)
> + {
> + printf ("newdata d_align isn't 1\n");
> + exit (1);
> + }
> +
> + if (data->d_version != EV_CURRENT)
> + {
> + printf ("newdata d_version isn't EV_CURRENT\n");
> + exit (1);
> + }
> +
> data->d_buf = buf;
> - data->d_type = ELF_T_BYTE;
> data->d_size = len;
> - data->d_align = 1;
> - data->d_version = EV_CURRENT;
>
> // Let the library compute the internal structure information.
> if (elf_update (elf, ELF_C_NULL) < 0)
> --
> 2.53.0
>
