* [PATCH v2] mips: dl-machine-reject-phdr: Get rid of alloca.
@ 2023-06-23 17:24 Joe Simmons-Talbott
2023-06-25 12:20 ` Florian Weimer
0 siblings, 1 reply; 6+ messages in thread
From: Joe Simmons-Talbott @ 2023-06-23 17:24 UTC (permalink / raw)
To: libc-alpha; +Cc: Joe Simmons-Talbott
Use a scratch_buffer rather than alloca to avoid potential stack overflow.
Checked with build-many-glibcs.py on mips-linux-gnu
---
Changes to v1:
* Only allocate space for and read the ABI flags data rather than the
whole segment.
sysdeps/mips/dl-machine-reject-phdr.h | 86 ++++++++++++++++++++-------
1 file changed, 65 insertions(+), 21 deletions(-)
diff --git a/sysdeps/mips/dl-machine-reject-phdr.h b/sysdeps/mips/dl-machine-reject-phdr.h
index 104b590661..7645e5a517 100644
--- a/sysdeps/mips/dl-machine-reject-phdr.h
+++ b/sysdeps/mips/dl-machine-reject-phdr.h
@@ -20,6 +20,7 @@
#define _DL_MACHINE_REJECT_PHDR_H 1
#include <unistd.h>
+#include <scratch_buffer.h>
#include <sys/prctl.h>
#if defined PR_GET_FP_MODE && defined PR_SET_FP_MODE
@@ -172,27 +173,41 @@ elf_machine_reject_phdr_p (const ElfW(Phdr) *phdr, unsigned int phnum,
cur_mode = __prctl (PR_GET_FP_MODE);
# endif
#endif
+ struct scratch_buffer sbuf;
+ scratch_buffer_init (&sbuf);
/* Read the attributes section. */
if (ph != NULL)
{
- ElfW(Addr) size = ph->p_filesz;
+ ElfW(Addr) size = sizeof (Elf_MIPS_ABIFlags_v0);
+
+ if (ph->p_filesz < sizeof (Elf_MIPS_ABIFlags_v0))
+ {
+ scratch_buffer_free (&sbuf);
+ REJECT (" contains malformed PT_MIPS_ABIFLAGS\n");
+ }
if (ph->p_offset + size <= len)
mips_abiflags = (Elf_MIPS_ABIFlags_v0 *) (buf + ph->p_offset);
else
{
- mips_abiflags = alloca (size);
+ if (!scratch_buffer_set_array_size (&sbuf, 1, size))
+ REJECT (" unable to allocate memory\n");
+ mips_abiflags = sbuf.data;
+
__lseek (fd, ph->p_offset, SEEK_SET);
if (__libc_read (fd, (void *) mips_abiflags, size) != size)
- REJECT (" unable to read PT_MIPS_ABIFLAGS\n");
+ {
+ scratch_buffer_free (&sbuf);
+ REJECT (" unable to read PT_MIPS_ABIFLAGS\n");
+ }
}
- if (size < sizeof (Elf_MIPS_ABIFlags_v0))
- REJECT (" contains malformed PT_MIPS_ABIFLAGS\n");
-
if (__glibc_unlikely (mips_abiflags->flags2 != 0))
- REJECT (" unknown MIPS.abiflags flags2: %u\n", mips_abiflags->flags2);
+ {
+ scratch_buffer_free (&sbuf);
+ REJECT (" unknown MIPS.abiflags flags2: %u\n", mips_abiflags->flags2);
+ }
in_abi = mips_abiflags->fp_abi;
}
@@ -202,7 +217,10 @@ elf_machine_reject_phdr_p (const ElfW(Phdr) *phdr, unsigned int phnum,
/* Unknown ABIs are rejected. */
if (in_abi != -1 && in_abi > Val_GNU_MIPS_ABI_FP_MAX)
- REJECT (" uses unknown FP ABI: %u\n", in_abi);
+ {
+ scratch_buffer_free (&sbuf);
+ REJECT (" uses unknown FP ABI: %u\n", in_abi);
+ }
/* Obtain the initial requirements. */
in_req = (in_abi == -1) ? none_req : reqs[in_abi];
@@ -215,7 +233,10 @@ elf_machine_reject_phdr_p (const ElfW(Phdr) *phdr, unsigned int phnum,
struct abi_req existing_req;
if (cached_fpabi_reject_phdr_p (l))
- return true;
+ {
+ scratch_buffer_free (&sbuf);
+ return true;
+ }
#if _MIPS_SIM == _ABIO32
/* A special case arises for O32 FP64 and FP64A where the kernel
@@ -229,8 +250,11 @@ elf_machine_reject_phdr_p (const ElfW(Phdr) *phdr, unsigned int phnum,
if ((l->l_mach.fpabi == Val_GNU_MIPS_ABI_FP_64A
|| l->l_mach.fpabi == Val_GNU_MIPS_ABI_FP_64)
&& cur_mode == -1)
- REJECT (" found %s running in the wrong mode\n",
- fpabi_string (l->l_mach.fpabi));
+ {
+ scratch_buffer_free (&sbuf);
+ REJECT (" found %s running in the wrong mode\n",
+ fpabi_string (l->l_mach.fpabi));
+ }
#endif
/* Found a perfect match, success. */
@@ -238,7 +262,10 @@ elf_machine_reject_phdr_p (const ElfW(Phdr) *phdr, unsigned int phnum,
/* Unknown ABIs are rejected. */
if (l->l_mach.fpabi != -1 && l->l_mach.fpabi > Val_GNU_MIPS_ABI_FP_MAX)
- REJECT (" found unknown FP ABI: %u\n", l->l_mach.fpabi);
+ {
+ scratch_buffer_free (&sbuf);
+ REJECT (" found unknown FP ABI: %u\n", l->l_mach.fpabi);
+ }
existing_req = (l->l_mach.fpabi == -1 ? none_req
: reqs[l->l_mach.fpabi]);
@@ -262,9 +289,12 @@ elf_machine_reject_phdr_p (const ElfW(Phdr) *phdr, unsigned int phnum,
#endif
}
else
- REJECT (" uses %s, already loaded %s\n",
- fpabi_string (in_abi),
- fpabi_string (l->l_mach.fpabi));
+ {
+ scratch_buffer_free (&sbuf);
+ REJECT (" uses %s, already loaded %s\n",
+ fpabi_string (in_abi),
+ fpabi_string (l->l_mach.fpabi));
+ }
}
#if _MIPS_SIM == _ABIO32
@@ -281,7 +311,10 @@ elf_machine_reject_phdr_p (const ElfW(Phdr) *phdr, unsigned int phnum,
If the overall requirements cannot be met by FR0 then reject the
object. */
if (cur_mode == -1)
- return !in_req.fr0;
+ {
+ scratch_buffer_free (&sbuf);
+ return !in_req.fr0;
+ }
# if HAVE_PRCTL_FP_MODE
{
@@ -293,9 +326,12 @@ elf_machine_reject_phdr_p (const ElfW(Phdr) *phdr, unsigned int phnum,
can be either the FR1 mode or FR0 if the requirements are met by
FR0. */
if (cannot_mode_switch)
- return (!(in_req.fre && cur_mode == (PR_FP_MODE_FR | PR_FP_MODE_FRE))
- && !(in_req.fr1 && cur_mode == PR_FP_MODE_FR)
- && !(in_req.fr0 && cur_mode == 0));
+ {
+ scratch_buffer_free (&sbuf);
+ return (!(in_req.fre && cur_mode == (PR_FP_MODE_FR | PR_FP_MODE_FRE))
+ && !(in_req.fr1 && cur_mode == PR_FP_MODE_FR)
+ && !(in_req.fr0 && cur_mode == 0));
+ }
/* If the overall requirements can be satisfied by FRE but not FR1 then
fr1_mode must become FRE. */
@@ -305,14 +341,21 @@ elf_machine_reject_phdr_p (const ElfW(Phdr) *phdr, unsigned int phnum,
/* Set the new mode. Use fr1_mode if the requirements cannot be met by
FR0. */
if (!in_req.fr0)
- return __prctl (PR_SET_FP_MODE, fr1_mode) != 0;
+ {
+ scratch_buffer_free (&sbuf);
+ return __prctl (PR_SET_FP_MODE, fr1_mode) != 0;
+ }
else if (__prctl (PR_SET_FP_MODE, /* fr0_mode */ 0) != 0)
{
/* Setting FR0 can validly fail on an R6 core so retry with the FR1
mode as a fall back. */
if (errno != ENOTSUP)
- return true;
+ {
+ scratch_buffer_free (&sbuf);
+ return true;
+ }
+ scratch_buffer_free (&sbuf);
return __prctl (PR_SET_FP_MODE, fr1_mode) != 0;
}
}
@@ -320,6 +363,7 @@ elf_machine_reject_phdr_p (const ElfW(Phdr) *phdr, unsigned int phnum,
}
#endif /* _MIPS_SIM == _ABIO32 */
+ scratch_buffer_free (&sbuf);
return false;
}
--
2.39.2
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] mips: dl-machine-reject-phdr: Get rid of alloca.
2023-06-23 17:24 [PATCH v2] mips: dl-machine-reject-phdr: Get rid of alloca Joe Simmons-Talbott
@ 2023-06-25 12:20 ` Florian Weimer
0 siblings, 0 replies; 6+ messages in thread
From: Florian Weimer @ 2023-06-25 12:20 UTC (permalink / raw)
To: Joe Simmons-Talbott via Libc-alpha; +Cc: Joe Simmons-Talbott
* Joe Simmons-Talbott via Libc-alpha:
> Use a scratch_buffer rather than alloca to avoid potential stack
> overflow.
You don't need a scratch buffer anymore. Just read into the struct
directly.
Thanks,
Florian
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] mips: dl-machine-reject-phdr: Get rid of alloca.
2023-06-23 17:28 ` Joe Simmons-Talbott
@ 2023-06-23 17:34 ` Joe Simmons-Talbott
0 siblings, 0 replies; 6+ messages in thread
From: Joe Simmons-Talbott @ 2023-06-23 17:34 UTC (permalink / raw)
To: Florian Weimer, Joe Simmons-Talbott via Libc-alpha
On Fri, Jun 23, 2023 at 01:28:39PM -0400, Joe Simmons-Talbott via Libc-alpha wrote:
> On Fri, Jun 23, 2023 at 06:26:14PM +0200, Florian Weimer wrote:
> > * Joe Simmons-Talbott via Libc-alpha:
> >
> > > Use a scratch_buffer rather than alloca to avoid potential stack
> > > overflow.
> >
> > I think the code could read only the part of the program header that it
> > actually needs. I don't see anything that iterates through the entire
> > byte array.
> >
> > It was probably modeled on the old PT_NOTE handling code, which also
> > used alloca, and that had to iterate through the data.
> >
>
> I've posted a v2 that hopefully addresses this. Thanks for reviewing.
Oops! I sent the new version as v2 again. Should I resend it as v3 or
leave it as it is? Sorry for the mixup.
Thanks,
Joe
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] mips: dl-machine-reject-phdr: Get rid of alloca.
2023-06-23 16:26 ` Florian Weimer
@ 2023-06-23 17:28 ` Joe Simmons-Talbott
2023-06-23 17:34 ` Joe Simmons-Talbott
0 siblings, 1 reply; 6+ messages in thread
From: Joe Simmons-Talbott @ 2023-06-23 17:28 UTC (permalink / raw)
To: Florian Weimer; +Cc: Joe Simmons-Talbott via Libc-alpha
On Fri, Jun 23, 2023 at 06:26:14PM +0200, Florian Weimer wrote:
> * Joe Simmons-Talbott via Libc-alpha:
>
> > Use a scratch_buffer rather than alloca to avoid potential stack
> > overflow.
>
> I think the code could read only the part of the program header that it
> actually needs. I don't see anything that iterates through the entire
> byte array.
>
> It was probably modeled on the old PT_NOTE handling code, which also
> used alloca, and that had to iterate through the data.
>
I've posted a v2 that hopefully addresses this. Thanks for reviewing.
Thanks,
Joe
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] mips: dl-machine-reject-phdr: Get rid of alloca.
2023-06-23 13:49 Joe Simmons-Talbott
@ 2023-06-23 16:26 ` Florian Weimer
2023-06-23 17:28 ` Joe Simmons-Talbott
0 siblings, 1 reply; 6+ messages in thread
From: Florian Weimer @ 2023-06-23 16:26 UTC (permalink / raw)
To: Joe Simmons-Talbott via Libc-alpha; +Cc: Joe Simmons-Talbott
* Joe Simmons-Talbott via Libc-alpha:
> Use a scratch_buffer rather than alloca to avoid potential stack
> overflow.
I think the code could read only the part of the program header that it
actually needs. I don't see anything that iterates through the entire
byte array.
It was probably modeled on the old PT_NOTE handling code, which also
used alloca, and that had to iterate through the data.
Thanks,
Florian
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] mips: dl-machine-reject-phdr: Get rid of alloca.
@ 2023-06-23 13:49 Joe Simmons-Talbott
2023-06-23 16:26 ` Florian Weimer
0 siblings, 1 reply; 6+ messages in thread
From: Joe Simmons-Talbott @ 2023-06-23 13:49 UTC (permalink / raw)
To: libc-alpha; +Cc: Joe Simmons-Talbott
Use a scratch_buffer rather than alloca to avoid potential stack overflow.
Checked with build-many-glibcs.py on mips-linux-gnu
---
Changes to v1:
* Use sbuf not buf for scratch_buffer_set_array_size
* Move sbuf definition out of conditional block.
sysdeps/mips/dl-machine-reject-phdr.h | 80 +++++++++++++++++++++------
1 file changed, 62 insertions(+), 18 deletions(-)
diff --git a/sysdeps/mips/dl-machine-reject-phdr.h b/sysdeps/mips/dl-machine-reject-phdr.h
index 104b590661..455ee6efbc 100644
--- a/sysdeps/mips/dl-machine-reject-phdr.h
+++ b/sysdeps/mips/dl-machine-reject-phdr.h
@@ -20,6 +20,7 @@
#define _DL_MACHINE_REJECT_PHDR_H 1
#include <unistd.h>
+#include <scratch_buffer.h>
#include <sys/prctl.h>
#if defined PR_GET_FP_MODE && defined PR_SET_FP_MODE
@@ -172,6 +173,8 @@ elf_machine_reject_phdr_p (const ElfW(Phdr) *phdr, unsigned int phnum,
cur_mode = __prctl (PR_GET_FP_MODE);
# endif
#endif
+ struct scratch_buffer sbuf;
+ scratch_buffer_init (&sbuf);
/* Read the attributes section. */
if (ph != NULL)
@@ -182,17 +185,29 @@ elf_machine_reject_phdr_p (const ElfW(Phdr) *phdr, unsigned int phnum,
mips_abiflags = (Elf_MIPS_ABIFlags_v0 *) (buf + ph->p_offset);
else
{
- mips_abiflags = alloca (size);
+ if (!scratch_buffer_set_array_size (&sbuf, 1, size))
+ REJECT (" unable to allocate memory\n");
+ mips_abiflags = sbuf.data;
+
__lseek (fd, ph->p_offset, SEEK_SET);
if (__libc_read (fd, (void *) mips_abiflags, size) != size)
- REJECT (" unable to read PT_MIPS_ABIFLAGS\n");
+ {
+ scratch_buffer_free (&sbuf);
+ REJECT (" unable to read PT_MIPS_ABIFLAGS\n");
+ }
}
if (size < sizeof (Elf_MIPS_ABIFlags_v0))
- REJECT (" contains malformed PT_MIPS_ABIFLAGS\n");
+ {
+ scratch_buffer_free (&sbuf);
+ REJECT (" contains malformed PT_MIPS_ABIFLAGS\n");
+ }
if (__glibc_unlikely (mips_abiflags->flags2 != 0))
- REJECT (" unknown MIPS.abiflags flags2: %u\n", mips_abiflags->flags2);
+ {
+ scratch_buffer_free (&sbuf);
+ REJECT (" unknown MIPS.abiflags flags2: %u\n", mips_abiflags->flags2);
+ }
in_abi = mips_abiflags->fp_abi;
}
@@ -202,7 +217,10 @@ elf_machine_reject_phdr_p (const ElfW(Phdr) *phdr, unsigned int phnum,
/* Unknown ABIs are rejected. */
if (in_abi != -1 && in_abi > Val_GNU_MIPS_ABI_FP_MAX)
- REJECT (" uses unknown FP ABI: %u\n", in_abi);
+ {
+ scratch_buffer_free (&sbuf);
+ REJECT (" uses unknown FP ABI: %u\n", in_abi);
+ }
/* Obtain the initial requirements. */
in_req = (in_abi == -1) ? none_req : reqs[in_abi];
@@ -215,7 +233,10 @@ elf_machine_reject_phdr_p (const ElfW(Phdr) *phdr, unsigned int phnum,
struct abi_req existing_req;
if (cached_fpabi_reject_phdr_p (l))
- return true;
+ {
+ scratch_buffer_free (&sbuf);
+ return true;
+ }
#if _MIPS_SIM == _ABIO32
/* A special case arises for O32 FP64 and FP64A where the kernel
@@ -229,8 +250,11 @@ elf_machine_reject_phdr_p (const ElfW(Phdr) *phdr, unsigned int phnum,
if ((l->l_mach.fpabi == Val_GNU_MIPS_ABI_FP_64A
|| l->l_mach.fpabi == Val_GNU_MIPS_ABI_FP_64)
&& cur_mode == -1)
- REJECT (" found %s running in the wrong mode\n",
- fpabi_string (l->l_mach.fpabi));
+ {
+ scratch_buffer_free (&sbuf);
+ REJECT (" found %s running in the wrong mode\n",
+ fpabi_string (l->l_mach.fpabi));
+ }
#endif
/* Found a perfect match, success. */
@@ -238,7 +262,10 @@ elf_machine_reject_phdr_p (const ElfW(Phdr) *phdr, unsigned int phnum,
/* Unknown ABIs are rejected. */
if (l->l_mach.fpabi != -1 && l->l_mach.fpabi > Val_GNU_MIPS_ABI_FP_MAX)
- REJECT (" found unknown FP ABI: %u\n", l->l_mach.fpabi);
+ {
+ scratch_buffer_free (&sbuf);
+ REJECT (" found unknown FP ABI: %u\n", l->l_mach.fpabi);
+ }
existing_req = (l->l_mach.fpabi == -1 ? none_req
: reqs[l->l_mach.fpabi]);
@@ -262,9 +289,12 @@ elf_machine_reject_phdr_p (const ElfW(Phdr) *phdr, unsigned int phnum,
#endif
}
else
- REJECT (" uses %s, already loaded %s\n",
- fpabi_string (in_abi),
- fpabi_string (l->l_mach.fpabi));
+ {
+ scratch_buffer_free (&sbuf);
+ REJECT (" uses %s, already loaded %s\n",
+ fpabi_string (in_abi),
+ fpabi_string (l->l_mach.fpabi));
+ }
}
#if _MIPS_SIM == _ABIO32
@@ -281,7 +311,10 @@ elf_machine_reject_phdr_p (const ElfW(Phdr) *phdr, unsigned int phnum,
If the overall requirements cannot be met by FR0 then reject the
object. */
if (cur_mode == -1)
- return !in_req.fr0;
+ {
+ scratch_buffer_free (&sbuf);
+ return !in_req.fr0;
+ }
# if HAVE_PRCTL_FP_MODE
{
@@ -293,9 +326,12 @@ elf_machine_reject_phdr_p (const ElfW(Phdr) *phdr, unsigned int phnum,
can be either the FR1 mode or FR0 if the requirements are met by
FR0. */
if (cannot_mode_switch)
- return (!(in_req.fre && cur_mode == (PR_FP_MODE_FR | PR_FP_MODE_FRE))
- && !(in_req.fr1 && cur_mode == PR_FP_MODE_FR)
- && !(in_req.fr0 && cur_mode == 0));
+ {
+ scratch_buffer_free (&sbuf);
+ return (!(in_req.fre && cur_mode == (PR_FP_MODE_FR | PR_FP_MODE_FRE))
+ && !(in_req.fr1 && cur_mode == PR_FP_MODE_FR)
+ && !(in_req.fr0 && cur_mode == 0));
+ }
/* If the overall requirements can be satisfied by FRE but not FR1 then
fr1_mode must become FRE. */
@@ -305,14 +341,21 @@ elf_machine_reject_phdr_p (const ElfW(Phdr) *phdr, unsigned int phnum,
/* Set the new mode. Use fr1_mode if the requirements cannot be met by
FR0. */
if (!in_req.fr0)
- return __prctl (PR_SET_FP_MODE, fr1_mode) != 0;
+ {
+ scratch_buffer_free (&sbuf);
+ return __prctl (PR_SET_FP_MODE, fr1_mode) != 0;
+ }
else if (__prctl (PR_SET_FP_MODE, /* fr0_mode */ 0) != 0)
{
/* Setting FR0 can validly fail on an R6 core so retry with the FR1
mode as a fall back. */
if (errno != ENOTSUP)
- return true;
+ {
+ scratch_buffer_free (&sbuf);
+ return true;
+ }
+ scratch_buffer_free (&sbuf);
return __prctl (PR_SET_FP_MODE, fr1_mode) != 0;
}
}
@@ -320,6 +363,7 @@ elf_machine_reject_phdr_p (const ElfW(Phdr) *phdr, unsigned int phnum,
}
#endif /* _MIPS_SIM == _ABIO32 */
+ scratch_buffer_free (&sbuf);
return false;
}
--
2.39.2
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-06-25 12:20 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-23 17:24 [PATCH v2] mips: dl-machine-reject-phdr: Get rid of alloca Joe Simmons-Talbott
2023-06-25 12:20 ` Florian Weimer
-- strict thread matches above, loose matches on Subject: below --
2023-06-23 13:49 Joe Simmons-Talbott
2023-06-23 16:26 ` Florian Weimer
2023-06-23 17:28 ` Joe Simmons-Talbott
2023-06-23 17:34 ` Joe Simmons-Talbott
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).