Re: [PATCH] libdwfl: fix DEREF_OF_NULL.EX in dwfl_segment_report_module.c
Hi Anton,
On Sat, Feb 01, 2025 at 02:10:03AM +0300, Anton Moryakov wrote:
> Report of the static analyzer:
> After having been assigned to a NULL value at
> dwfl_segment_report_module.c:187, pointer 'retval' is
> dereferenced at dwfl_segment_report_module.c:195 by
> calling function 'strcmp'. (CWE476)
These line numbers seem off. Which version of the file are you
checking against?
> Corrections explained:
> When processing file notes, the code could dereference
> a NULL pointer if 'retval' was not initialized. This patch
> adds a check to ensure 'retval' is not NULL before using it
> in strcmp.
>
> The fix ensures that the function safely handles cases where
> 'retval' is NULL, avoiding potential crashes.
Isn't this the same as we discussed before?
https://inbox.sourceware.org/elfutils-devel/fafbecf35ed2545ecd161dde1c5bbb4c1b4961b6.ca...@klomp.org/
and
https://inbox.sourceware.org/elfutils-devel/20240702111528.ga29...@gnu.wildebeest.org/
Was the analyzis in the above messages incorrect?
Thanks,
Mark
> Triggers found by static analyzer Svace.
>
> Signed-off-by: Anton Moryakov
>
> ---
> libdwfl/dwfl_segment_report_module.c | 7 +--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/libdwfl/dwfl_segment_report_module.c
> b/libdwfl/dwfl_segment_report_module.c
> index 32f44af8..565884f0 100644
> --- a/libdwfl/dwfl_segment_report_module.c
> +++ b/libdwfl/dwfl_segment_report_module.c
> @@ -205,8 +205,11 @@ handle_file_note (GElf_Addr module_start, GElf_Addr
> module_end,
> return NULL;
>if (mix == firstix)
> retval = fptr;
> - if (firstix < mix && mix <= lastix && strcmp (fptr, retval) != 0)
> - return NULL;
> + if (firstix < mix && mix <= lastix)
> + {
> +if (retval == NULL || strcmp(fptr, retval) != 0)
> + return NULL;
> + }
>fptr = fnext + 1;
> }
>return retval;
> --
> 2.30.2
>
Re: [PATCH 2/5] libdwfl/offline.c: Avoid closing invalid fd
Hi Aaron, On Thu, Jan 30, 2025 at 09:35:51PM -0500, Aaron Merey wrote: > process_archive may be called with an fd argument of -1, which > libelf interprets as "no file opened". However when closing > the fd process_archive does not check whether the fd is valid > and may attempt to close an fd of -1. Nice find. Less syscalls (that do nothing/just error) is always better. I was puzzled for a moment how this could happen. But there is a comment in offline.c already that explains: /* It is ok to pass fd == -1 here, because libelf uses it as a value for "no file opened" and supports working with files without fd, thanks to the existence of the elf_memory function. */ Could you apply this patch before enabling valgrind --track-fds-yes? Thanks, Mark > Signed-off-by: Aaron Merey > --- > libdwfl/offline.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/libdwfl/offline.c b/libdwfl/offline.c > index 24e9e180..dc099d2b 100644 > --- a/libdwfl/offline.c > +++ b/libdwfl/offline.c > @@ -271,7 +271,8 @@ process_archive (Dwfl *dwfl, const char *name, const char > *file_name, int fd, > zero, that module will close FD. If no modules survived the predicate, > we are all done with the file right here. */ >if (mod != NULL/* If no modules, caller will clean up. */ > - && elf_end (archive) == 0) > + && elf_end (archive) == 0 > + && fd >= 0) > close (fd); > >return mod; > -- > 2.48.1 >
Re: [PATCH 5/5] tests: Avoid leaking file descriptors
Hi Aaron,
On Thu, Jan 30, 2025 at 09:35:54PM -0500, Aaron Merey wrote:
> Add calls to close for all test programs that leak file descriptors
> in order to prevent test failures when run under valgrind
> --track-fds=yes.
These all look correct to me.
Could you commit this before enabling valgrind --track-fds=yes?
Thanks,
Mark
> Signed-off-by: Aaron Merey
> ---
> tests/all-dwarf-ranges.c| 2 ++
> tests/alldts.c | 1 +
> tests/dwarf-getmacros.c | 3 ++-
> tests/dwarf-ranges.c| 3 ++-
> tests/dwarfcfi.c| 1 +
> tests/dwfl-core-noncontig.c | 2 ++
> tests/early-offscn.c| 1 +
> tests/ecp.c | 1 +
> tests/newfile.c | 1 +
> tests/rerequest_tag.c | 2 ++
> tests/test-flag-nobits.c| 2 ++
> tests/update1.c | 1 +
> tests/update2.c | 1 +
> tests/update3.c | 1 +
> tests/update4.c | 1 +
> 15 files changed, 21 insertions(+), 2 deletions(-)
>
> diff --git a/tests/all-dwarf-ranges.c b/tests/all-dwarf-ranges.c
> index 4331a05b..5924c0ff 100644
> --- a/tests/all-dwarf-ranges.c
> +++ b/tests/all-dwarf-ranges.c
> @@ -26,6 +26,7 @@
> #include
> #include
> #include
> +#include
>
> static void
> ranges_die (Dwarf_Die *die)
> @@ -85,6 +86,7 @@ main (int argc, char *argv[])
>walk_tree (&die);
> }
>dwarf_end (dbg);
> + close (fd);
>
>return 0;
> }
> diff --git a/tests/alldts.c b/tests/alldts.c
> index d0fe4f24..cd12c14d 100644
> --- a/tests/alldts.c
> +++ b/tests/alldts.c
> @@ -268,5 +268,6 @@ main (void)
>return 1;
> }
>
> + close (fd);
>return 0;
> }
> diff --git a/tests/dwarf-getmacros.c b/tests/dwarf-getmacros.c
> index 8381d42c..a9f90ee4 100644
> --- a/tests/dwarf-getmacros.c
> +++ b/tests/dwarf-getmacros.c
> @@ -26,6 +26,7 @@
> #include
> #include
> #include
> +#include
>
> static void include (Dwarf *dbg, Dwarf_Off macoff, ptrdiff_t token);
>
> @@ -174,6 +175,6 @@ main (int argc, char *argv[])
> }
>
>dwarf_end (dbg);
> -
> + close (fd);
>return 0;
> }
> diff --git a/tests/dwarf-ranges.c b/tests/dwarf-ranges.c
> index 4bcf96ce..e111a608 100644
> --- a/tests/dwarf-ranges.c
> +++ b/tests/dwarf-ranges.c
> @@ -26,6 +26,7 @@
> #include
> #include
> #include
> +#include
>
> int
> main (int argc, char *argv[])
> @@ -52,6 +53,6 @@ main (int argc, char *argv[])
> start, end, base);
>
>dwarf_end (dbg);
> -
> + close (fd);
>return 0;
> }
> diff --git a/tests/dwarfcfi.c b/tests/dwarfcfi.c
> index 29849e71..5f25228d 100644
> --- a/tests/dwarfcfi.c
> +++ b/tests/dwarfcfi.c
> @@ -170,6 +170,7 @@ main (int argc, char *argv[])
>
>dwarf_end (dwarf);
>elf_end (elf);
> + close (fd);
>
>return result;
> }
> diff --git a/tests/dwfl-core-noncontig.c b/tests/dwfl-core-noncontig.c
> index 04558e28..f4170206 100644
> --- a/tests/dwfl-core-noncontig.c
> +++ b/tests/dwfl-core-noncontig.c
> @@ -21,6 +21,7 @@
> #include
> #include ELFUTILS_HEADER(dwfl)
> #include ELFUTILS_HEADER(elf)
> +#include
>
> static const Dwfl_Callbacks cb =
> {
> @@ -77,6 +78,7 @@ main (int argc, char **argv)
>
>dwfl_end (dwfl);
>elf_end (elf);
> + close (fd);
>
>return 0;
> }
> diff --git a/tests/early-offscn.c b/tests/early-offscn.c
> index af29da5a..9ebba29c 100644
> --- a/tests/early-offscn.c
> +++ b/tests/early-offscn.c
> @@ -48,5 +48,6 @@ main (int argc, char *argv[])
> error (3, 0, "gelf_offscn: %s", elf_errmsg (-1));
>
>elf_end (elf);
> + close (fd);
>return 0;
> }
> diff --git a/tests/ecp.c b/tests/ecp.c
> index 44a7bda2..eb16eb4a 100644
> --- a/tests/ecp.c
> +++ b/tests/ecp.c
> @@ -94,6 +94,7 @@ main (int argc, char *argv[])
>close (outfd);
>
>elf_end (inelf);
> + close (infd);
>
>return 0;
> }
> diff --git a/tests/newfile.c b/tests/newfile.c
> index 5eabdcb7..be3bd42a 100644
> --- a/tests/newfile.c
> +++ b/tests/newfile.c
> @@ -166,5 +166,6 @@ main (int argc, char *argv[] __attribute__ ((unused)))
>(void) elf_end (elf);
> }
>
> + close (fd);
>return result;
> }
> diff --git a/tests/rerequest_tag.c b/tests/rerequest_tag.c
> index b4d46271..058b8c49 100644
> --- a/tests/rerequest_tag.c
> +++ b/tests/rerequest_tag.c
> @@ -21,6 +21,7 @@
> #include
> #include
> #include
> +#include
>
> int
> main (int argc, char **argv)
> @@ -43,5 +44,6 @@ main (int argc, char **argv)
>assert (dwarf_tag (die) == 0);
>
>dwarf_end (dw);
> + close (i);
>return 0;
> }
> diff --git a/tests/test-flag-nobits.c b/tests/test-flag-nobits.c
> index 15d44ea8..c6658d9f 100644
> --- a/tests/test-flag-nobits.c
> +++ b/tests/test-flag-nobits.c
> @@ -21,6 +21,7 @@
> #include
> #include
> #include
> +#include
>
> int
> main (int argc, char **argv)
> @@ -38,5 +39,6 @@ main (int argc, char **argv)
> elf_flagdata (elf_getdata (scn, NULL), ELF_C_SET, ELF_F_DIRTY);
>
>elf_end (stripped);
> + close (f
Re: [PATCH] src: fix DEREF_AFTER_NULL.EX in elflint.c
Hi Anton,
On Sat, Feb 01, 2025 at 01:43:44AM +0300, Anton Moryakov wrote:
> Report of the static analyzer:
> After having been compared to a NULL value at
> elflint.c:252, pointer 'suffix' is dereferenced at elflint.c:260
> by calling function 'stpcpy'
>
> Corrections explained:
> When processing a file with a NULL suffix, the code could dereference
> a NULL pointer, leading to undefined behavior. This patch adds a check
> to ensure suffix is not NULL before using it in stpcpy.
>
> The fix ensures that new_suffix is properly initialized even when
> suffix is NULL, avoiding potential crashes.
>
> Triggers found by static analyzer Svace.
Thanks, this analyzis is correct locally in this process_file
function.
But in practice this (static) function is called either with both
prefix and suffix as initialized (stack) pointers:
char new_prefix[prefix_len + 1 + fname_len];
char new_suffix[(suffix == NULL ? 0 : strlen (suffix)) + 2];
[...]
process_file (fd, subelf, new_prefix, new_suffix,
arhdr->ar_name, arhdr->ar_size, false);
Or with both prefix and suffix NULL:
process_file (fd, elf, NULL, NULL, argv[remaining], st.st_size,
only_one);
So the code path where prefix != NULL also implies suffix != NULL.
Maybe the code needs a comment, check or assert to verify this?
Cheers,
Mark
> Signed-off-by: Anton Moryakov
>
> ---
> src/elflint.c | 5 -
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/src/elflint.c b/src/elflint.c
> index cdc6108d..fba18f5a 100644
> --- a/src/elflint.c
> +++ b/src/elflint.c
> @@ -257,7 +257,10 @@ process_file (int fd, Elf *elf, const char *prefix,
> const char *suffix,
> {
> cp = mempcpy (cp, prefix, prefix_len);
> *cp++ = '(';
> - strcpy (stpcpy (new_suffix, suffix), ")");
> + if(suffix != NULL)
> + strcpy (stpcpy (new_suffix, suffix), ")");
> + else
> + new_suffix[0] = '\0';
> }
> else
> new_suffix[0] = '\0';
> --
> 2.30.2
>
[Bug debuginfod/32629] add setter functions to debuginfod_client object as alternative to getenv/setenv
https://sourceware.org/bugzilla/show_bug.cgi?id=32629 --- Comment #3 from Mark Wielaard --- Aha, I see what you mean. I assumed this was just for DEBUGINFOD_URLS. But there are some others that don't have setter functions atm. DEBUGINFOD_CACHE_PATH, DEBUGINFOD_TIMEOUT, DEBUGINFOD_PROGRESS, DEBUGINFOD_VERBOSE, DEBUGINFOD_RETRY_LIMIT, DEBUGINFOD_MAXSIZE, DEBUGINFOD_MAXTIME and DEBUGINFOD_IMA_CERT_PATH. I am not sure any of them, except maybe DEBUGINFOD_VERBOSE, should be "reset" during the life time of the object. If you don't like the debuginfod_client handle you created, you just destroy it and create a new one with different settings. Changing them during usage feels confusing. It makes sense imho, specifically for DEBUGINFOD_URLS, DEBUGINFOD_CACHE_PATH and DEBUGINFOD_IMA_CERT_PATH to be fixed. It allows the implementation to keep the connection to the server, the cache dir and the cert files open for easy/fast reuse. -- You are receiving this mail because: You are on the CC list for the bug.
Re: [PATCH 10/10 v5] Add tests/run-eu-search-die.sh
On Fri, Jan 31, 2025 at 07:50:16PM -0500, Aaron Merey wrote: > * tests/.gitignore: Add eu_search_die. > * tests/Makefile.am: Add eu_search_die, run-eu-search-die.sh. > * tests/eu_search_die.c: New file. > * tests/run-eu-search-die.sh: New file. > > Signed-off-by: Heather S. McIntyre > Signed-off-by: Aaron Merey > Signed-off-by: Mark Wielaard > --- > v5 changes: pthread added to eu_search_die_LDFLAGS unconditionally. Looks good. Thanks, Mark
Re: [PATCH 07/10 v5] Add tests/run-eu-search-cfi.sh
[Now added elfutils-devel@ to CC, which was the problem noted below.] Hi Aaron, For some reason I only received patches 07 to 10 in this series. And I don't see any of them on the mailinglist. Did something went wrong somewhere? The list is on CC. On Fri, Jan 31, 2025 at 07:50:13PM -0500, Aaron Merey wrote: > From: Heather McIntyre > > * tests/.gitignore: Add eu_search_cfi. > * tests/Makefile.am: Add eu_search_cfi, run-eu-search-cfi.sh. > * tests/eu_search_cfi.c: New file. > * tests/run-eu-search-cfi.sh: New file. > > Signed-off-by: Heather S. McIntyre > Signed-off-by: Aaron Merey > Signed-off-by: Mark Wielaard > --- > v5 changes: pthread added to eu_search_cfi_LDFLAGS unconditionally. This looks good. Thanks, Mark
Re: [PATCH 08/10 v5] Add tests/run-eu-search-macros.sh
[Adding elfutils-devel@ to CC.] Hi Aaron, On Fri, Jan 31, 2025 at 07:50:14PM -0500, Aaron Merey wrote: > * tests/.gitignore: Add eu_search_macros.sh > * tests/Makefile.am: Add eu_search_macros, > run-eu-search-macros.sh. > * tests/eu_search_macros.c: New file. > * tests/run-eu-search-macros.sh: New file. > > Signed-off-by: Heather S. McIntyre > Signed-off-by: Aaron Merey > Signed-off-by: Mark Wielaard > --- > v5 changes: pthread added to eu_search_macros_LDFLAGS unconditionally. > Add missing error exit to thread_work. Looks good. Thanks, Mark
Re: [PATCH 4/5] tests/test-elf_cntl_gelf_getshdr.c: Close fd unconditionally
Hi Aaron,
On Thu, Jan 30, 2025 at 09:35:53PM -0500, Aaron Merey wrote:
> test-elf_cntl_gelf_getshdr conditionally closes a file descriptor
> depending on a command line argument. This causes an error when run
> under valgrind --track-fds=yes.
>
> Fix this by unconditionally closing the fd.
I think this subtly changes what is being tested here. If I remember
correctly this tests that gelf_getshdr works correctly even when the
underlying fd is closed and the file is either mmapped or elf_cntl
with ELF_C_FDREAD is called.
The idea being that if the file is mmapped then everything needed for
reading the ELF file is already in memory. And when elf_cntl is called
with ELF_C_FDREAD then libelf must make sure the same is true.
You could argue that the test should explicitly call elf_cntl with
ELF_C_FDDONE after the close (fd) though.
So I don't think close should be called unconditionally. Maybe the
correct solution is to do if (!close_fd) close (fd); after elf_end?
Cheers,
Mark
> Signed-off-by: Aaron Merey
> ---
> tests/test-elf_cntl_gelf_getshdr.c | 20 +---
> 1 file changed, 5 insertions(+), 15 deletions(-)
>
> diff --git a/tests/test-elf_cntl_gelf_getshdr.c
> b/tests/test-elf_cntl_gelf_getshdr.c
> index 7371110c..9f78bec2 100644
> --- a/tests/test-elf_cntl_gelf_getshdr.c
> +++ b/tests/test-elf_cntl_gelf_getshdr.c
> @@ -43,22 +43,12 @@ main (int argc, char *argv[])
> }
>
>bool do_mmap = false;
> - bool close_fd = false;
>if (strcmp (argv[1], "READ") == 0)
> -{
> - do_mmap = false;
> - close_fd = false;
> -}
> +do_mmap = false;
>else if (strcmp (argv[1], "MMAP") == 0)
> -{
> - do_mmap = true;
> - close_fd = false;
> -}
> +do_mmap = true;
>else if (strcmp (argv[1], "FDREAD") == 0)
> -{
> - do_mmap = false;
> - close_fd = true;
> -}
> +do_mmap = false;
>else
> {
>fprintf (stderr, "First arg needs to be 'READ', 'MMAP' or 'FDREAD'\n");
> @@ -83,7 +73,7 @@ main (int argc, char *argv[])
>exit (2);
> }
>
> - if (! do_mmap && close_fd)
> + if (! do_mmap)
> {
>if (elf_cntl (elf, ELF_C_FDREAD) < 0)
> {
> @@ -91,7 +81,6 @@ main (int argc, char *argv[])
> elf_errmsg (-1));
> exit (1);
> }
> - close (fd);
> }
>
>Elf_Scn *scn = NULL;
> @@ -103,5 +92,6 @@ main (int argc, char *argv[])
> }
>
>elf_end (elf);
> + close (fd);
>exit (0);
> }
> --
> 2.48.1
>
[Bug debuginfod/32629] add setter functions to debuginfod_client object as alternative to getenv/setenv
https://sourceware.org/bugzilla/show_bug.cgi?id=32629 Mark Wielaard changed: What|Removed |Added CC||mark at klomp dot org --- Comment #1 from Mark Wielaard --- Would it make sense to create an explicit "constructor" function? debuginfod_client *debuginfod_begin_urls (const char *urls); Which you would use to create a debuginfo_client handle that uses the provided urls. The existing debuginfod_begin () constructor would then simply be: return debuginfod_begin_urls (getenv (DEBUGINFOD_URLS_ENV_VAR)); -- You are receiving this mail because: You are on the CC list for the bug.
Re: [PATCH] libelf: fix DEREF_OF_NULL.RET in objdump.c
Hi Anton,
On Sat, Feb 01, 2025 at 02:21:24AM +0300, Anton Moryakov wrote:
> Report of the static analyzer:
> Pointer, returned from function 'elf_getarhdr' at
> objdump.c:314, may be NULL and is dereferenced at
> objdump.c:317. (CWE476, CWE690)
Nice catch.
> Corrections explained:
> When processing archive elements, the code could dereference
> a NULL pointer if 'elf_getarhdr' returns NULL. This patch adds
> a check to ensure 'arhdr' is not NULL before using it.
>
> The fix ensures that the function safely handles cases where
> 'elf_getarhdr' fails, avoiding potential crashes.
>
> Triggers found by static analyzer Svace.
>
> Signed-off-by: Anton Moryakov
>
> ---
> src/objdump.c | 7 +++
> 1 file changed, 7 insertions(+)
>
> diff --git a/src/objdump.c b/src/objdump.c
> index 1b38da23..9a66d362 100644
> --- a/src/objdump.c
> +++ b/src/objdump.c
> @@ -312,6 +312,13 @@ handle_ar (int fd, Elf *elf, const char *prefix, const
> char *fname,
>/* The the header for this element. */
>Elf_Arhdr *arhdr = elf_getarhdr (subelf);
>
> +if (arhdr == NULL)
> +{
> +error(0, 0, _("%s: failed to get archive header"), fname);
> +result = 1;
> +continue;
> +}
> +
I think this is wrong. You don't do any cleanup of the elf and the
continue seems to just recreate the same subelf, so eventually you run
out of memory.
It is probably simpler to just add the check here:
>/* Skip over the index entries. */
>if (strcmp (arhdr->ar_name, "/") != 0
> && strcmp (arhdr->ar_name, "//") != 0)
if (arhdr != NULL
&& strcmp (arhdr->ar_name, "/") != 0
&& strcmp (arhdr->ar_name, "//") != 0)
Cheers,
Mark
Re: [PATCH 09/10 v5] Add tests/run-eu-search-lines.sh
Hi Aaron, Found the issue with the patches not showing up on the list. They were sent to elfutils-patches@ which doesn't exists... Add elfutils-devel to the CC now. On Fri, Jan 31, 2025 at 07:50:15PM -0500, Aaron Merey wrote: > * tests/.gitignore: Add eu_search-lines. > * tests/Makefile.am: Add eu_search_lines, > run-eu-search-lines.sh. > * tests/eu_search_lines.c: New file. > tests/run-eu-search-lines.sh: New file. > > Signed-off-by: Heather S. McIntyre > Signed-off-by: Aaron Merey > Signed-off-by: Mark Wielaard > --- > v5 changes: pthread added to eu_search_lines_LDFLAGS unconditionally. Looks good. Thanks, Mark
Re: [PATCH 3/5] tests/backtrace-subr.sh: Avoid valgrind track-fds in check_native_core
Hi Aaron,
On Thu, Jan 30, 2025 at 09:35:52PM -0500, Aaron Merey wrote:
> valgrind --track-fds=yes might incorrectly report an error due to the use
> of inherited file descriptors in check_native_core.
Note that this fixed upstream by
commit 9f0e4107c140b47ea2a9c097afcac73a8454e17f
https://bugs.kde.org/show_bug.cgi?id=487296#c6
But that isn't in any released valgrind version. So doing this (now)
unconditionally is the right thing.
> Prevent this false positive by temporarily removing "--track-fds=yes" from
> $VALGRIND_CMD for the duration of the testrun in check_native_core.
Cute solution.
IMHO it makes sense to commit this before (or together with) enabling
--track-fds=yes by default (patch 1). Otherwise the backtrace tests
fail till this patch is applied.
Thanks,
Mark
> Signed-off-by: Aaron Merey
> ---
> tests/backtrace-subr.sh | 10 +-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/tests/backtrace-subr.sh b/tests/backtrace-subr.sh
> index b63e3814..0a5b38f8 100644
> --- a/tests/backtrace-subr.sh
> +++ b/tests/backtrace-subr.sh
> @@ -187,7 +187,9 @@ check_native_core()
>fi
>
>if [ "x$SAVED_VALGRIND_CMD" != "x" ]; then
> -VALGRIND_CMD="$SAVED_VALGRIND_CMD"
> +# Restore $VALGRIND_CMD but disable --track-fds for the following
> testrun.
> +# Valgrind --track-fds might complain about an inherited fd.
> +VALGRIND_CMD=$(sed 's/--track-fds=yes//g' <<< "$SAVED_VALGRIND_CMD")
> export VALGRIND_CMD
>fi
>
> @@ -195,6 +197,12 @@ check_native_core()
># - see function check_err.
>tempfiles $core{,.{bt,err}}
>(set +ex; testrun ${abs_builddir}/backtrace -e ${abs_builddir}/$child
> --core=$core 1>$core.bt 2>$core.err; true)
> +
> + if [ "x$SAVED_VALGRIND_CMD" != "x" ]; then
> +VALGRIND_CMD="$SAVED_VALGRIND_CMD"
> +export VALGRIND_CMD
> + fi
> +
>cat $core.{bt,err}
>check_native_unsupported $core.err $child-$core
>check_all $core.{bt,err} $child-$core
> --
> 2.48.1
>
[Bug debuginfod/32629] add setter functions to debuginfod_client object as alternative to getenv/setenv
https://sourceware.org/bugzilla/show_bug.cgi?id=32629 --- Comment #2 from Frank Ch. Eigler --- The _begin function is already a constructor, :-) it just doesn't take such parameters. There are other env vars that we take as inputs too, and it would be odd to take just one as a function parameter here. And then it needs to be fixed for the life of the object? Dunno if that makes sense. -- You are receiving this mail because: You are on the CC list for the bug.
Re: [PATCH 1/5] tests/Makefile.am: Add --track-fds=yes to valgrind_cmd
Hi Aaron, On Thu, Jan 30, 2025 at 09:35:50PM -0500, Aaron Merey wrote: > `valgrind --track-fds=yes` will report errors for file descriptor leaks > and attempts at closing invalid file descriptors. This looks good to me, but could we apply the patches in "reverse" order so we don't get test failures in between the patches? Would help when bisecting. And not getting the buildbots yelling at us. Cheers, Mark P.S. Sasha gave a really nice presentation on valgrind --track-fds at Fosdem this weekend: https://fosdem.org/2025/schedule/event/fosdem-2025-4178-using-the-valgrind-error-manager-for-file-descriptor-tracking/ > Signed-off-by: Aaron Merey > --- > tests/Makefile.am | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tests/Makefile.am b/tests/Makefile.am > index 8f087798..625a014f 100644 > --- a/tests/Makefile.am > +++ b/tests/Makefile.am > @@ -692,7 +692,7 @@ EXTRA_DIST = run-arextract.sh run-arsymtest.sh run-ar.sh \ > > > if USE_VALGRIND > -valgrind_cmd=valgrind -q --leak-check=full --error-exitcode=1 > +valgrind_cmd=valgrind -q --leak-check=full --error-exitcode=1 --track-fds=yes > endif
Re: [PATCH] libdw: fix DEREF_AFTER_NULL.EX in dwarf_ranges.c
Hi Anton,
On Sat, Feb 01, 2025 at 01:56:34AM +0300, Anton Moryakov wrote:
> Report of the static analyzer:
> After having been compared to a NULL value at
> dwarf_ranges.c:492, pointer 'd' is dereferenced at
> dwarf_ranges.c:531. (CWE476)
But there is a lot of code between those points. On line 526 there is
a call to initial_offset which will check cu->dbg->sectiondata[secidx]
(which is what d points to) and return an error if it is NULL. And on
line __libdw_offset_in_section which does a simular check. So I
believe the code cannot reach line 531 unless d != NULL.
> Corrections explained:
> When processing a DIE with missing or invalid section data,
> the code could dereference a NULL pointer, leading to undefined
> behavior. This patch adds a check to ensure 'd' is not NULL
> before using it.
>
> The fix ensures that the function safely handles cases where
> section data is missing, avoiding potential crashes.
Do you have an example where this happens? It might be that your
analyzer is right, but then it would be good to have some kind of
proof and/or reproducer.
Thanks,
Mark
> Triggers found by static analyzer Svace.
>
> Signed-off-by: Anton Moryakov
>
> ---
> libdw/dwarf_ranges.c | 6 +-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/libdw/dwarf_ranges.c b/libdw/dwarf_ranges.c
> index b853e4b9..e42d21cd 100644
> --- a/libdw/dwarf_ranges.c
> +++ b/libdw/dwarf_ranges.c
> @@ -532,7 +532,11 @@ dwarf_ranges (Dwarf_Die *die, ptrdiff_t offset,
> Dwarf_Addr *basep,
>secidx, offset, 1))
> return -1;
> }
> -
> + if(d == NULL)
> + {
> + __libdw_seterrno(DWARF_E_INVALID_DWARF);
> + return -1
> + }
>readp = d->d_buf + offset;
>readendp = d->d_buf + d->d_size;
>
> --
> 2.30.2
>
