* [PATCH v2] check_native: Get rid of alloca
@ 2023-06-01 14:55 Joe Simmons-Talbott
2023-06-09 15:00 ` Joe Simmons-Talbott
2023-06-12 21:09 ` Adhemerval Zanella Netto
0 siblings, 2 replies; 4+ messages in thread
From: Joe Simmons-Talbott @ 2023-06-01 14:55 UTC (permalink / raw)
To: libc-alpha; +Cc: Joe Simmons-Talbott
Use malloc rather than alloca to avoid potential stack overflow.
---
Changes to v1:
* Do not use a cleanup handler.
sysdeps/unix/sysv/linux/check_native.c | 35 ++++++++------------------
1 file changed, 11 insertions(+), 24 deletions(-)
diff --git a/sysdeps/unix/sysv/linux/check_native.c b/sysdeps/unix/sysv/linux/check_native.c
index 34876ca624..601f14e5b0 100644
--- a/sysdeps/unix/sysv/linux/check_native.c
+++ b/sysdeps/unix/sysv/linux/check_native.c
@@ -48,11 +48,20 @@ __check_native (uint32_t a1_index, int *a1_native,
nladdr.nl_family = AF_NETLINK;
socklen_t addr_len = sizeof (nladdr);
- bool use_malloc = false;
if (fd < 0)
return;
+ /* Netlink requires that user buffer needs to be either 8kb or page size
+ (whichever is bigger), however this has been changed over time and now
+ 8Kb is sufficient (check NLMSG_DEFAULT_SIZE on Linux
+ linux/include/linux/netlink.h). */
+ const size_t buf_size = 8192;
+ char *buf = malloc (buf_size);
+
+ if (buf == NULL)
+ goto out;
+
if (__bind (fd, (struct sockaddr *) &nladdr, sizeof (nladdr)) != 0
|| __getsockname (fd, (struct sockaddr *) &nladdr, &addr_len) != 0)
goto out;
@@ -81,26 +90,6 @@ __check_native (uint32_t a1_index, int *a1_native,
memset (&nladdr, '\0', sizeof (nladdr));
nladdr.nl_family = AF_NETLINK;
-#ifdef PAGE_SIZE
- /* Help the compiler optimize out the malloc call if PAGE_SIZE
- is constant and smaller or equal to PTHREAD_STACK_MIN/4. */
- const size_t buf_size = PAGE_SIZE;
-#else
- const size_t buf_size = __getpagesize ();
-#endif
- char *buf;
-
- if (__libc_use_alloca (buf_size))
- buf = alloca (buf_size);
- else
- {
- buf = malloc (buf_size);
- if (buf != NULL)
- use_malloc = true;
- else
- goto out;
- }
-
struct iovec iov = { buf, buf_size };
if (TEMP_FAILURE_RETRY (__sendto (fd, (void *) &req, sizeof (req), 0,
@@ -170,7 +159,5 @@ __check_native (uint32_t a1_index, int *a1_native,
out:
__close_nocancel_nostatus (fd);
-
- if (use_malloc)
- free (buf);
+ free(buf);
}
--
2.39.2
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] check_native: Get rid of alloca
2023-06-01 14:55 [PATCH v2] check_native: Get rid of alloca Joe Simmons-Talbott
@ 2023-06-09 15:00 ` Joe Simmons-Talbott
2023-06-12 21:09 ` Adhemerval Zanella Netto
1 sibling, 0 replies; 4+ messages in thread
From: Joe Simmons-Talbott @ 2023-06-09 15:00 UTC (permalink / raw)
To: libc-alpha
On Thu, Jun 01, 2023 at 10:55:00AM -0400, Joe Simmons-Talbott wrote:
> Use malloc rather than alloca to avoid potential stack overflow.
Ping.
Thanks,
Joe
> ---
> Changes to v1:
> * Do not use a cleanup handler.
>
> sysdeps/unix/sysv/linux/check_native.c | 35 ++++++++------------------
> 1 file changed, 11 insertions(+), 24 deletions(-)
>
> diff --git a/sysdeps/unix/sysv/linux/check_native.c b/sysdeps/unix/sysv/linux/check_native.c
> index 34876ca624..601f14e5b0 100644
> --- a/sysdeps/unix/sysv/linux/check_native.c
> +++ b/sysdeps/unix/sysv/linux/check_native.c
> @@ -48,11 +48,20 @@ __check_native (uint32_t a1_index, int *a1_native,
> nladdr.nl_family = AF_NETLINK;
>
> socklen_t addr_len = sizeof (nladdr);
> - bool use_malloc = false;
>
> if (fd < 0)
> return;
>
> + /* Netlink requires that user buffer needs to be either 8kb or page size
> + (whichever is bigger), however this has been changed over time and now
> + 8Kb is sufficient (check NLMSG_DEFAULT_SIZE on Linux
> + linux/include/linux/netlink.h). */
> + const size_t buf_size = 8192;
> + char *buf = malloc (buf_size);
> +
> + if (buf == NULL)
> + goto out;
> +
> if (__bind (fd, (struct sockaddr *) &nladdr, sizeof (nladdr)) != 0
> || __getsockname (fd, (struct sockaddr *) &nladdr, &addr_len) != 0)
> goto out;
> @@ -81,26 +90,6 @@ __check_native (uint32_t a1_index, int *a1_native,
> memset (&nladdr, '\0', sizeof (nladdr));
> nladdr.nl_family = AF_NETLINK;
>
> -#ifdef PAGE_SIZE
> - /* Help the compiler optimize out the malloc call if PAGE_SIZE
> - is constant and smaller or equal to PTHREAD_STACK_MIN/4. */
> - const size_t buf_size = PAGE_SIZE;
> -#else
> - const size_t buf_size = __getpagesize ();
> -#endif
> - char *buf;
> -
> - if (__libc_use_alloca (buf_size))
> - buf = alloca (buf_size);
> - else
> - {
> - buf = malloc (buf_size);
> - if (buf != NULL)
> - use_malloc = true;
> - else
> - goto out;
> - }
> -
> struct iovec iov = { buf, buf_size };
>
> if (TEMP_FAILURE_RETRY (__sendto (fd, (void *) &req, sizeof (req), 0,
> @@ -170,7 +159,5 @@ __check_native (uint32_t a1_index, int *a1_native,
>
> out:
> __close_nocancel_nostatus (fd);
> -
> - if (use_malloc)
> - free (buf);
> + free(buf);
> }
> --
> 2.39.2
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] check_native: Get rid of alloca
2023-06-01 14:55 [PATCH v2] check_native: Get rid of alloca Joe Simmons-Talbott
2023-06-09 15:00 ` Joe Simmons-Talbott
@ 2023-06-12 21:09 ` Adhemerval Zanella Netto
2023-06-13 14:06 ` Joe Simmons-Talbott
1 sibling, 1 reply; 4+ messages in thread
From: Adhemerval Zanella Netto @ 2023-06-12 21:09 UTC (permalink / raw)
To: libc-alpha, Joe Simmons-Talbott
On 01/06/23 11:55, Joe Simmons-Talbott via Libc-alpha wrote:
> Use malloc rather than alloca to avoid potential stack overflow.
LGTM with the small nit below fixed.
Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
> ---
> Changes to v1:
> * Do not use a cleanup handler.
>
> sysdeps/unix/sysv/linux/check_native.c | 35 ++++++++------------------
> 1 file changed, 11 insertions(+), 24 deletions(-)
>
> diff --git a/sysdeps/unix/sysv/linux/check_native.c b/sysdeps/unix/sysv/linux/check_native.c
> index 34876ca624..601f14e5b0 100644
> --- a/sysdeps/unix/sysv/linux/check_native.c
> +++ b/sysdeps/unix/sysv/linux/check_native.c
> @@ -48,11 +48,20 @@ __check_native (uint32_t a1_index, int *a1_native,
> nladdr.nl_family = AF_NETLINK;
>
> socklen_t addr_len = sizeof (nladdr);
> - bool use_malloc = false;
>
> if (fd < 0)
> return;
>
> + /* Netlink requires that user buffer needs to be either 8kb or page size
> + (whichever is bigger), however this has been changed over time and now
> + 8Kb is sufficient (check NLMSG_DEFAULT_SIZE on Linux
> + linux/include/linux/netlink.h). */
> + const size_t buf_size = 8192;
> + char *buf = malloc (buf_size);
> +
> + if (buf == NULL)
> + goto out;
> +
> if (__bind (fd, (struct sockaddr *) &nladdr, sizeof (nladdr)) != 0
> || __getsockname (fd, (struct sockaddr *) &nladdr, &addr_len) != 0)
> goto out;
> @@ -81,26 +90,6 @@ __check_native (uint32_t a1_index, int *a1_native,
> memset (&nladdr, '\0', sizeof (nladdr));
> nladdr.nl_family = AF_NETLINK;
>
> -#ifdef PAGE_SIZE
> - /* Help the compiler optimize out the malloc call if PAGE_SIZE
> - is constant and smaller or equal to PTHREAD_STACK_MIN/4. */
> - const size_t buf_size = PAGE_SIZE;
> -#else
> - const size_t buf_size = __getpagesize ();
> -#endif
> - char *buf;
> -
> - if (__libc_use_alloca (buf_size))
> - buf = alloca (buf_size);
> - else
> - {
> - buf = malloc (buf_size);
> - if (buf != NULL)
> - use_malloc = true;
> - else
> - goto out;
> - }
> -
> struct iovec iov = { buf, buf_size };
>
> if (TEMP_FAILURE_RETRY (__sendto (fd, (void *) &req, sizeof (req), 0,
> @@ -170,7 +159,5 @@ __check_native (uint32_t a1_index, int *a1_native,
>
> out:
> __close_nocancel_nostatus (fd);
> -
> - if (use_malloc)
> - free (buf);
> + free(buf);
Space before '('.
> }
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] check_native: Get rid of alloca
2023-06-12 21:09 ` Adhemerval Zanella Netto
@ 2023-06-13 14:06 ` Joe Simmons-Talbott
0 siblings, 0 replies; 4+ messages in thread
From: Joe Simmons-Talbott @ 2023-06-13 14:06 UTC (permalink / raw)
To: Adhemerval Zanella Netto; +Cc: libc-alpha
On Mon, Jun 12, 2023 at 06:09:58PM -0300, Adhemerval Zanella Netto wrote:
>
>
> On 01/06/23 11:55, Joe Simmons-Talbott via Libc-alpha wrote:
> > Use malloc rather than alloca to avoid potential stack overflow.
>
> LGTM with the small nit below fixed.
Thanks for the review. Fixed in v3.
Thanks,
Joe
>
> Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
>
> > ---
> > Changes to v1:
> > * Do not use a cleanup handler.
> >
> > sysdeps/unix/sysv/linux/check_native.c | 35 ++++++++------------------
> > 1 file changed, 11 insertions(+), 24 deletions(-)
> >
> > diff --git a/sysdeps/unix/sysv/linux/check_native.c b/sysdeps/unix/sysv/linux/check_native.c
> > index 34876ca624..601f14e5b0 100644
> > --- a/sysdeps/unix/sysv/linux/check_native.c
> > +++ b/sysdeps/unix/sysv/linux/check_native.c
> > @@ -48,11 +48,20 @@ __check_native (uint32_t a1_index, int *a1_native,
> > nladdr.nl_family = AF_NETLINK;
> >
> > socklen_t addr_len = sizeof (nladdr);
> > - bool use_malloc = false;
> >
> > if (fd < 0)
> > return;
> >
> > + /* Netlink requires that user buffer needs to be either 8kb or page size
> > + (whichever is bigger), however this has been changed over time and now
> > + 8Kb is sufficient (check NLMSG_DEFAULT_SIZE on Linux
> > + linux/include/linux/netlink.h). */
> > + const size_t buf_size = 8192;
> > + char *buf = malloc (buf_size);
> > +
> > + if (buf == NULL)
> > + goto out;
> > +
> > if (__bind (fd, (struct sockaddr *) &nladdr, sizeof (nladdr)) != 0
> > || __getsockname (fd, (struct sockaddr *) &nladdr, &addr_len) != 0)
> > goto out;
> > @@ -81,26 +90,6 @@ __check_native (uint32_t a1_index, int *a1_native,
> > memset (&nladdr, '\0', sizeof (nladdr));
> > nladdr.nl_family = AF_NETLINK;
> >
> > -#ifdef PAGE_SIZE
> > - /* Help the compiler optimize out the malloc call if PAGE_SIZE
> > - is constant and smaller or equal to PTHREAD_STACK_MIN/4. */
> > - const size_t buf_size = PAGE_SIZE;
> > -#else
> > - const size_t buf_size = __getpagesize ();
> > -#endif
> > - char *buf;
> > -
> > - if (__libc_use_alloca (buf_size))
> > - buf = alloca (buf_size);
> > - else
> > - {
> > - buf = malloc (buf_size);
> > - if (buf != NULL)
> > - use_malloc = true;
> > - else
> > - goto out;
> > - }
> > -
> > struct iovec iov = { buf, buf_size };
> >
> > if (TEMP_FAILURE_RETRY (__sendto (fd, (void *) &req, sizeof (req), 0,
> > @@ -170,7 +159,5 @@ __check_native (uint32_t a1_index, int *a1_native,
> >
> > out:
> > __close_nocancel_nostatus (fd);
> > -
> > - if (use_malloc)
> > - free (buf);
> > + free(buf);
>
> Space before '('.
>
> > }
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-06-13 14:06 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-01 14:55 [PATCH v2] check_native: Get rid of alloca Joe Simmons-Talbott
2023-06-09 15:00 ` Joe Simmons-Talbott
2023-06-12 21:09 ` Adhemerval Zanella Netto
2023-06-13 14:06 ` 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).