httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jeff Trawick <traw...@gmail.com>
Subject Re: Universal setting for APR_HOOK_PROBES_ENABLED
Date Mon, 09 May 2011 21:06:12 GMT
On Sun, May 8, 2011 at 7:58 AM, Jeff Trawick <trawick@gmail.com> wrote:
> a potential concern is that by doing it that way (really, that's the
> only way httpd can help out, whether the include of third-party code
> is in ap_hooks.h or elsewhere) the provider of the probe macros is "on
> the hook" to handle absolutely every hook, which can a bit tedious to
> do
>
> * having separate macros for every individual hook gives you type
> safety and the ability to ignore some or, in general, have different
> functionality for different hooks (e.g., connection-based vs.
> request-based, ignoring everything else); but that gets out of date
> (not a killer)
> * having common macros for all hooks breaks because of challenges with
> the parameter lists (a couple of hooks have 0 parameters!!!) and type
> safety (first parm isn't always a pointer, though you can look at the
> name of the hook)
> ** shall we give optional_fn_retrieve a server_rec & ?  (I suggested
> this previously on-list, but only niq responded, and not possitively
> IIRC)
> ** I forget what the other one was
the other one was mpm_name, not a likely candidate for a parameter

This patch disables hook probes for our two hooks which don't have args:

Index: server/config.c
===================================================================
--- server/config.c	(revision 1101166)
+++ server/config.c	(working copy)
@@ -167,6 +167,20 @@
 AP_IMPLEMENT_HOOK_RUN_FIRST(int, quick_handler, (request_rec *r, int lookup),
                             (r, lookup), DECLINED)

+/* hooks with no args are implemented last, after disabling APR hook probes */
+#if defined(APR_HOOK_PROBES_ENABLED)
+#undef APR_HOOK_PROBES_ENABLED
+#undef APR_HOOK_PROBE_ENTRY
+#define APR_HOOK_PROBE_ENTRY(ud,ns,name,args)
+#undef APR_HOOK_PROBE_RETURN
+#define APR_HOOK_PROBE_RETURN(ud,ns,name,rv,args)
+#undef APR_HOOK_PROBE_INVOKE
+#define APR_HOOK_PROBE_INVOKE(ud,ns,name,src,args)
+#undef APR_HOOK_PROBE_COMPLETE
+#define APR_HOOK_PROBE_COMPLETE(ud,ns,name,src,rv,args)
+#undef APR_HOOK_INT_DCL_UD
+#define APR_HOOK_INT_DCL_UD
+#endif
 AP_IMPLEMENT_HOOK_VOID(optional_fn_retrieve, (void), ())

 /****************************************************************
Index: server/mpm_common.c
===================================================================
--- server/mpm_common.c	(revision 1101166)
+++ server/mpm_common.c	(working copy)
@@ -98,9 +98,6 @@
 AP_IMPLEMENT_HOOK_RUN_FIRST(apr_status_t, mpm_register_timed_callback,
                             (apr_time_t t, ap_mpm_callback_fn_t
*cbfn, void *baton),
                             (t, cbfn, baton), APR_ENOTIMPL)
-AP_IMPLEMENT_HOOK_RUN_FIRST(const char *, mpm_get_name,
-                            (void),
-                            (), NULL)
 AP_IMPLEMENT_HOOK_VOID(end_generation,
                        (server_rec *s, ap_generation_t gen),
                        (s, gen))
@@ -108,6 +105,24 @@
                        (server_rec *s, pid_t pid, ap_generation_t
gen, int slot, mpm_child_status status),
                        (s,pid,gen,slot,status))

+/* hooks with no args are implemented last, after disabling APR hook probes */
+#if defined(APR_HOOK_PROBES_ENABLED)
+#undef APR_HOOK_PROBES_ENABLED
+#undef APR_HOOK_PROBE_ENTRY
+#define APR_HOOK_PROBE_ENTRY(ud,ns,name,args)
+#undef APR_HOOK_PROBE_RETURN
+#define APR_HOOK_PROBE_RETURN(ud,ns,name,rv,args)
+#undef APR_HOOK_PROBE_INVOKE
+#define APR_HOOK_PROBE_INVOKE(ud,ns,name,src,args)
+#undef APR_HOOK_PROBE_COMPLETE
+#define APR_HOOK_PROBE_COMPLETE(ud,ns,name,src,rv,args)
+#undef APR_HOOK_INT_DCL_UD
+#define APR_HOOK_INT_DCL_UD
+#endif
+AP_IMPLEMENT_HOOK_RUN_FIRST(const char *, mpm_get_name,
+                            (void),
+                            (), NULL)
+
 typedef struct mpm_gen_info_t {
     APR_RING_ENTRY(mpm_gen_info_t) link;
     int gen;          /* which gen? */

ugly but effective

>
> but there's probably practical no way to individually select classes
> of hooks (e.g., it would be nice to enable only for connection-based
> and request-based) other than invasive preprocessor work within .c
> files
>
> should a distinction be made for core+bundled modules vs. third-party
> code?  maybe the define to include the third-party .h file is for
> internal CPPFLAGS only, so that it doesn't blow (or get the hook probe
> implementer on the hook to handle stuff she's never heard of) when
> building third-party modules against a probe-enabled build?

fixed now

>
> as far as potential bundled exploitation:
>
> * DTrace is one flavor (not kept out of date, horrible build changes
> never cleaned up/committed)
> * it would be cool to have some teaching/debugging mode available at
> configure time which resulted in a lot of crap written to the error
> log during hook execution which a script could create a nice display
> from, combining error and access log information
> ** perhaps enabled at runtime via -Dfoo
> * another flavor I've played with is just maintaining
> r->notes{"ActiveModule"} and r->notes{"RequestFailer"} for access from
> mod_whatkilledus or access log, respectively

I rehashed that with the latest code and the ugly no-arg-hook patch
above.  Attached is a .c file implementation and a corresponding
ap_hook_probes.h for this sample feature set.  The .c file has to be
included from some httpd source file to get it linked in.  It won't
work without the workaround for our two no-arg hooks.

What's a cleaner way to add the code to the server, assuming that
somebody implementing hook probes needs to add

1) a .h file that gets included by ap_hooks.h
2) a .c file that should be linked in to httpd

?

Maybe --enable-hook-probes=/path/to/ap_hook_probes.{c,h}

Alternately, the code that needs to get linked in could be implemented
with a drop-in modules/myprobes/{config.m4,mod_myprobes.c,etc.} and
then --enable the module statically.

Mime
View raw message