Re: git: bc598959090d - main - autofs: Plug memory leak

On Sat, Aug 9, 2025 at 11:19 PM Konstantin Belousov <kostikbel@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 <ngie@FreeBSD.org>
> > AuthorDate: 2025-08-10 01:52:31 +0000
> > Commit:     Warner Losh <imp@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 trix@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