Hi, While working on a particularly nasty module, I discovered a few bugs in ap_hook.c relating to reuse of va_lists. On some platforms, va_lists cannot be reused in mulitple function calls. For example, this:
void e(va_list f, int g); void a(int b, ...) { va_list c; int d; va_start(c, b); for (d=0; d<4; d++) { e(c, d); } va_end(c); } is unsafe - on some platforms, calls to va_arg in the second call to e() will not return arguments starting with the argument after 'b'. Instead, va_arg will return garbage. The only such platform I know of at the moment is s390 Linux, but there are probably others. Anyway, here's a patch to mod_ssl 2.8.15 that fixes ap_hook_call() and ap_hook_use(). I've simply moved all the code that calls va_ functions inside the for(hooks) loops in those functions. I haven't checked for other instances of va_list reuse in mod_ssl, but my guess is someone would have noticed by now if they were anywhere less obscure than these. You probably don't want to know what I'm doing to make this problem show up. -jonathan. --- pkg.eapi/ap_hook.c 2003-09-24 17:23:59.000000000 +1000 +++ pkg.eapi/ap_hook.c 2003-09-24 18:15:32.000000000 +1000 @@ -314,23 +314,6 @@ va_list ap; int rc; - va_start(ap, modeid); - - if (modeid == AP_HOOK_MODE_DECLINE || modeid == AP_HOOK_MODE_DECLTMP) { - if (AP_HOOK_SIG_HAS(sig, RC, char)) - modeval.v_char = va_arg(ap, va_type(char)); - else if (AP_HOOK_SIG_HAS(sig, RC, int)) - modeval.v_int = va_arg(ap, va_type(int)); - else if (AP_HOOK_SIG_HAS(sig, RC, long)) - modeval.v_long = va_arg(ap, va_type(long)); - else if (AP_HOOK_SIG_HAS(sig, RC, float)) - modeval.v_float = va_arg(ap, va_type(float)); - else if (AP_HOOK_SIG_HAS(sig, RC, double)) - modeval.v_double = va_arg(ap, va_type(double)); - else if (AP_HOOK_SIG_HAS(sig, RC, ptr)) - modeval.v_ptr = va_arg(ap, va_type(ptr)); - } - if ((he = ap_hook_create(hook)) == NULL) return FALSE; @@ -341,9 +324,29 @@ he->he_modeval = modeval; } - for (i = 0; he->he_func[i] != NULL; i++) - if (ap_hook_call_func(ap, he, he->he_func[i])) + for (i = 0; he->he_func[i] != NULL; i++) { + va_start(ap, modeid); + + if (modeid == AP_HOOK_MODE_DECLINE || modeid == AP_HOOK_MODE_DECLTMP) { + if (AP_HOOK_SIG_HAS(sig, RC, char)) + modeval.v_char = va_arg(ap, va_type(char)); + else if (AP_HOOK_SIG_HAS(sig, RC, int)) + modeval.v_int = va_arg(ap, va_type(int)); + else if (AP_HOOK_SIG_HAS(sig, RC, long)) + modeval.v_long = va_arg(ap, va_type(long)); + else if (AP_HOOK_SIG_HAS(sig, RC, float)) + modeval.v_float = va_arg(ap, va_type(float)); + else if (AP_HOOK_SIG_HAS(sig, RC, double)) + modeval.v_double = va_arg(ap, va_type(double)); + else if (AP_HOOK_SIG_HAS(sig, RC, ptr)) + modeval.v_ptr = va_arg(ap, va_type(ptr)); + } + if (ap_hook_call_func(ap, he, he->he_func[i])) { + va_end(ap); break; + } + va_end(ap); + } if (i > 0 && he->he_modeid == AP_HOOK_MODE_ALL) rc = TRUE; @@ -352,7 +355,6 @@ else rc = TRUE; - va_end(ap); return rc; } @@ -366,21 +368,23 @@ va_list ap; int rc; - va_start(ap, hook); - if ((he = ap_hook_find(hook)) == NULL) { - va_end(ap); return FALSE; } if ( he->he_sig == AP_HOOK_SIG_UNKNOWN || he->he_modeid == AP_HOOK_MODE_UNKNOWN) { - va_end(ap); return FALSE; } - for (i = 0; he->he_func[i] != NULL; i++) - if (ap_hook_call_func(ap, he, he->he_func[i])) + for (i = 0; he->he_func[i] != NULL; i++) { + va_start(ap, hook); + if (ap_hook_call_func(ap, he, he->he_func[i])) { + va_end(ap); break; + } + va_end(ap); + } + if (i > 0 && he->he_modeid == AP_HOOK_MODE_ALL) rc = TRUE; @@ -389,7 +393,6 @@ else rc = TRUE; - va_end(ap); return rc; } ______________________________________________________________________ Apache Interface to OpenSSL (mod_ssl) www.modssl.org User Support Mailing List [EMAIL PROTECTED] Automated List Manager [EMAIL PROTECTED]