* [PATCH] findtextrel: do not use unbound alloca
@ 2021-09-06 18:00 Dmitry V. Levin
2021-09-08 22:22 ` Mark Wielaard
2021-09-09 11:31 ` [PATCH v2] " Dmitry V. Levin
0 siblings, 2 replies; 4+ messages in thread
From: Dmitry V. Levin @ 2021-09-06 18:00 UTC (permalink / raw)
To: elfutils-devel
This fixes the following compilation warning:
findtextrel.c:184:1: warning: stack usage might be unbounded [-Wstack-usage=]
Signed-off-by: Dmitry V. Levin <ldv@altlinux.org>
---
src/ChangeLog | 5 +++++
src/Makefile.am | 1 -
src/findtextrel.c | 31 +++++++++----------------------
3 files changed, 14 insertions(+), 23 deletions(-)
diff --git a/src/ChangeLog b/src/ChangeLog
index 297627df..2238916f 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,5 +1,10 @@
2021-09-06 Dmitry V. Levin <ldv@altlinux.org>
+ * findtextrel.c: Include "libeu.h".
+ (process_file): Use xasprintf instead of alloca followed by series
+ of mempcpy and stpcpy.
+ * Makefile.am (findtextrel_no_Wstack_usage): Remove.
+
* objdump.c (show_disasm): Replace asprintf followed by
error(EXIT_FAILURE) with xasprintf.
* readelf.c (handle_gnu_hash): Likewise.
diff --git a/src/Makefile.am b/src/Makefile.am
index 88d0ac8f..ee695d5d 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -61,7 +61,6 @@ nm_no_Wstack_usage = yes
size_no_Wstack_usage = yes
strip_no_Wstack_usage = yes
elflint_no_Wstack_usage = yes
-findtextrel_no_Wstack_usage = yes
elfcmp_no_Wstack_usage = yes
objdump_no_Wstack_usage = yes
ranlib_no_Wstack_usage = yes
diff --git a/src/findtextrel.c b/src/findtextrel.c
index 220ee909..4bee80ae 100644
--- a/src/findtextrel.c
+++ b/src/findtextrel.c
@@ -36,6 +36,7 @@
#include <unistd.h>
#include <printversion.h>
+#include "libeu.h"
#include "system.h"
struct segments
@@ -185,25 +186,17 @@ process_file (const char *fname, bool more_than_one)
{
int result = 0;
void *knownsrcs = NULL;
+ char *new_fname = NULL;
- size_t fname_len = strlen (fname);
- size_t rootdir_len = strlen (rootdir);
const char *real_fname = fname;
if (fname[0] == '/' && (rootdir[0] != '/' || rootdir[1] != '\0'))
- {
- /* Prepend the user-provided root directory. */
- char *new_fname = alloca (rootdir_len + fname_len + 2);
- *((char *) mempcpy (stpcpy (mempcpy (new_fname, rootdir, rootdir_len),
- "/"),
- fname, fname_len)) = '\0';
- real_fname = new_fname;
- }
+ real_fname = new_fname = xasprintf ("%s/%s", rootdir, fname);
int fd = open (real_fname, O_RDONLY);
if (fd == -1)
{
error (0, errno, _("cannot open '%s'"), fname);
- return 1;
+ goto err_free;
}
Elf *elf = elf_begin (fd, ELF_C_READ_MMAP, NULL);
@@ -225,6 +218,8 @@ process_file (const char *fname, bool more_than_one)
elf_end (elf);
err_close:
close (fd);
+ err_free:
+ free (new_fname);
return 1;
}
@@ -362,18 +357,10 @@ cannot get program header index at offset %zd: %s"),
is specified with an absolute path. */
if (dw == NULL && fname[0] == '/')
{
- size_t debuginfo_rootlen = strlen (debuginfo_root);
- char *difname = (char *) alloca (rootdir_len + debuginfo_rootlen
- + fname_len + 8);
- strcpy (mempcpy (stpcpy (mempcpy (mempcpy (difname, rootdir,
- rootdir_len),
- debuginfo_root,
- debuginfo_rootlen),
- "/"),
- fname, fname_len),
- ".debug");
-
+ char *difname =
+ xasprintf("%s%s/%s.debug", rootdir, debuginfo_root, fname);
fd2 = open (difname, O_RDONLY);
+ free (difname);
if (fd2 != -1
&& (elf2 = elf_begin (fd2, ELF_C_READ_MMAP, NULL)) != NULL)
dw = dwarf_begin_elf (elf2, DWARF_C_READ, NULL);
--
ldv
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] findtextrel: do not use unbound alloca
2021-09-06 18:00 [PATCH] findtextrel: do not use unbound alloca Dmitry V. Levin
@ 2021-09-08 22:22 ` Mark Wielaard
2021-09-09 11:31 ` [PATCH v2] " Dmitry V. Levin
1 sibling, 0 replies; 4+ messages in thread
From: Mark Wielaard @ 2021-09-08 22:22 UTC (permalink / raw)
To: Dmitry V. Levin; +Cc: elfutils-devel
Hi Dmitry,
On Mon, Sep 06, 2021 at 06:00:00PM +0000, Dmitry V. Levin wrote:
> This fixes the following compilation warning:
>
> findtextrel.c:184:1: warning: stack usage might be unbounded [-Wstack-usage=]
And it simplifies the code a lot IMHO.
Looks good, please apply.
Thanks,
Mark
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v2] findtextrel: do not use unbound alloca
2021-09-06 18:00 [PATCH] findtextrel: do not use unbound alloca Dmitry V. Levin
2021-09-08 22:22 ` Mark Wielaard
@ 2021-09-09 11:31 ` Dmitry V. Levin
2021-09-09 16:40 ` Mark Wielaard
1 sibling, 1 reply; 4+ messages in thread
From: Dmitry V. Levin @ 2021-09-09 11:31 UTC (permalink / raw)
To: elfutils-devel
This fixes the following compilation warning:
findtextrel.c:184:1: warning: stack usage might be unbounded [-Wstack-usage=]
Signed-off-by: Dmitry V. Levin <ldv@altlinux.org>
---
v1 introduced a memory leak, so in v2 I rearranged the code a bit
to make clear the new code does not introduce any memory leaks.
src/ChangeLog | 7 +++++++
src/Makefile.am | 1 -
src/findtextrel.c | 52 +++++++++++++++++++++--------------------------
3 files changed, 30 insertions(+), 30 deletions(-)
diff --git a/src/ChangeLog b/src/ChangeLog
index 297627df..449ca17b 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,10 @@
+2021-09-09 Dmitry V. Levin <ldv@altlinux.org>
+
+ * findtextrel.c: Include "libeu.h".
+ (open_rootdir_file): New function.
+ (process_file): Use it to open file inside rootdir.
+ * Makefile.am (findtextrel_no_Wstack_usage): Remove.
+
2021-09-06 Dmitry V. Levin <ldv@altlinux.org>
* objdump.c (show_disasm): Replace asprintf followed by
diff --git a/src/Makefile.am b/src/Makefile.am
index 88d0ac8f..ee695d5d 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -61,7 +61,6 @@ nm_no_Wstack_usage = yes
size_no_Wstack_usage = yes
strip_no_Wstack_usage = yes
elflint_no_Wstack_usage = yes
-findtextrel_no_Wstack_usage = yes
elfcmp_no_Wstack_usage = yes
objdump_no_Wstack_usage = yes
ranlib_no_Wstack_usage = yes
diff --git a/src/findtextrel.c b/src/findtextrel.c
index 220ee909..fd7baddb 100644
--- a/src/findtextrel.c
+++ b/src/findtextrel.c
@@ -36,6 +36,7 @@
#include <unistd.h>
#include <printversion.h>
+#include "libeu.h"
#include "system.h"
struct segments
@@ -181,30 +182,31 @@ noop (void *arg __attribute__ ((unused)))
static int
-process_file (const char *fname, bool more_than_one)
+open_rootdir_file (const char *fname)
{
- int result = 0;
- void *knownsrcs = NULL;
-
- size_t fname_len = strlen (fname);
- size_t rootdir_len = strlen (rootdir);
+ char *new_fname = NULL;
const char *real_fname = fname;
+
if (fname[0] == '/' && (rootdir[0] != '/' || rootdir[1] != '\0'))
- {
- /* Prepend the user-provided root directory. */
- char *new_fname = alloca (rootdir_len + fname_len + 2);
- *((char *) mempcpy (stpcpy (mempcpy (new_fname, rootdir, rootdir_len),
- "/"),
- fname, fname_len)) = '\0';
- real_fname = new_fname;
- }
+ real_fname = new_fname = xasprintf ("%s/%s", rootdir, fname);
int fd = open (real_fname, O_RDONLY);
if (fd == -1)
- {
- error (0, errno, _("cannot open '%s'"), fname);
- return 1;
- }
+ error (0, errno, _("cannot open '%s'"), fname);
+
+ free (new_fname);
+ return fd;
+}
+
+
+static int
+process_file (const char *fname, bool more_than_one)
+{
+ int result = 0;
+ void *knownsrcs = NULL;
+ int fd = open_rootdir_file (fname);
+ if (fd == -1)
+ return 1;
Elf *elf = elf_begin (fd, ELF_C_READ_MMAP, NULL);
if (elf == NULL)
@@ -362,18 +364,10 @@ cannot get program header index at offset %zd: %s"),
is specified with an absolute path. */
if (dw == NULL && fname[0] == '/')
{
- size_t debuginfo_rootlen = strlen (debuginfo_root);
- char *difname = (char *) alloca (rootdir_len + debuginfo_rootlen
- + fname_len + 8);
- strcpy (mempcpy (stpcpy (mempcpy (mempcpy (difname, rootdir,
- rootdir_len),
- debuginfo_root,
- debuginfo_rootlen),
- "/"),
- fname, fname_len),
- ".debug");
-
+ char *difname =
+ xasprintf("%s%s/%s.debug", rootdir, debuginfo_root, fname);
fd2 = open (difname, O_RDONLY);
+ free (difname);
if (fd2 != -1
&& (elf2 = elf_begin (fd2, ELF_C_READ_MMAP, NULL)) != NULL)
dw = dwarf_begin_elf (elf2, DWARF_C_READ, NULL);
--
ldv
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] findtextrel: do not use unbound alloca
2021-09-09 11:31 ` [PATCH v2] " Dmitry V. Levin
@ 2021-09-09 16:40 ` Mark Wielaard
0 siblings, 0 replies; 4+ messages in thread
From: Mark Wielaard @ 2021-09-09 16:40 UTC (permalink / raw)
To: Dmitry V. Levin, elfutils-devel
Hi Dmitry,
On Thu, 2021-09-09 at 14:31 +0300, Dmitry V. Levin wrote:
> This fixes the following compilation warning:
>
> findtextrel.c:184:1: warning: stack usage might be unbounded [-
> Wstack-usage=]
>
> Signed-off-by: Dmitry V. Levin <ldv@altlinux.org>
> ---
>
> v1 introduced a memory leak, so in v2 I rearranged the code a bit
> to make clear the new code does not introduce any memory leaks.
Sorry I missed that in my original review. But yes, in the original you
only freed on error. Now always. Did the testsuite catch it with
configure --enable-valgrind ?
Looks good, please apply.
Thanks,
Mark
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-09-09 16:40 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-06 18:00 [PATCH] findtextrel: do not use unbound alloca Dmitry V. Levin
2021-09-08 22:22 ` Mark Wielaard
2021-09-09 11:31 ` [PATCH v2] " Dmitry V. Levin
2021-09-09 16:40 ` Mark Wielaard
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).