d/freebsd-skq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Avoid dropping the topology lock in gmirror's dumpconf implementation.

Browse Source 
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
This commit is contained in:
markj 2018-05-06 00:03:24 +00:00
parent d4a327c789
commit 3579270d7e
1 changed files with 9 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

24
sys/geom/mirror/g_mirror.c
View File

@ -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, struct g_geom *gp,
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, struct g_geom *gp,
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, struct g_geom *gp,
else
sbuf_printf(sb, "%s", "DEGRADED");
sbuf_printf(sb, "</State>\n");
sx_xunlock(&sc->sc_lock);
g_topology_lock();
}
}