[PATCH] do not free string returned by login_getcapstr

Damien Miller djm at mindrot.org
Tue Feb 16 10:50:24 AEDT 2021


On Mon, 15 Feb 2021, Ed Maste wrote:

> From the login_getcapstr man page,
> > Note that with all functions in this group, you should not call free(3)
> > on any pointers returned.  Memory allocated during retrieval or
> > processing of capability tags is automatically reused by subsequent calls
> > to functions in this group, or deallocated on calling login_close().

This seems to be a divergence between FreeBSD and OpenBSD. OpenBSD has

> CAVEATS
>     The string returned by login_getcapstr() is allocated via
>     malloc(3) when the specified capability is present and thus
>     it is the responsibility of the caller to free() this space.
>     However, if the capability was not found or an error
>     occurred and def or err (whichever is relevant) are non-NULL
>     the returned value is simply what was passed in to
>     login_getcapstr().  Therefore it is not possible to blindly
>     free() the return value without first checking it against
>     def and err.

NetBSD is idential to OpenBSD. I guess we'll need to special-case FreeBSD
and anything else that derives from that codebase. Does anyone know what
else does it the FreeBSD way? (I'm guessing Dragonfly...)

-d

> From FreeBSD d93a896ef959 by des@, "Upgrade to OpenSSH 7.5p1."
> 
> Differential Revision:	https://reviews.freebsd.org/D28617
> ---
>  session.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/session.c b/session.c
> index 27ca8a104..ec1755d21 100644
> --- a/session.c
> +++ b/session.c
> @@ -1279,7 +1279,8 @@ static void
>  do_nologin(struct passwd *pw)
>  {
>  	FILE *f = NULL;
> -	char buf[1024], *nl, *def_nl = _PATH_NOLOGIN;
> +	const char *nl;
> +	char buf[1024], *def_nl = _PATH_NOLOGIN;
>  	struct stat sb;
>  
>  #ifdef HAVE_LOGIN_CAP
> @@ -1291,11 +1292,8 @@ do_nologin(struct passwd *pw)
>  		return;
>  	nl = def_nl;
>  #endif
> -	if (stat(nl, &sb) == -1) {
> -		if (nl != def_nl)
> -			free(nl);
> +	if (stat(nl, &sb) == -1)
>  		return;
> -	}
>  
>  	/* /etc/nologin exists.  Print its contents if we can and exit. */
>  	logit("User %.100s not allowed because %s exists", pw->pw_name, nl);
> -- 
> 2.30.0
> 
> _______________________________________________
> openssh-unix-dev mailing list
> openssh-unix-dev at mindrot.org
> https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev
> 


More information about the openssh-unix-dev mailing list