2017-12-06 19:12 GMT+03:00 Vadim Zhukov <[email protected]>:
>> The aldap_close() return value is never checked, and I do not see
>> any good reason to do that. Also, in case close(2) fails, it'll miss
>> freeing other data.
>
> I'm blind. :-\
>
> The problem I was looking for was right here: the aldap_close() closes
> the wrong file descriptor. So here is a better patch that solves
> actual leak. I ever treat this as a candidate for -STABLE, since
> when ypldap get stuck, you could be locked out of system.
Sorry for noise, I'm just trying to make this patch go in. I think it
should because it fixes a real issue seen in the wild (if an isolated
AD-enabled LAN could be called "wild"). Well, actually it fixes two
issues, but I found zero code paths for making close() fail in current
code.
The patched version happily runs for more than a week on (otherwise) 6.2-STABLE.
> Index: aldap.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ypldap/aldap.c,v
> retrieving revision 1.37
> diff -u -p -r1.37 aldap.c
> --- aldap.c 30 May 2017 09:33:31 -0000 1.37
> +++ aldap.c 6 Dec 2017 13:11:59 -0000
> @@ -67,18 +67,14 @@ aldap_application(struct ber_element *el
> return BER_TYPE_OCTETSTRING;
> }
>
> -int
> +void
> aldap_close(struct aldap *al)
> {
> if (al->fd != -1)
> - if (close(al->ber.fd) == -1)
> - return (-1);
> -
> + close(al->fd);
> ber_free(&al->ber);
> evbuffer_free(al->buf);
> free(al);
> -
> - return (0);
> }
>
> struct aldap *
> Index: aldap.h
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ypldap/aldap.h,v
> retrieving revision 1.10
> diff -u -p -r1.10 aldap.h
> --- aldap.h 30 May 2017 09:33:31 -0000 1.10
> +++ aldap.h 6 Dec 2017 13:11:59 -0000
> @@ -206,7 +206,7 @@ enum subfilter {
> struct aldap *aldap_init(int);
> int aldap_tls(struct aldap *, struct tls_config *,
> const char *);
> -int aldap_close(struct aldap *);
> +void aldap_close(struct aldap *);
> struct aldap_message *aldap_parse(struct aldap *);
> void aldap_freemsg(struct aldap_message *);
>
--
WBR,
Vadim Zhukov
