>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().>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
Darren Tucker
2021-Feb-15 23:19 UTC
[PATCH] do not free string returned by login_getcapstr
On Tue, 16 Feb 2021 at 05:49, Ed Maste <emaste at freebsd.org> 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().That seems to be a difference between FreeBSD and OpenBSD, since the latter says: 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. -- Darren Tucker (dtucker at dtucker.net) GPG key 11EAA6FA / A86E 3E07 5B19 5880 E860 37F4 9357 ECEF 11EA A6FA (new) Good judgement comes with experience. Unfortunately, the experience usually comes from bad judgement.
Damien Miller
2021-Feb-15 23:50 UTC
[PATCH] do not free string returned by login_getcapstr
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 >