From: Jeff King <peff@peff.net>
Date: Sun, 26 Jun 2016 23:42:51 GMT
Subject: Re: [PATCH 1/2] xread: retry after poll on EAGAIN/EWOULDBLOCK
Message-ID: <20160626234251.GA21668@sigill.intra.peff.net>
In-Reply-To: <20160626232112.721-2-e@80x24.org>

On Sun, Jun 26, 2016 at 11:21:11PM +0000, Eric Wong wrote:

> We should continue to loop after EAGAIN/EWOULDBLOCK as the
> intent of xread is to read as much as possible until an
> EOF or real error occurs.
> 
> Fixes: 1079c4be0b720 ("xread: poll on non blocking fds")
> 
> Signed-off-by: Eric Wong <e@80x24.org>
> ---
>  wrapper.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/wrapper.c b/wrapper.c
> index 5dc4e15..f1155d0 100644
> --- a/wrapper.c
> +++ b/wrapper.c
> @@ -255,6 +255,7 @@ ssize_t xread(int fd, void *buf, size_t len)
>  				 * call to read(2).
>  				 */
>  				poll(&pfd, 1, -1);
> +				continue;
>  			}
>  		}
>  		return nr;

Eek. This is a real bug that could cause racy and apparently random
failures. I guess we haven't seen many reports of it because it only
triggers when you have unexpected non-blocking descriptors, which are
not all that common. But your fix is definitely the right thing to do.

I also wondered how we managed to miss such an obvious point in review
of the original patch. Sadly, we _did_ notice it[1] but it looks like we
never fixed the problem. That is even more disturbing.

-Peff

[1] http://mid.gmane.org/20151218031336.GA8467@sigill.intra.peff.net