nss: Get rid of alloca usage in db-XXX

Message ID 20230927153831.2638026-1-josimmon@redhat.com
State Superseded
Headers
Series nss: Get rid of alloca usage in db-XXX |

Checks

Context Check Description
redhat-pt-bot/TryBot-apply_patch success Patch applied to master at the time it was sent
redhat-pt-bot/TryBot-32bit success Build for i686
linaro-tcwg-bot/tcwg_glibc_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_glibc_check--master-arm success Testing passed
linaro-tcwg-bot/tcwg_glibc_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_glibc_check--master-aarch64 success Testing passed

Commit Message

Joe Simmons-Talbott Sept. 27, 2023, 3:38 p.m. UTC
  Replace alloca usage with a scratch_buffer.
---
 nss/nss_db/db-XXX.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)
  

Comments

Andreas Schwab Sept. 27, 2023, 3:55 p.m. UTC | #1
On Sep 27 2023, Joe Simmons-Talbott wrote:

> Replace alloca usage with a scratch_buffer.
> ---
>  nss/nss_db/db-XXX.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/nss/nss_db/db-XXX.c b/nss/nss_db/db-XXX.c
> index 0ce730db5a..2df31a8077 100644
> --- a/nss/nss_db/db-XXX.c
> +++ b/nss/nss_db/db-XXX.c
> @@ -18,6 +18,7 @@
>  
>  #include <dlfcn.h>
>  #include <fcntl.h>
> +#include <scratch_buffer.h>
>  #include <stdint.h>
>  #include <sys/mman.h>
>  #include <libc-lock.h>
> @@ -126,6 +127,8 @@ enum nss_status								      \
>  			char *buffer, size_t buflen, int *errnop H_ERRNO_PROTO)\
>  {									      \
>    struct parser_data *data = (void *) buffer;				      \
> +  struct scratch_buffer sbuf;						      \
> +  scratch_buffer_init (&sbuf);						      \
>  									      \
>    if (buflen < sizeof *data)						      \
>      {									      \
> @@ -160,7 +163,13 @@ enum nss_status								      \
>    else									      \
>      {									      \
>        const size_t size = (keysize) + 1;				      \
> -      key = alloca (size);						      \
> +      if (!scratch_buffer_set_array_size (&sbuf, 1, size))		      \
> +	{								      \
> +	  H_ERRNO_SET (NETDB_INTERNAL);					      \
> +	  status = NSS_STATUS_TRYAGAIN;					      \
> +	  return status;						      \

That needs to set *errnop and goto out.
  
Joe Simmons-Talbott Sept. 27, 2023, 8:55 p.m. UTC | #2
On Wed, Sep 27, 2023 at 05:55:05PM +0200, Andreas Schwab wrote:
> On Sep 27 2023, Joe Simmons-Talbott wrote:
> 
> > Replace alloca usage with a scratch_buffer.
> > ---
> >  nss/nss_db/db-XXX.c | 12 +++++++++++-
> >  1 file changed, 11 insertions(+), 1 deletion(-)
> >
> > diff --git a/nss/nss_db/db-XXX.c b/nss/nss_db/db-XXX.c
> > index 0ce730db5a..2df31a8077 100644
> > --- a/nss/nss_db/db-XXX.c
> > +++ b/nss/nss_db/db-XXX.c
> > @@ -18,6 +18,7 @@
> >  
> >  #include <dlfcn.h>
> >  #include <fcntl.h>
> > +#include <scratch_buffer.h>
> >  #include <stdint.h>
> >  #include <sys/mman.h>
> >  #include <libc-lock.h>
> > @@ -126,6 +127,8 @@ enum nss_status								      \
> >  			char *buffer, size_t buflen, int *errnop H_ERRNO_PROTO)\
> >  {									      \
> >    struct parser_data *data = (void *) buffer;				      \
> > +  struct scratch_buffer sbuf;						      \
> > +  scratch_buffer_init (&sbuf);						      \
> >  									      \
> >    if (buflen < sizeof *data)						      \
> >      {									      \
> > @@ -160,7 +163,13 @@ enum nss_status								      \
> >    else									      \
> >      {									      \
> >        const size_t size = (keysize) + 1;				      \
> > -      key = alloca (size);						      \
> > +      if (!scratch_buffer_set_array_size (&sbuf, 1, size))		      \
> > +	{								      \
> > +	  H_ERRNO_SET (NETDB_INTERNAL);					      \
> > +	  status = NSS_STATUS_TRYAGAIN;					      \
> > +	  return status;						      \
> 
> That needs to set *errnop and goto out.
> 

Thanks.  Fixed in v2.  I left the H_ERRNO_SET call in there as I wasn't
sure if I needed it or not.

Thanks,
Joe
  
Andreas Schwab Sept. 28, 2023, 8:37 a.m. UTC | #3
On Sep 27 2023, Joe Simmons-Talbott wrote:

> Thanks.  Fixed in v2.  I left the H_ERRNO_SET call in there as I wasn't
> sure if I needed it or not.

It is not redundant.  NETDB_INTERNAL is the resolver error that says
that there was a system error, with the cause recorded in errno.
  

Patch

diff --git a/nss/nss_db/db-XXX.c b/nss/nss_db/db-XXX.c
index 0ce730db5a..2df31a8077 100644
--- a/nss/nss_db/db-XXX.c
+++ b/nss/nss_db/db-XXX.c
@@ -18,6 +18,7 @@ 
 
 #include <dlfcn.h>
 #include <fcntl.h>
+#include <scratch_buffer.h>
 #include <stdint.h>
 #include <sys/mman.h>
 #include <libc-lock.h>
@@ -126,6 +127,8 @@  enum nss_status								      \
 			char *buffer, size_t buflen, int *errnop H_ERRNO_PROTO)\
 {									      \
   struct parser_data *data = (void *) buffer;				      \
+  struct scratch_buffer sbuf;						      \
+  scratch_buffer_init (&sbuf);						      \
 									      \
   if (buflen < sizeof *data)						      \
     {									      \
@@ -160,7 +163,13 @@  enum nss_status								      \
   else									      \
     {									      \
       const size_t size = (keysize) + 1;				      \
-      key = alloca (size);						      \
+      if (!scratch_buffer_set_array_size (&sbuf, 1, size))		      \
+	{								      \
+	  H_ERRNO_SET (NETDB_INTERNAL);					      \
+	  status = NSS_STATUS_TRYAGAIN;					      \
+	  return status;						      \
+	}								      \
+      key = sbuf.data;							      \
 									      \
       KEYPRINTF keypattern;						      \
     }									      \
@@ -216,6 +225,7 @@  enum nss_status								      \
 									      \
  out:									      \
   internal_endent (&state);						      \
+  scratch_buffer_free (&sbuf);						      \
 									      \
   return status;							      \
 }