On Sat, Aug 9, 2025 at 11:19 PM Konstantin Belousov <kostik...@gmail.com> wrote:
>
> On Sun, Aug 10, 2025 at 01:56:22AM +0000, Warner Losh wrote:
> > The branch main has been updated by imp:
> >
> > URL:
> > https://cgit.FreeBSD.org/src/commit/?id=bc598959090d43aa0fc6b91355979016a9449041
> >
> > commit bc598959090d43aa0fc6b91355979016a9449041
> > Author: Enji Cooper <n...@freebsd.org>
> > AuthorDate: 2025-08-10 01:52:31 +0000
> > Commit: Warner Losh <i...@freebsd.org>
> > CommitDate: 2025-08-10 01:54:42 +0000
> >
> > autofs: Plug memory leak
> >
> > Originally, this was an extra free, but ngie@ suggested this
> > change. Since that's the whole thing, I've set her as the author for
> > this ancient review instead of t...@juniper.net.
> >
> > Sugggested by: ngie
> > Differential Revision: https://reviews.freebsd.org/D10063
> > Sponsored by: Netflix
> > ---
> > usr.sbin/autofs/common.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/usr.sbin/autofs/common.c b/usr.sbin/autofs/common.c
> > index 18756752876c..6b98214162ae 100644
> > --- a/usr.sbin/autofs/common.c
> > +++ b/usr.sbin/autofs/common.c
> > @@ -149,7 +149,7 @@ create_directory(const char *path)
> > error = mkdir(partial, 0755);
> > if (error != 0 && errno != EEXIST) {
> > log_warn("cannot create %s", partial);
> > - return;
> > + break;
> > }
> > }
> >
> Isn't there another leak, occuring when the first break in the loop is taken?
> Then the 'partial' duped string is not freed.
I think it's always leaked. checked_strdup allocates it, then concat
allocates a new string that's set to partial. So no matter what, we
should free it, no?
Eg
diff --git a/usr.sbin/autofs/common.c b/usr.sbin/autofs/common.c
index 6b98214162ae..2dd7c290cebc 100644
--- a/usr.sbin/autofs/common.c
+++ b/usr.sbin/autofs/common.c
@@ -153,6 +153,7 @@ create_directory(const char *path)
}
}
+ free(partial);
free(tofree);
}
Warner
