From: Omar Polo <op@omarpolo.com>
Subject: Re: aldap_parse: infinit loop when connection closed
To: Philipp <philipp+openbsd@bureaucracy.de>
Cc: tech@openbsd.org
Date: Tue, 20 Feb 2024 19:21:27 +0100

Hello,

On 2024/02/19 14:10:37 +0100, Philipp <philipp+openbsd@bureaucracy.de> wrote:
> Hi
> 
> I noticed that aldap_parse() get stuck in an infinit loop when the fd is
> closed. The read loop only breaks when successfull parse a message or
> the read fails. But read() on a closed fd return 0 not -1. A patch is
> attached.
> 
> Philipp

I don't use ldap so can't properly test, but the proposed diff in itself
seems fine to me.  We should handle the interrupted connection
gracefully.  Maybe it could use a different error type since it's not
really a parser error, but I'm nitpicking.

so, fwiw, ok op@ :)

Thanks!

> diff --git a/libexec/login_ldap/aldap.c b/libexec/login_ldap/aldap.c
> index d5f5769f5d5..e1bd5c04600 100644
> --- a/libexec/login_ldap/aldap.c
> +++ b/libexec/login_ldap/aldap.c
> @@ -369,7 +369,7 @@ aldap_parse(struct aldap *ldap)
>  			} else
>  				ret = read(ldap->fd, rbuf, sizeof(rbuf));
>  
> -			if (ret == -1) {
> +			if (ret <= 0) {
>  				goto parsefail;
>  			}
>  
> diff --git a/usr.bin/ldap/aldap.c b/usr.bin/ldap/aldap.c
> index aee14a62a7a..26ffd8a6055 100644
> --- a/usr.bin/ldap/aldap.c
> +++ b/usr.bin/ldap/aldap.c
> @@ -369,7 +369,7 @@ aldap_parse(struct aldap *ldap)
>  			} else
>  				ret = read(ldap->fd, rbuf, sizeof(rbuf));
>  
> -			if (ret == -1) {
> +			if (ret <= 0) {
>  				goto parsefail;
>  			}
>  
> diff --git a/usr.sbin/ypldap/aldap.c b/usr.sbin/ypldap/aldap.c
> index 4efedbeffdb..6fb1c75cc61 100644
> --- a/usr.sbin/ypldap/aldap.c
> +++ b/usr.sbin/ypldap/aldap.c
> @@ -405,7 +405,7 @@ aldap_parse(struct aldap *ldap)
>  			} else
>  				ret = read(ldap->fd, rbuf, sizeof(rbuf));
>  
> -			if (ret == -1) {
> +			if (ret <= 0) {
>  				goto parsefail;
>  			}
>