You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by David Deaves <Da...@dd.id.au> on 2003/05/10 09:34:08 UTC
PATCH - Uninitialized Variables
I've submitted this patch under BugID 19242, but thought I would submit
it here as well so it doesn't get overlooked.
In modules/ssl/ssl_engine_io.c of Apache 2.0.45
I traced the problem to the 'block' member of
the 'bio_filter_in_ctx_t' structure not being
initialized before being used.
So I added initializatoins for the 'block' member
and the 'mode' member (also not initialized but
didn't contribute to our problem) to the
'ssl_io_input_add_filter' function.
Of interest, the unitialized values ended up being
bad ones after a SSL connection to a MicroSoft ISA
server. The ISA server doesn't send a 'close_notify'
at the end of the session.
--- modules/ssl/ssl_engine_io-old.c 2003-02-27 22:57:34.000000000 +1100
+++ modules/ssl/ssl_engine_io.c 2003-05-10 02:35:14.000000000 +1000
@@ -1360,6 +1360,8 @@
inctx->bb = apr_brigade_create(c->pool, c->bucket_alloc);
inctx->cbuf.length = 0;
+ inctx->block = APR_BLOCK_READ;
+ inctx->mode = AP_MODE_READBYTES;
inctx->pool = c->pool;
}
--
David Deaves <Da...@dd.id.au> +61 413 003 552
"Luck is the residue of design" - Branch Rickey
Re: PATCH - Uninitialized Variables
Posted by Jeff Trawick <tr...@attglobal.net>.
Justin Erenkrantz wrote:
> --On Wednesday, May 14, 2003 12:05 AM -0400 Jeff Trawick
> <tr...@attglobal.net> wrote:
>
>> patch looks very good to me, but I'm extremely unfamiliar with mod_ssl...
>>
>> if nobody chimes in I'll commit it to 2.1-dev in a couple of days
>
>
> Hmm. Look at line ssl_engine_io.c:1306 (1223 in 2.0) in HEAD. Where is
> inctx->mode and inctx->block used before that?
nowhere, upon closer examination
Re: PATCH - Uninitialized Variables
Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
--On Wednesday, May 14, 2003 12:05 AM -0400 Jeff Trawick
<tr...@attglobal.net> wrote:
> patch looks very good to me, but I'm extremely unfamiliar with mod_ssl...
>
> if nobody chimes in I'll commit it to 2.1-dev in a couple of days
Hmm. Look at line ssl_engine_io.c:1306 (1223 in 2.0) in HEAD. Where is
inctx->mode and inctx->block used before that?
I'd be curious to know exactly where it is used. My hunch is that it might be
used in the negotiation before the input filters are explicitly called. Not
clear though. Would like to know for sure before we add the initialization.
-- justin
Re: PATCH - Uninitialized Variables
Posted by Jeff Trawick <tr...@attglobal.net>.
David Deaves wrote:
> I've submitted this patch under BugID 19242, but thought I would submit
> it here as well so it doesn't get overlooked.
>
> In modules/ssl/ssl_engine_io.c of Apache 2.0.45
>
> I traced the problem to the 'block' member of
> the 'bio_filter_in_ctx_t' structure not being
> initialized before being used.
wow, it is apr_palloc()-ed :(
> So I added initializatoins for the 'block' member
> and the 'mode' member (also not initialized but
> didn't contribute to our problem) to the
> 'ssl_io_input_add_filter' function.
patch looks very good to me, but I'm extremely unfamiliar with mod_ssl...
if nobody chimes in I'll commit it to 2.1-dev in a couple of days
Re: PATCH - Uninitialized Variables
Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
--On Friday, May 16, 2003 1:44 AM -0500 "William A. Rowe, Jr."
<wr...@rowe-clan.net> wrote:
> The reason folks are confused about -why- this is a bug, is that we really
> need to reset these on each call to ssl_io_filter_output. Reason is simple,
> ssl output may be both read and write, however we must block on read
> when this involves a blocking write through ssl_io_filter_output (ssl needs
> a chance to completely handshake the required data.)
Ah, yes, that explains it.
> My gut reaction is that they are paired so we should tie them together
> as a single filter_ctx_t shared by the input and output filters. Thoughts?
I assume you mean joining the bio_filter_in/out_ctx_t's as we already have a
unified ssl_filter_ctx_t (filter_ctx in both bio_filter_in and
bio_filter_out). ISTR, we tried this (unified bio ctx_t) once before and it
got really complicated to understand what was what. My thought is to keep
them split and add the reciprocal pointers and ensure that the output BIO
filter always (re)sets the input BIO to blocking.
That should make it obvious what we're trying to do. -- justin
Re: PATCH - Uninitialized Variables
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
At 02:34 AM 5/10/2003, David Deaves wrote:
>I've submitted this patch under BugID 19242, but thought I would submit
>it here as well so it doesn't get overlooked.
>
>--- modules/ssl/ssl_engine_io-old.c 2003-02-27 22:57:34.000000000 +1100
>+++ modules/ssl/ssl_engine_io.c 2003-05-10 02:35:14.000000000 +1000
>@@ -1360,6 +1360,8 @@
> inctx->bb = apr_brigade_create(c->pool, c->bucket_alloc);
>
> inctx->cbuf.length = 0;
>+ inctx->block = APR_BLOCK_READ;
>+ inctx->mode = AP_MODE_READBYTES;
>
> inctx->pool = c->pool;
> }
The reason folks are confused about -why- this is a bug, is that we really
need to reset these on each call to ssl_io_filter_output. Reason is simple,
ssl output may be both read and write, however we must block on read
when this involves a blocking write through ssl_io_filter_output (ssl needs
a chance to completely handshake the required data.)
So the two line patch above is the right thought in the wrong place, it
needs to go in the ssl_io_filter_output() implementation. However, it
seems our inctx contains the filter_ctx (output) reference, but that
isn't reciprocal.
My gut reaction is that they are paired so we should tie them together
as a single filter_ctx_t shared by the input and output filters. Thoughts?
Bill