* [PATCH] resolv/res_query: Add note indicating that alloca usage is safe.
@ 2023-07-05 18:13 Joe Simmons-Talbott
2023-08-10 13:46 ` Joe Simmons-Talbott
2023-08-28 13:50 ` Adhemerval Zanella Netto
0 siblings, 2 replies; 7+ messages in thread
From: Joe Simmons-Talbott @ 2023-07-05 18:13 UTC (permalink / raw)
To: libc-alpha; +Cc: Joe Simmons-Talbott
The buffer size is small (< 1024) and fixed sized so alloca is safe
here.
---
resolv/res_query.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/resolv/res_query.c b/resolv/res_query.c
index 049de91b95..0e0e7be624 100644
--- a/resolv/res_query.c
+++ b/resolv/res_query.c
@@ -117,6 +117,7 @@ __res_context_query (struct resolv_context *ctx, const char *name,
int n, use_malloc = 0;
size_t bufsize = (type == T_QUERY_A_AND_AAAA ? 2 : 1) * QUERYSIZE;
+ /* alloca is safe here since bufsize < 1024 and fixed sized. */
u_char *buf = alloca (bufsize);
u_char *query1 = buf;
int nquery1 = -1;
--
2.39.2
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] resolv/res_query: Add note indicating that alloca usage is safe.
2023-07-05 18:13 [PATCH] resolv/res_query: Add note indicating that alloca usage is safe Joe Simmons-Talbott
@ 2023-08-10 13:46 ` Joe Simmons-Talbott
2023-08-28 13:21 ` Joe Simmons-Talbott
2023-08-28 13:50 ` Adhemerval Zanella Netto
1 sibling, 1 reply; 7+ messages in thread
From: Joe Simmons-Talbott @ 2023-08-10 13:46 UTC (permalink / raw)
To: libc-alpha
On Wed, Jul 05, 2023 at 02:13:41PM -0400, Joe Simmons-Talbott wrote:
> The buffer size is small (< 1024) and fixed sized so alloca is safe
> here.
Ping.
Thanks,
Joe
> ---
> resolv/res_query.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/resolv/res_query.c b/resolv/res_query.c
> index 049de91b95..0e0e7be624 100644
> --- a/resolv/res_query.c
> +++ b/resolv/res_query.c
> @@ -117,6 +117,7 @@ __res_context_query (struct resolv_context *ctx, const char *name,
> int n, use_malloc = 0;
>
> size_t bufsize = (type == T_QUERY_A_AND_AAAA ? 2 : 1) * QUERYSIZE;
> + /* alloca is safe here since bufsize < 1024 and fixed sized. */
> u_char *buf = alloca (bufsize);
> u_char *query1 = buf;
> int nquery1 = -1;
> --
> 2.39.2
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] resolv/res_query: Add note indicating that alloca usage is safe.
2023-08-10 13:46 ` Joe Simmons-Talbott
@ 2023-08-28 13:21 ` Joe Simmons-Talbott
0 siblings, 0 replies; 7+ messages in thread
From: Joe Simmons-Talbott @ 2023-08-28 13:21 UTC (permalink / raw)
To: libc-alpha
Ping.
On Thu, Aug 10, 2023 at 09:46:55AM -0400, Joe Simmons-Talbott via Libc-alpha wrote:
> On Wed, Jul 05, 2023 at 02:13:41PM -0400, Joe Simmons-Talbott wrote:
> > The buffer size is small (< 1024) and fixed sized so alloca is safe
> > here.
> Ping.
>
> Thanks,
> Joe
> > ---
> > resolv/res_query.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/resolv/res_query.c b/resolv/res_query.c
> > index 049de91b95..0e0e7be624 100644
> > --- a/resolv/res_query.c
> > +++ b/resolv/res_query.c
> > @@ -117,6 +117,7 @@ __res_context_query (struct resolv_context *ctx, const char *name,
> > int n, use_malloc = 0;
> >
> > size_t bufsize = (type == T_QUERY_A_AND_AAAA ? 2 : 1) * QUERYSIZE;
> > + /* alloca is safe here since bufsize < 1024 and fixed sized. */
> > u_char *buf = alloca (bufsize);
> > u_char *query1 = buf;
> > int nquery1 = -1;
> > --
> > 2.39.2
> >
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] resolv/res_query: Add note indicating that alloca usage is safe.
2023-07-05 18:13 [PATCH] resolv/res_query: Add note indicating that alloca usage is safe Joe Simmons-Talbott
2023-08-10 13:46 ` Joe Simmons-Talbott
@ 2023-08-28 13:50 ` Adhemerval Zanella Netto
2023-08-28 14:58 ` Joe Simmons-Talbott
1 sibling, 1 reply; 7+ messages in thread
From: Adhemerval Zanella Netto @ 2023-08-28 13:50 UTC (permalink / raw)
To: libc-alpha, Joe Simmons-Talbott
On 05/07/23 15:13, Joe Simmons-Talbott via Libc-alpha wrote:
> The buffer size is small (< 1024) and fixed sized so alloca is safe
> here.
> ---
> resolv/res_query.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/resolv/res_query.c b/resolv/res_query.c
> index 049de91b95..0e0e7be624 100644
> --- a/resolv/res_query.c
> +++ b/resolv/res_query.c
> @@ -117,6 +117,7 @@ __res_context_query (struct resolv_context *ctx, const char *name,
> int n, use_malloc = 0;
>
> size_t bufsize = (type == T_QUERY_A_AND_AAAA ? 2 : 1) * QUERYSIZE;
> + /* alloca is safe here since bufsize < 1024 and fixed sized. */
> u_char *buf = alloca (bufsize);
The bufsize on current Linux build is:
size_t bufsize = (type == 439963904 ? 2 : 1) * (12 + 4 + 255 + 1);
So with upper bound as 544 (2 * (12 + 4 + 255 + 1)). However, it might
increase to 2 * PACKETSIZE later with malloc. This is exactly the scenarion
scratch_buffer was created, so maybe we should use it. Below a complete
untested patch:
diff --git a/resolv/res_query.c b/resolv/res_query.c
index 049de91b95..b234db83c1 100644
--- a/resolv/res_query.c
+++ b/resolv/res_query.c
@@ -80,6 +80,7 @@
#include <stdlib.h>
#include <string.h>
#include <shlib-compat.h>
+#include <scratch_buffer.h>
#if PACKETSZ > 65536
#define MAXPACKET PACKETSZ
@@ -114,11 +115,14 @@ __res_context_query (struct resolv_context *ctx, const char *name,
struct __res_state *statp = ctx->resp;
UHEADER *hp = (UHEADER *) answer;
UHEADER *hp2;
- int n, use_malloc = 0;
-
- size_t bufsize = (type == T_QUERY_A_AND_AAAA ? 2 : 1) * QUERYSIZE;
- u_char *buf = alloca (bufsize);
- u_char *query1 = buf;
+ int n;
+
+ /* It requires 2 times QUERYSIZE for type == T_QUERY_A_AND_AAAA. */
+ struct scratch_buffer buf;
+ scratch_buffer_init (&buf);
+ _Static_assert (2 * QUERYSIZE <= sizeof (buf.__space.__c),
+ "scratch_buffer too small");
+ u_char *query1 = buf.data;
int nquery1 = -1;
u_char *query2 = NULL;
int nquery2 = 0;
@@ -129,14 +133,14 @@ __res_context_query (struct resolv_context *ctx, const char *name,
if (type == T_QUERY_A_AND_AAAA)
{
n = __res_context_mkquery (ctx, QUERY, name, class, T_A, NULL,
- query1, bufsize);
+ query1, buf.length);
if (n > 0)
{
if ((statp->options & (RES_USE_EDNS0|RES_USE_DNSSEC)) != 0)
{
/* Use RESOLV_EDNS_BUFFER_SIZE because the receive
buffer can be reallocated. */
- n = __res_nopt (ctx, n, query1, bufsize,
+ n = __res_nopt (ctx, n, query1, buf.length,
RESOLV_EDNS_BUFFER_SIZE);
if (n < 0)
goto unspec_nomem;
@@ -146,20 +150,20 @@ __res_context_query (struct resolv_context *ctx, const char *name,
/* Align the buffer. */
int npad = ((nquery1 + __alignof__ (HEADER) - 1)
& ~(__alignof__ (HEADER) - 1)) - nquery1;
- if (n > bufsize - npad)
+ if (n > buf.length - npad)
{
n = -1;
goto unspec_nomem;
}
int nused = n + npad;
- query2 = buf + nused;
+ query2 = buf.data + nused;
n = __res_context_mkquery (ctx, QUERY, name, class, T_AAAA,
- NULL, query2, bufsize - nused);
+ NULL, query2, buf.length - nused);
if (n > 0
&& (statp->options & (RES_USE_EDNS0|RES_USE_DNSSEC)) != 0)
/* Use RESOLV_EDNS_BUFFER_SIZE because the receive
buffer can be reallocated. */
- n = __res_nopt (ctx, n, query2, bufsize,
+ n = __res_nopt (ctx, n, query2, buf.length,
RESOLV_EDNS_BUFFER_SIZE);
nquery2 = n;
}
@@ -169,7 +173,7 @@ __res_context_query (struct resolv_context *ctx, const char *name,
else
{
n = __res_context_mkquery (ctx, QUERY, name, class, type, NULL,
- query1, bufsize);
+ query1, buf.length);
if (n > 0
&& (statp->options & (RES_USE_EDNS0|RES_USE_DNSSEC)) != 0)
@@ -181,27 +185,25 @@ __res_context_query (struct resolv_context *ctx, const char *name,
advertise = anslen;
else
advertise = RESOLV_EDNS_BUFFER_SIZE;
- n = __res_nopt (ctx, n, query1, bufsize, advertise);
+ n = __res_nopt (ctx, n, query1, buf.length, advertise);
}
nquery1 = n;
}
- if (__glibc_unlikely (n <= 0) && !use_malloc) {
+ if (__glibc_unlikely (n <= 0)) {
/* Retry just in case res_nmkquery failed because of too
short buffer. Shouldn't happen. */
- bufsize = (type == T_QUERY_A_AND_AAAA ? 2 : 1) * MAXPACKET;
- buf = malloc (bufsize);
- if (buf != NULL) {
- query1 = buf;
- use_malloc = 1;
+ if (scratch_buffer_set_array_size (&buf,
+ T_QUERY_A_AND_AAAA ? 2 : 1,
+ MAXPACKET)) {
+ query1 = buf.data;
goto again;
}
}
if (__glibc_unlikely (n <= 0)) {
RES_SET_H_ERRNO(statp, NO_RECOVERY);
- if (use_malloc)
- free (buf);
+ scratch_buffer_free (&buf);
return (n);
}
@@ -224,8 +226,7 @@ __res_context_query (struct resolv_context *ctx, const char *name,
answerp2_malloced);
}
- if (use_malloc)
- free (buf);
+ scratch_buffer_free (&buf);
if (n < 0) {
RES_SET_H_ERRNO(statp, TRY_AGAIN);
return (n);
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] resolv/res_query: Add note indicating that alloca usage is safe.
2023-08-28 13:50 ` Adhemerval Zanella Netto
@ 2023-08-28 14:58 ` Joe Simmons-Talbott
2023-09-14 15:04 ` Joe Simmons-Talbott
0 siblings, 1 reply; 7+ messages in thread
From: Joe Simmons-Talbott @ 2023-08-28 14:58 UTC (permalink / raw)
To: Adhemerval Zanella Netto; +Cc: libc-alpha
On Mon, Aug 28, 2023 at 10:50:21AM -0300, Adhemerval Zanella Netto wrote:
>
>
> On 05/07/23 15:13, Joe Simmons-Talbott via Libc-alpha wrote:
> > The buffer size is small (< 1024) and fixed sized so alloca is safe
> > here.
> > ---
> > resolv/res_query.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/resolv/res_query.c b/resolv/res_query.c
> > index 049de91b95..0e0e7be624 100644
> > --- a/resolv/res_query.c
> > +++ b/resolv/res_query.c
> > @@ -117,6 +117,7 @@ __res_context_query (struct resolv_context *ctx, const char *name,
> > int n, use_malloc = 0;
> >
> > size_t bufsize = (type == T_QUERY_A_AND_AAAA ? 2 : 1) * QUERYSIZE;
> > + /* alloca is safe here since bufsize < 1024 and fixed sized. */
> > u_char *buf = alloca (bufsize);
>
> The bufsize on current Linux build is:
>
> size_t bufsize = (type == 439963904 ? 2 : 1) * (12 + 4 + 255 + 1);
>
> So with upper bound as 544 (2 * (12 + 4 + 255 + 1)). However, it might
> increase to 2 * PACKETSIZE later with malloc. This is exactly the scenarion
> scratch_buffer was created, so maybe we should use it. Below a complete
> untested patch:
Thanks for the patch. I've tested it with 'make check' on
x86_64-linux-gnu. I'm not sure how to handle this and don't feel
comfortable submitting it on your behalf or as my own work but am happy
to give a Reviewed-by once there's a commit message.
Thanks,
Joe
>
> diff --git a/resolv/res_query.c b/resolv/res_query.c
> index 049de91b95..b234db83c1 100644
> --- a/resolv/res_query.c
> +++ b/resolv/res_query.c
> @@ -80,6 +80,7 @@
> #include <stdlib.h>
> #include <string.h>
> #include <shlib-compat.h>
> +#include <scratch_buffer.h>
>
> #if PACKETSZ > 65536
> #define MAXPACKET PACKETSZ
> @@ -114,11 +115,14 @@ __res_context_query (struct resolv_context *ctx, const char *name,
> struct __res_state *statp = ctx->resp;
> UHEADER *hp = (UHEADER *) answer;
> UHEADER *hp2;
> - int n, use_malloc = 0;
> -
> - size_t bufsize = (type == T_QUERY_A_AND_AAAA ? 2 : 1) * QUERYSIZE;
> - u_char *buf = alloca (bufsize);
> - u_char *query1 = buf;
> + int n;
> +
> + /* It requires 2 times QUERYSIZE for type == T_QUERY_A_AND_AAAA. */
> + struct scratch_buffer buf;
> + scratch_buffer_init (&buf);
> + _Static_assert (2 * QUERYSIZE <= sizeof (buf.__space.__c),
> + "scratch_buffer too small");
> + u_char *query1 = buf.data;
> int nquery1 = -1;
> u_char *query2 = NULL;
> int nquery2 = 0;
> @@ -129,14 +133,14 @@ __res_context_query (struct resolv_context *ctx, const char *name,
> if (type == T_QUERY_A_AND_AAAA)
> {
> n = __res_context_mkquery (ctx, QUERY, name, class, T_A, NULL,
> - query1, bufsize);
> + query1, buf.length);
> if (n > 0)
> {
> if ((statp->options & (RES_USE_EDNS0|RES_USE_DNSSEC)) != 0)
> {
> /* Use RESOLV_EDNS_BUFFER_SIZE because the receive
> buffer can be reallocated. */
> - n = __res_nopt (ctx, n, query1, bufsize,
> + n = __res_nopt (ctx, n, query1, buf.length,
> RESOLV_EDNS_BUFFER_SIZE);
> if (n < 0)
> goto unspec_nomem;
> @@ -146,20 +150,20 @@ __res_context_query (struct resolv_context *ctx, const char *name,
> /* Align the buffer. */
> int npad = ((nquery1 + __alignof__ (HEADER) - 1)
> & ~(__alignof__ (HEADER) - 1)) - nquery1;
> - if (n > bufsize - npad)
> + if (n > buf.length - npad)
> {
> n = -1;
> goto unspec_nomem;
> }
> int nused = n + npad;
> - query2 = buf + nused;
> + query2 = buf.data + nused;
> n = __res_context_mkquery (ctx, QUERY, name, class, T_AAAA,
> - NULL, query2, bufsize - nused);
> + NULL, query2, buf.length - nused);
> if (n > 0
> && (statp->options & (RES_USE_EDNS0|RES_USE_DNSSEC)) != 0)
> /* Use RESOLV_EDNS_BUFFER_SIZE because the receive
> buffer can be reallocated. */
> - n = __res_nopt (ctx, n, query2, bufsize,
> + n = __res_nopt (ctx, n, query2, buf.length,
> RESOLV_EDNS_BUFFER_SIZE);
> nquery2 = n;
> }
> @@ -169,7 +173,7 @@ __res_context_query (struct resolv_context *ctx, const char *name,
> else
> {
> n = __res_context_mkquery (ctx, QUERY, name, class, type, NULL,
> - query1, bufsize);
> + query1, buf.length);
>
> if (n > 0
> && (statp->options & (RES_USE_EDNS0|RES_USE_DNSSEC)) != 0)
> @@ -181,27 +185,25 @@ __res_context_query (struct resolv_context *ctx, const char *name,
> advertise = anslen;
> else
> advertise = RESOLV_EDNS_BUFFER_SIZE;
> - n = __res_nopt (ctx, n, query1, bufsize, advertise);
> + n = __res_nopt (ctx, n, query1, buf.length, advertise);
> }
>
> nquery1 = n;
> }
>
> - if (__glibc_unlikely (n <= 0) && !use_malloc) {
> + if (__glibc_unlikely (n <= 0)) {
> /* Retry just in case res_nmkquery failed because of too
> short buffer. Shouldn't happen. */
> - bufsize = (type == T_QUERY_A_AND_AAAA ? 2 : 1) * MAXPACKET;
> - buf = malloc (bufsize);
> - if (buf != NULL) {
> - query1 = buf;
> - use_malloc = 1;
> + if (scratch_buffer_set_array_size (&buf,
> + T_QUERY_A_AND_AAAA ? 2 : 1,
> + MAXPACKET)) {
> + query1 = buf.data;
> goto again;
> }
> }
> if (__glibc_unlikely (n <= 0)) {
> RES_SET_H_ERRNO(statp, NO_RECOVERY);
> - if (use_malloc)
> - free (buf);
> + scratch_buffer_free (&buf);
> return (n);
> }
>
> @@ -224,8 +226,7 @@ __res_context_query (struct resolv_context *ctx, const char *name,
> answerp2_malloced);
> }
>
> - if (use_malloc)
> - free (buf);
> + scratch_buffer_free (&buf);
> if (n < 0) {
> RES_SET_H_ERRNO(statp, TRY_AGAIN);
> return (n);
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] resolv/res_query: Add note indicating that alloca usage is safe.
2023-08-28 14:58 ` Joe Simmons-Talbott
@ 2023-09-14 15:04 ` Joe Simmons-Talbott
2023-09-14 17:36 ` Adhemerval Zanella Netto
0 siblings, 1 reply; 7+ messages in thread
From: Joe Simmons-Talbott @ 2023-09-14 15:04 UTC (permalink / raw)
To: Adhemerval Zanella Netto, libc-alpha
Ping.
On Mon, Aug 28, 2023 at 10:58:25AM -0400, Joe Simmons-Talbott via Libc-alpha wrote:
> On Mon, Aug 28, 2023 at 10:50:21AM -0300, Adhemerval Zanella Netto wrote:
> >
> >
> > On 05/07/23 15:13, Joe Simmons-Talbott via Libc-alpha wrote:
> > > The buffer size is small (< 1024) and fixed sized so alloca is safe
> > > here.
> > > ---
> > > resolv/res_query.c | 1 +
> > > 1 file changed, 1 insertion(+)
> > >
> > > diff --git a/resolv/res_query.c b/resolv/res_query.c
> > > index 049de91b95..0e0e7be624 100644
> > > --- a/resolv/res_query.c
> > > +++ b/resolv/res_query.c
> > > @@ -117,6 +117,7 @@ __res_context_query (struct resolv_context *ctx, const char *name,
> > > int n, use_malloc = 0;
> > >
> > > size_t bufsize = (type == T_QUERY_A_AND_AAAA ? 2 : 1) * QUERYSIZE;
> > > + /* alloca is safe here since bufsize < 1024 and fixed sized. */
> > > u_char *buf = alloca (bufsize);
> >
> > The bufsize on current Linux build is:
> >
> > size_t bufsize = (type == 439963904 ? 2 : 1) * (12 + 4 + 255 + 1);
> >
> > So with upper bound as 544 (2 * (12 + 4 + 255 + 1)). However, it might
> > increase to 2 * PACKETSIZE later with malloc. This is exactly the scenarion
> > scratch_buffer was created, so maybe we should use it. Below a complete
> > untested patch:
>
> Thanks for the patch. I've tested it with 'make check' on
> x86_64-linux-gnu. I'm not sure how to handle this and don't feel
> comfortable submitting it on your behalf or as my own work but am happy
> to give a Reviewed-by once there's a commit message.
>
> Thanks,
> Joe
> >
> > diff --git a/resolv/res_query.c b/resolv/res_query.c
> > index 049de91b95..b234db83c1 100644
> > --- a/resolv/res_query.c
> > +++ b/resolv/res_query.c
> > @@ -80,6 +80,7 @@
> > #include <stdlib.h>
> > #include <string.h>
> > #include <shlib-compat.h>
> > +#include <scratch_buffer.h>
> >
> > #if PACKETSZ > 65536
> > #define MAXPACKET PACKETSZ
> > @@ -114,11 +115,14 @@ __res_context_query (struct resolv_context *ctx, const char *name,
> > struct __res_state *statp = ctx->resp;
> > UHEADER *hp = (UHEADER *) answer;
> > UHEADER *hp2;
> > - int n, use_malloc = 0;
> > -
> > - size_t bufsize = (type == T_QUERY_A_AND_AAAA ? 2 : 1) * QUERYSIZE;
> > - u_char *buf = alloca (bufsize);
> > - u_char *query1 = buf;
> > + int n;
> > +
> > + /* It requires 2 times QUERYSIZE for type == T_QUERY_A_AND_AAAA. */
> > + struct scratch_buffer buf;
> > + scratch_buffer_init (&buf);
> > + _Static_assert (2 * QUERYSIZE <= sizeof (buf.__space.__c),
> > + "scratch_buffer too small");
> > + u_char *query1 = buf.data;
> > int nquery1 = -1;
> > u_char *query2 = NULL;
> > int nquery2 = 0;
> > @@ -129,14 +133,14 @@ __res_context_query (struct resolv_context *ctx, const char *name,
> > if (type == T_QUERY_A_AND_AAAA)
> > {
> > n = __res_context_mkquery (ctx, QUERY, name, class, T_A, NULL,
> > - query1, bufsize);
> > + query1, buf.length);
> > if (n > 0)
> > {
> > if ((statp->options & (RES_USE_EDNS0|RES_USE_DNSSEC)) != 0)
> > {
> > /* Use RESOLV_EDNS_BUFFER_SIZE because the receive
> > buffer can be reallocated. */
> > - n = __res_nopt (ctx, n, query1, bufsize,
> > + n = __res_nopt (ctx, n, query1, buf.length,
> > RESOLV_EDNS_BUFFER_SIZE);
> > if (n < 0)
> > goto unspec_nomem;
> > @@ -146,20 +150,20 @@ __res_context_query (struct resolv_context *ctx, const char *name,
> > /* Align the buffer. */
> > int npad = ((nquery1 + __alignof__ (HEADER) - 1)
> > & ~(__alignof__ (HEADER) - 1)) - nquery1;
> > - if (n > bufsize - npad)
> > + if (n > buf.length - npad)
> > {
> > n = -1;
> > goto unspec_nomem;
> > }
> > int nused = n + npad;
> > - query2 = buf + nused;
> > + query2 = buf.data + nused;
> > n = __res_context_mkquery (ctx, QUERY, name, class, T_AAAA,
> > - NULL, query2, bufsize - nused);
> > + NULL, query2, buf.length - nused);
> > if (n > 0
> > && (statp->options & (RES_USE_EDNS0|RES_USE_DNSSEC)) != 0)
> > /* Use RESOLV_EDNS_BUFFER_SIZE because the receive
> > buffer can be reallocated. */
> > - n = __res_nopt (ctx, n, query2, bufsize,
> > + n = __res_nopt (ctx, n, query2, buf.length,
> > RESOLV_EDNS_BUFFER_SIZE);
> > nquery2 = n;
> > }
> > @@ -169,7 +173,7 @@ __res_context_query (struct resolv_context *ctx, const char *name,
> > else
> > {
> > n = __res_context_mkquery (ctx, QUERY, name, class, type, NULL,
> > - query1, bufsize);
> > + query1, buf.length);
> >
> > if (n > 0
> > && (statp->options & (RES_USE_EDNS0|RES_USE_DNSSEC)) != 0)
> > @@ -181,27 +185,25 @@ __res_context_query (struct resolv_context *ctx, const char *name,
> > advertise = anslen;
> > else
> > advertise = RESOLV_EDNS_BUFFER_SIZE;
> > - n = __res_nopt (ctx, n, query1, bufsize, advertise);
> > + n = __res_nopt (ctx, n, query1, buf.length, advertise);
> > }
> >
> > nquery1 = n;
> > }
> >
> > - if (__glibc_unlikely (n <= 0) && !use_malloc) {
> > + if (__glibc_unlikely (n <= 0)) {
> > /* Retry just in case res_nmkquery failed because of too
> > short buffer. Shouldn't happen. */
> > - bufsize = (type == T_QUERY_A_AND_AAAA ? 2 : 1) * MAXPACKET;
> > - buf = malloc (bufsize);
> > - if (buf != NULL) {
> > - query1 = buf;
> > - use_malloc = 1;
> > + if (scratch_buffer_set_array_size (&buf,
> > + T_QUERY_A_AND_AAAA ? 2 : 1,
> > + MAXPACKET)) {
> > + query1 = buf.data;
> > goto again;
> > }
> > }
> > if (__glibc_unlikely (n <= 0)) {
> > RES_SET_H_ERRNO(statp, NO_RECOVERY);
> > - if (use_malloc)
> > - free (buf);
> > + scratch_buffer_free (&buf);
> > return (n);
> > }
> >
> > @@ -224,8 +226,7 @@ __res_context_query (struct resolv_context *ctx, const char *name,
> > answerp2_malloced);
> > }
> >
> > - if (use_malloc)
> > - free (buf);
> > + scratch_buffer_free (&buf);
> > if (n < 0) {
> > RES_SET_H_ERRNO(statp, TRY_AGAIN);
> > return (n);
> >
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] resolv/res_query: Add note indicating that alloca usage is safe.
2023-09-14 15:04 ` Joe Simmons-Talbott
@ 2023-09-14 17:36 ` Adhemerval Zanella Netto
0 siblings, 0 replies; 7+ messages in thread
From: Adhemerval Zanella Netto @ 2023-09-14 17:36 UTC (permalink / raw)
To: Joe Simmons-Talbott, libc-alpha
On 14/09/23 12:04, Joe Simmons-Talbott wrote:
> Ping.
>
> On Mon, Aug 28, 2023 at 10:58:25AM -0400, Joe Simmons-Talbott via Libc-alpha wrote:
>> On Mon, Aug 28, 2023 at 10:50:21AM -0300, Adhemerval Zanella Netto wrote:
>>>
>>>
>>> On 05/07/23 15:13, Joe Simmons-Talbott via Libc-alpha wrote:
>>>> The buffer size is small (< 1024) and fixed sized so alloca is safe
>>>> here.
>>>> ---
>>>> resolv/res_query.c | 1 +
>>>> 1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/resolv/res_query.c b/resolv/res_query.c
>>>> index 049de91b95..0e0e7be624 100644
>>>> --- a/resolv/res_query.c
>>>> +++ b/resolv/res_query.c
>>>> @@ -117,6 +117,7 @@ __res_context_query (struct resolv_context *ctx, const char *name,
>>>> int n, use_malloc = 0;
>>>>
>>>> size_t bufsize = (type == T_QUERY_A_AND_AAAA ? 2 : 1) * QUERYSIZE;
>>>> + /* alloca is safe here since bufsize < 1024 and fixed sized. */
>>>> u_char *buf = alloca (bufsize);
>>>
>>> The bufsize on current Linux build is:
>>>
>>> size_t bufsize = (type == 439963904 ? 2 : 1) * (12 + 4 + 255 + 1);
>>>
>>> So with upper bound as 544 (2 * (12 + 4 + 255 + 1)). However, it might
>>> increase to 2 * PACKETSIZE later with malloc. This is exactly the scenarion
>>> scratch_buffer was created, so maybe we should use it. Below a complete
>>> untested patch:
>>
>> Thanks for the patch. I've tested it with 'make check' on
>> x86_64-linux-gnu. I'm not sure how to handle this and don't feel
>> comfortable submitting it on your behalf or as my own work but am happy
>> to give a Reviewed-by once there's a commit message.
I had forgotten about this, I will send a proper patch.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-09-14 17:36 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-07-05 18:13 [PATCH] resolv/res_query: Add note indicating that alloca usage is safe Joe Simmons-Talbott
2023-08-10 13:46 ` Joe Simmons-Talbott
2023-08-28 13:21 ` Joe Simmons-Talbott
2023-08-28 13:50 ` Adhemerval Zanella Netto
2023-08-28 14:58 ` Joe Simmons-Talbott
2023-09-14 15:04 ` Joe Simmons-Talbott
2023-09-14 17:36 ` Adhemerval Zanella Netto
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).