Author: markj
Date: Sun May 6 00:03:24 2018
New Revision: 333278
URL: https://svnweb.freebsd.org/changeset/base/333278
Log:
Avoid dropping the topology lock in gmirror's dumpconf implementation.
Doing so introduces races which can lead to a use-after-free when
grabbing a snapshot of the GEOM mesh.
To ensure that a mirror's disk list remains stable, change its locking
protocol: both the softc lock and the topology lock are now required
to modify the list, so either lock is sufficient for traversal.
Tested by: pho
MFC after: 2 weeks
Sponsored by: Dell EMC Isilon
Modified:
head/sys/geom/mirror/g_mirror.c
Modified: head/sys/geom/mirror/g_mirror.c
==============================================================================
--- head/sys/geom/mirror/g_mirror.c Sat May 5 22:40:40 2018
(r333277)
+++ head/sys/geom/mirror/g_mirror.c Sun May 6 00:03:24 2018
(r333278)
@@ -277,8 +277,6 @@ g_mirror_ndisks(struct g_mirror_softc *sc, int state)
struct g_mirror_disk *disk;
u_int n = 0;
- sx_assert(&sc->sc_lock, SX_LOCKED);
-
LIST_FOREACH(disk, &sc->sc_disks, d_next) {
if (state == -1 || disk->d_state == state)
n++;
@@ -495,7 +493,9 @@ g_mirror_destroy_disk(struct g_mirror_disk *disk)
sc = disk->d_softc;
sx_assert(&sc->sc_lock, SX_XLOCKED);
+ g_topology_lock();
LIST_REMOVE(disk, d_next);
+ g_topology_unlock();
g_mirror_event_cancel(disk);
if (sc->sc_hint == disk)
sc->sc_hint = NULL;
@@ -522,6 +522,8 @@ static void
g_mirror_free_device(struct g_mirror_softc *sc)
{
+ g_topology_assert();
+
mtx_destroy(&sc->sc_queue_mtx);
mtx_destroy(&sc->sc_events_mtx);
mtx_destroy(&sc->sc_done_mtx);
@@ -2626,6 +2628,7 @@ again:
DISK_STATE_CHANGED();
disk->d_state = state;
+ g_topology_lock();
if (LIST_EMPTY(&sc->sc_disks))
LIST_INSERT_HEAD(&sc->sc_disks, disk, d_next);
else {
@@ -2643,6 +2646,7 @@ again:
if (dp != NULL)
LIST_INSERT_AFTER(dp, disk, d_next);
}
+ g_topology_unlock();
G_MIRROR_DEBUG(1, "Device %s: provider %s detected.",
sc->sc_name, g_mirror_get_diskname(disk));
if (sc->sc_state == G_MIRROR_DEVICE_STATE_STARTING)
@@ -3328,24 +3332,20 @@ g_mirror_dumpconf(struct sbuf *sb, const char *indent,
disk = cp->private;
if (disk == NULL)
return;
- g_topology_unlock();
- sx_xlock(&sc->sc_lock);
sbuf_printf(sb, "%s<ID>%u</ID>\n", indent, (u_int)disk->d_id);
if (disk->d_state == G_MIRROR_DISK_STATE_SYNCHRONIZING) {
sbuf_printf(sb, "%s<Synchronized>", indent);
if (disk->d_sync.ds_offset == 0)
sbuf_printf(sb, "0%%");
- else {
+ else
sbuf_printf(sb, "%u%%",
(u_int)((disk->d_sync.ds_offset * 100) /
- sc->sc_provider->mediasize));
- }
+ sc->sc_mediasize));
sbuf_printf(sb, "</Synchronized>\n");
- if (disk->d_sync.ds_offset > 0) {
+ if (disk->d_sync.ds_offset > 0)
sbuf_printf(sb, "%s<BytesSynced>%jd"
"</BytesSynced>\n", indent,
(intmax_t)disk->d_sync.ds_offset);
- }
}
sbuf_printf(sb, "%s<SyncID>%u</SyncID>\n", indent,
disk->d_sync.ds_syncid);
@@ -3380,11 +3380,7 @@ g_mirror_dumpconf(struct sbuf *sb, const char *indent,
disk->d_priority);
sbuf_printf(sb, "%s<State>%s</State>\n", indent,
g_mirror_disk_state2str(disk->d_state));
- sx_xunlock(&sc->sc_lock);
- g_topology_lock();
} else {
- g_topology_unlock();
- sx_xlock(&sc->sc_lock);
sbuf_printf(sb, "%s<Type>", indent);
switch (sc->sc_type) {
case G_MIRROR_TYPE_AUTOMATIC:
@@ -3436,8 +3432,6 @@ g_mirror_dumpconf(struct sbuf *sb, const char *indent,
else
sbuf_printf(sb, "%s", "DEGRADED");
sbuf_printf(sb, "</State>\n");
- sx_xunlock(&sc->sc_lock);
- g_topology_lock();
}
}
_______________________________________________
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
