racy dumpconf interface

[Mark Johnston]

Hi,

Currently, when fetching a snapshot of the mesh we call the class'
dumpconf method once for each consumer:

254         LIST_FOREACH(cp2, &gp->consumer, consumer) {
255                 if (cp != NULL && cp != cp2)
256                         continue;
257                 g_conf_consumer(sb, cp2); /* calls dumpconf */
258         }

Some dumpconf implementations (such as gmirror's) drop the topology lock
to acquire an internal lock which comes first in the lock order. The
problem with this is that cp2 may be freed while the topology lock is
dropped. The problem doesn't really affect g_geom and g_provider
pointers because they are freed only by the g_event thread (I think),
which is also the thread which calls g_conf_geom(). This is not true of
consumers, however, so it's possible to hit a use-after-free in the list
traversal pasted above.

One solution for gmirror's case is to simply avoid acquiring the softc
lock when fetching a snapshot of the consumer. The topology lock is
sufficient to ensure that cp->private doesn't get freed in the middle of
the dumpconf invocation. However, this of course means that we may
record an inconsistent snapshot of the disk's info.

Another solution would be to fetch info for all consumers in a single
call to dumpconf. This way the consumer list of a given GEOM may be
traversed safely after dropping and reacquiring the topology lock.
However, this would probably involve changing the dumpconf interface (or
adding a "dumpconf_consumers" class method).

Does anyone have any thoughts or preferences on this matter?