* [PATCH] getpw: Get rid of alloca
@ 2023-07-07 20:04 Joe Simmons-Talbott
2023-08-10 13:45 ` Joe Simmons-Talbott
2023-08-28 17:01 ` Adhemerval Zanella Netto
0 siblings, 2 replies; 5+ messages in thread
From: Joe Simmons-Talbott @ 2023-07-07 20:04 UTC (permalink / raw)
To: libc-alpha; +Cc: Joe Simmons-Talbott
Use a scratch_buffer rather than alloca to avoid potential stack
overflow.
---
pwd/getpw.c | 34 +++++++++++++++++++++++++++-------
1 file changed, 27 insertions(+), 7 deletions(-)
diff --git a/pwd/getpw.c b/pwd/getpw.c
index cf747374b8..7a27d79910 100644
--- a/pwd/getpw.c
+++ b/pwd/getpw.c
@@ -15,8 +15,8 @@
License along with the GNU C Library; if not, see
<https://www.gnu.org/licenses/>. */
-#include <alloca.h>
#include <errno.h>
+#include <scratch_buffer.h>
#include <stdio.h>
#include <unistd.h>
#include <pwd.h>
@@ -34,28 +34,48 @@ __getpw (__uid_t uid, char *buf)
size_t buflen;
char *tmpbuf;
struct passwd resbuf, *p;
+ int retval = 0;
+ struct scratch_buffer sbuf;
+ scratch_buffer_init (&sbuf);
if (buf == NULL)
{
__set_errno (EINVAL);
- return -1;
+ retval = -1;
+ goto error_out;
}
buflen = __sysconf (_SC_GETPW_R_SIZE_MAX);
- tmpbuf = alloca (buflen);
+ if (!scratch_buffer_set_array_size (&sbuf, 1, buflen))
+ {
+ retval = -1;
+ goto error_out;
+ }
+ tmpbuf = sbuf.data;
if (__getpwuid_r (uid, &resbuf, tmpbuf, buflen, &p) != 0)
- return -1;
+ {
+ retval = -1;
+ goto error_out;
+ }
if (p == NULL)
- return -1;
+ {
+ retval = -1;
+ goto error_out;
+ }
if (sprintf (buf, "%s:%s:%lu:%lu:%s:%s:%s", p->pw_name, p->pw_passwd,
(unsigned long int) p->pw_uid, (unsigned long int) p->pw_gid,
p->pw_gecos, p->pw_dir, p->pw_shell) < 0)
- return -1;
+ {
+ retval = -1;
+ goto error_out;
+ }
- return 0;
+error_out:
+ scratch_buffer_free (&sbuf);
+ return retval;
}
weak_alias (__getpw, getpw)
--
2.39.2
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] getpw: Get rid of alloca
2023-07-07 20:04 [PATCH] getpw: Get rid of alloca Joe Simmons-Talbott
@ 2023-08-10 13:45 ` Joe Simmons-Talbott
2023-08-28 13:21 ` Joe Simmons-Talbott
2023-08-28 17:01 ` Adhemerval Zanella Netto
1 sibling, 1 reply; 5+ messages in thread
From: Joe Simmons-Talbott @ 2023-08-10 13:45 UTC (permalink / raw)
To: libc-alpha
On Fri, Jul 07, 2023 at 04:04:00PM -0400, Joe Simmons-Talbott wrote:
> Use a scratch_buffer rather than alloca to avoid potential stack
> overflow.
Ping.
Thanks,
Joe
> ---
> pwd/getpw.c | 34 +++++++++++++++++++++++++++-------
> 1 file changed, 27 insertions(+), 7 deletions(-)
>
> diff --git a/pwd/getpw.c b/pwd/getpw.c
> index cf747374b8..7a27d79910 100644
> --- a/pwd/getpw.c
> +++ b/pwd/getpw.c
> @@ -15,8 +15,8 @@
> License along with the GNU C Library; if not, see
> <https://www.gnu.org/licenses/>. */
>
> -#include <alloca.h>
> #include <errno.h>
> +#include <scratch_buffer.h>
> #include <stdio.h>
> #include <unistd.h>
> #include <pwd.h>
> @@ -34,28 +34,48 @@ __getpw (__uid_t uid, char *buf)
> size_t buflen;
> char *tmpbuf;
> struct passwd resbuf, *p;
> + int retval = 0;
> + struct scratch_buffer sbuf;
> + scratch_buffer_init (&sbuf);
>
> if (buf == NULL)
> {
> __set_errno (EINVAL);
> - return -1;
> + retval = -1;
> + goto error_out;
> }
>
> buflen = __sysconf (_SC_GETPW_R_SIZE_MAX);
> - tmpbuf = alloca (buflen);
> + if (!scratch_buffer_set_array_size (&sbuf, 1, buflen))
> + {
> + retval = -1;
> + goto error_out;
> + }
> + tmpbuf = sbuf.data;
>
> if (__getpwuid_r (uid, &resbuf, tmpbuf, buflen, &p) != 0)
> - return -1;
> + {
> + retval = -1;
> + goto error_out;
> + }
>
> if (p == NULL)
> - return -1;
> + {
> + retval = -1;
> + goto error_out;
> + }
>
> if (sprintf (buf, "%s:%s:%lu:%lu:%s:%s:%s", p->pw_name, p->pw_passwd,
> (unsigned long int) p->pw_uid, (unsigned long int) p->pw_gid,
> p->pw_gecos, p->pw_dir, p->pw_shell) < 0)
> - return -1;
> + {
> + retval = -1;
> + goto error_out;
> + }
>
> - return 0;
> +error_out:
> + scratch_buffer_free (&sbuf);
> + return retval;
> }
> weak_alias (__getpw, getpw)
>
> --
> 2.39.2
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] getpw: Get rid of alloca
2023-08-10 13:45 ` Joe Simmons-Talbott
@ 2023-08-28 13:21 ` Joe Simmons-Talbott
0 siblings, 0 replies; 5+ 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:45:47AM -0400, Joe Simmons-Talbott via Libc-alpha wrote:
> On Fri, Jul 07, 2023 at 04:04:00PM -0400, Joe Simmons-Talbott wrote:
> > Use a scratch_buffer rather than alloca to avoid potential stack
> > overflow.
>
> Ping.
>
> Thanks,
> Joe
> > ---
> > pwd/getpw.c | 34 +++++++++++++++++++++++++++-------
> > 1 file changed, 27 insertions(+), 7 deletions(-)
> >
> > diff --git a/pwd/getpw.c b/pwd/getpw.c
> > index cf747374b8..7a27d79910 100644
> > --- a/pwd/getpw.c
> > +++ b/pwd/getpw.c
> > @@ -15,8 +15,8 @@
> > License along with the GNU C Library; if not, see
> > <https://www.gnu.org/licenses/>. */
> >
> > -#include <alloca.h>
> > #include <errno.h>
> > +#include <scratch_buffer.h>
> > #include <stdio.h>
> > #include <unistd.h>
> > #include <pwd.h>
> > @@ -34,28 +34,48 @@ __getpw (__uid_t uid, char *buf)
> > size_t buflen;
> > char *tmpbuf;
> > struct passwd resbuf, *p;
> > + int retval = 0;
> > + struct scratch_buffer sbuf;
> > + scratch_buffer_init (&sbuf);
> >
> > if (buf == NULL)
> > {
> > __set_errno (EINVAL);
> > - return -1;
> > + retval = -1;
> > + goto error_out;
> > }
> >
> > buflen = __sysconf (_SC_GETPW_R_SIZE_MAX);
> > - tmpbuf = alloca (buflen);
> > + if (!scratch_buffer_set_array_size (&sbuf, 1, buflen))
> > + {
> > + retval = -1;
> > + goto error_out;
> > + }
> > + tmpbuf = sbuf.data;
> >
> > if (__getpwuid_r (uid, &resbuf, tmpbuf, buflen, &p) != 0)
> > - return -1;
> > + {
> > + retval = -1;
> > + goto error_out;
> > + }
> >
> > if (p == NULL)
> > - return -1;
> > + {
> > + retval = -1;
> > + goto error_out;
> > + }
> >
> > if (sprintf (buf, "%s:%s:%lu:%lu:%s:%s:%s", p->pw_name, p->pw_passwd,
> > (unsigned long int) p->pw_uid, (unsigned long int) p->pw_gid,
> > p->pw_gecos, p->pw_dir, p->pw_shell) < 0)
> > - return -1;
> > + {
> > + retval = -1;
> > + goto error_out;
> > + }
> >
> > - return 0;
> > +error_out:
> > + scratch_buffer_free (&sbuf);
> > + return retval;
> > }
> > weak_alias (__getpw, getpw)
> >
> > --
> > 2.39.2
> >
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] getpw: Get rid of alloca
2023-07-07 20:04 [PATCH] getpw: Get rid of alloca Joe Simmons-Talbott
2023-08-10 13:45 ` Joe Simmons-Talbott
@ 2023-08-28 17:01 ` Adhemerval Zanella Netto
2023-08-28 20:37 ` Joe Simmons-Talbott
1 sibling, 1 reply; 5+ messages in thread
From: Adhemerval Zanella Netto @ 2023-08-28 17:01 UTC (permalink / raw)
To: libc-alpha, Joe Simmons-Talbott
On 07/07/23 17:04, Joe Simmons-Talbott via Libc-alpha wrote:
> Use a scratch_buffer rather than alloca to avoid potential stack
> overflow.
> ---
> pwd/getpw.c | 34 +++++++++++++++++++++++++++-------
> 1 file changed, 27 insertions(+), 7 deletions(-)
>
> diff --git a/pwd/getpw.c b/pwd/getpw.c
> index cf747374b8..7a27d79910 100644
> --- a/pwd/getpw.c
> +++ b/pwd/getpw.c
> @@ -15,8 +15,8 @@
> License along with the GNU C Library; if not, see
> <https://www.gnu.org/licenses/>. */
>
> -#include <alloca.h>
> #include <errno.h>
> +#include <scratch_buffer.h>
> #include <stdio.h>
> #include <unistd.h>
> #include <pwd.h>
> @@ -34,28 +34,48 @@ __getpw (__uid_t uid, char *buf)
> size_t buflen;
> char *tmpbuf;
> struct passwd resbuf, *p;
> + int retval = 0;
> + struct scratch_buffer sbuf;
> + scratch_buffer_init (&sbuf);
>
> if (buf == NULL)
> {
> __set_errno (EINVAL);
> - return -1;
> + retval = -1;
> + goto error_out;
> }
>
There is no need to call scratch_buffer_free here. You can move the
scratch_buffer initialization later.
> buflen = __sysconf (_SC_GETPW_R_SIZE_MAX);
> - tmpbuf = alloca (buflen);
> + if (!scratch_buffer_set_array_size (&sbuf, 1, buflen))
The _SC_GETPW_R_SIZE_MAX will be always NSS_BUFLEN_PASSWD so there is no need to
a scratch_buffer here (similar to sysdeps/posix/cuserid.c assumption). Since the
functions is historical tricky to be used correctly, I think it should continue to
fail with passwords larger than _SC_GETPW_R_SIZE_MAX.
> + {
> + retval = -1;
> + goto error_out;
> + }
> + tmpbuf = sbuf.data;
>
> if (__getpwuid_r (uid, &resbuf, tmpbuf, buflen, &p) != 0)
> - return -1;
> + {
> + retval = -1;
> + goto error_out;
> + }
>
> if (p == NULL)
> - return -1;
> + {
> + retval = -1;
> + goto error_out;
> + }
>
> if (sprintf (buf, "%s:%s:%lu:%lu:%s:%s:%s", p->pw_name, p->pw_passwd,
> (unsigned long int) p->pw_uid, (unsigned long int) p->pw_gid,
> p->pw_gecos, p->pw_dir, p->pw_shell) < 0)
> - return -1;
> + {
> + retval = -1;
> + goto error_out;
> + }
>
> - return 0;
> +error_out:
> + scratch_buffer_free (&sbuf);
> + return retval;
> }
> weak_alias (__getpw, getpw)
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] getpw: Get rid of alloca
2023-08-28 17:01 ` Adhemerval Zanella Netto
@ 2023-08-28 20:37 ` Joe Simmons-Talbott
0 siblings, 0 replies; 5+ messages in thread
From: Joe Simmons-Talbott @ 2023-08-28 20:37 UTC (permalink / raw)
To: Adhemerval Zanella Netto; +Cc: libc-alpha
On Mon, Aug 28, 2023 at 02:01:54PM -0300, Adhemerval Zanella Netto wrote:
>
>
> On 07/07/23 17:04, Joe Simmons-Talbott via Libc-alpha wrote:
> > Use a scratch_buffer rather than alloca to avoid potential stack
> > overflow.
> > ---
> > pwd/getpw.c | 34 +++++++++++++++++++++++++++-------
> > 1 file changed, 27 insertions(+), 7 deletions(-)
> >
> > diff --git a/pwd/getpw.c b/pwd/getpw.c
> > index cf747374b8..7a27d79910 100644
> > --- a/pwd/getpw.c
> > +++ b/pwd/getpw.c
> > @@ -15,8 +15,8 @@
> > License along with the GNU C Library; if not, see
> > <https://www.gnu.org/licenses/>. */
> >
> > -#include <alloca.h>
> > #include <errno.h>
> > +#include <scratch_buffer.h>
> > #include <stdio.h>
> > #include <unistd.h>
> > #include <pwd.h>
> > @@ -34,28 +34,48 @@ __getpw (__uid_t uid, char *buf)
> > size_t buflen;
> > char *tmpbuf;
> > struct passwd resbuf, *p;
> > + int retval = 0;
> > + struct scratch_buffer sbuf;
> > + scratch_buffer_init (&sbuf);
> >
> > if (buf == NULL)
> > {
> > __set_errno (EINVAL);
> > - return -1;
> > + retval = -1;
> > + goto error_out;
> > }
> >
>
> There is no need to call scratch_buffer_free here. You can move the
> scratch_buffer initialization later.
>
> > buflen = __sysconf (_SC_GETPW_R_SIZE_MAX);
> > - tmpbuf = alloca (buflen);
> > + if (!scratch_buffer_set_array_size (&sbuf, 1, buflen))
>
> The _SC_GETPW_R_SIZE_MAX will be always NSS_BUFLEN_PASSWD so there is no need to
> a scratch_buffer here (similar to sysdeps/posix/cuserid.c assumption). Since the
> functions is historical tricky to be used correctly, I think it should continue to
> fail with passwords larger than _SC_GETPW_R_SIZE_MAX.
I removed the scratch_buffer in v2 and used a fixed sized array.
Thanks,
Joe
>
> > + {
> > + retval = -1;
> > + goto error_out;
> > + }
> > + tmpbuf = sbuf.data;
> >
> > if (__getpwuid_r (uid, &resbuf, tmpbuf, buflen, &p) != 0)
> > - return -1;
> > + {
> > + retval = -1;
> > + goto error_out;
> > + }
> >
> > if (p == NULL)
> > - return -1;
> > + {
> > + retval = -1;
> > + goto error_out;
> > + }
> >
> > if (sprintf (buf, "%s:%s:%lu:%lu:%s:%s:%s", p->pw_name, p->pw_passwd,
> > (unsigned long int) p->pw_uid, (unsigned long int) p->pw_gid,
> > p->pw_gecos, p->pw_dir, p->pw_shell) < 0)
> > - return -1;
> > + {
> > + retval = -1;
> > + goto error_out;
> > + }
> >
> > - return 0;
> > +error_out:
> > + scratch_buffer_free (&sbuf);
> > + return retval;
> > }
> > weak_alias (__getpw, getpw)
> >
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-08-28 20:37 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-07-07 20:04 [PATCH] getpw: Get rid of alloca Joe Simmons-Talbott
2023-08-10 13:45 ` Joe Simmons-Talbott
2023-08-28 13:21 ` Joe Simmons-Talbott
2023-08-28 17:01 ` Adhemerval Zanella Netto
2023-08-28 20:37 ` 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).