I may not have time to fully digest the change before the release date, but
I'm not sure this snippet quite works:

   if (ctx->read_start == ctx->read_end) { /* time to read more data */
       ctx->read_end = ctx->read_start = &(ctx->buf[BUF_OFFSET]);
       ctx->read_end += BIO_read(next, ctx->read_start, ENC_BLOCK_SIZE);
   }
   i = ctx->read_end - ctx->read_start;

   if (i <= 0) {

It's kind of an odd error-checking pattern and is only saved from undefined
behavior by BUF_OFFSET. (Is a custom BIO allowed to return -1,000,000 on
error or must it be -1? There are definitely some OpenSSL APIs which return
-2 expecting that the usual error-check patterns don't care.) Anyway, I
believe it gets stuck if non-blocking BIO causes BIO_read to fail on a
retryable error like EWOULDBLOCK and we try again. I see calls to
BIO_should_retry, so I gather this BIO is intended to work in front of a
non-blocking BIO.

Since the error path should only be reachable when BIO_read fails, maybe
move that inside the "read more data" codepath? Then you don't need pointer
tricks to avoid duplicating the code.

David

On Sun, Aug 21, 2016 at 5:57 PM Andy Polyakov via RT <r...@openssl.org> wrote:

> There are two commits, one that addresses bio_enc problems and one
> adding test. Please double-check.
>
>
> --
> Ticket here: http://rt.openssl.org/Ticket/Display.html?id=4628
> Please log in as guest with password guest if prompted
>
> --
> openssl-dev mailing list
> To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
>

-- 
Ticket here: http://rt.openssl.org/Ticket/Display.html?id=4628
Please log in as guest with password guest if prompted

-- 
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev

Reply via email to