On Sat, Sep 14, 2019 at 07:17:15AM +0200, Thorsten Kukuk wrote:
On Fri, Sep 13, Dmitry V. Levin wrote:
> I'm not sure I understand the intent here.
> Does it make sense to enable econf (by default) when vendordir is not enabled?
> Does it make sens to enable vendordir when econf is not available?
> It seems to me that these two have to be either both enabled or both
> disabled, please correct me if I got it wrong.
It makes sense to enable vendordir even if you don't have econf.
econf is only used to read login.defs, but for an alternate pam.d
directory or securetty file, it's not used.
OK, does it make sense to enable econf by default if vendordir is not
enabled?
> > --- a/libpam/pam_modutil_searchkey.c
> > +++ b/libpam/pam_modutil_searchkey.c
> > @@ -13,9 +13,39 @@
> > #include <string.h>
> > #include <stdlib.h>
> > #include <ctype.h>
> > +#ifdef USE_ECONF
> > +#include <libeconf.h>
> > +#endif
> >
> > #define BUF_SIZE 8192
> >
> > +#ifdef USE_ECONF
> > +#define LOGIN_DEFS "/etc/login.defs"
> > +
> > +#ifndef VENDORDIR
> > +#define VENDORDIR NULL
> > +#endif
> > +
> > +static char *
> > +econf_search_key (const char *name, const char *suffix, const char *key)
> > +{
> > + econf_file *key_file = NULL;
> > + char *val;
> > +
> > + if (econf_readDirs (&key_file, VENDORDIR, SYSCONFDIR, name, suffix,
> > + " \t", "#"))
> > + return NULL;
> > +
> > + if (econf_getStringValue (key_file, NULL, key, &val))
> > + return NULL;
> > +
> > + econf_free (key_file);
>
> I'm not familiar with econf API, but this looks like a key_file leak.
No, clearly not.
I mean, shouldn't key_file be freed regardless of econf_getStringValue
return code?
--
ldv