Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

drbd: validate resync_after dependency on attach already

We validated resync_after dependencies, if changed via disk-options.
But we did not validate them when first created via attach.
We also did not check or cleanup dependencies that used to be correct,
but now point to meanwhile removed minor devices.

If the drbd_resync_after_valid() validation in disk-options tried to
follow a dependency chain in this way, this could lead to NULL pointer
dereference.

Validate resync_after settings in drbd_adm_attach() already, as well as
in drbd_adm_disk_opts(), and and only reject dependency loops.
Depending on non-existing disks is allowed and equivalent to no dependency.

Signed-off-by: Philipp Reisner <philipp.reisner@linbit.com>
Signed-off-by: Lars Ellenberg <lars.ellenberg@linbit.com>
Signed-off-by: Jens Axboe <axboe@kernel.dk>

authored by

Lars Ellenberg and committed by
Jens Axboe
a3f8f7dc 94ad0a10

+18 -3
+6
drivers/block/drbd/drbd_nl.c
··· 1381 1381 goto fail; 1382 1382 } 1383 1383 1384 + write_lock_irq(&global_state_lock); 1385 + retcode = drbd_resync_after_valid(mdev, new_disk_conf->resync_after); 1386 + write_unlock_irq(&global_state_lock); 1387 + if (retcode != NO_ERROR) 1388 + goto fail; 1389 + 1384 1390 rcu_read_lock(); 1385 1391 nc = rcu_dereference(mdev->tconn->net_conf); 1386 1392 if (nc) {
+12 -3
drivers/block/drbd/drbd_worker.c
··· 1426 1426 int resync_after; 1427 1427 1428 1428 while (1) { 1429 - if (!odev->ldev) 1429 + if (!odev->ldev || odev->state.disk == D_DISKLESS) 1430 1430 return 1; 1431 1431 rcu_read_lock(); 1432 1432 resync_after = rcu_dereference(odev->ldev->disk_conf)->resync_after; ··· 1434 1434 if (resync_after == -1) 1435 1435 return 1; 1436 1436 odev = minor_to_mdev(resync_after); 1437 - if (!expect(odev)) 1437 + if (!odev) 1438 1438 return 1; 1439 1439 if ((odev->state.conn >= C_SYNC_SOURCE && 1440 1440 odev->state.conn <= C_PAUSED_SYNC_T) || ··· 1516 1516 1517 1517 if (o_minor == -1) 1518 1518 return NO_ERROR; 1519 - if (o_minor < -1 || minor_to_mdev(o_minor) == NULL) 1519 + if (o_minor < -1 || o_minor > MINORMASK) 1520 1520 return ERR_RESYNC_AFTER; 1521 1521 1522 1522 /* check for loops */ ··· 1524 1524 while (1) { 1525 1525 if (odev == mdev) 1526 1526 return ERR_RESYNC_AFTER_CYCLE; 1527 + 1528 + /* You are free to depend on diskless, non-existing, 1529 + * or not yet/no longer existing minors. 1530 + * We only reject dependency loops. 1531 + * We cannot follow the dependency chain beyond a detached or 1532 + * missing minor. 1533 + */ 1534 + if (!odev || !odev->ldev || odev->state.disk == D_DISKLESS) 1535 + return NO_ERROR; 1527 1536 1528 1537 rcu_read_lock(); 1529 1538 resync_after = rcu_dereference(odev->ldev->disk_conf)->resync_after;