From 859f906a4b4dfc132115f10f5d372ef4281a6479 Mon Sep 17 00:00:00 2001 From: Justin Gottula Date: Thu, 15 Aug 2024 14:13:18 -0700 Subject: [PATCH] Fix null ptr deref when renaming a zvol with snaps and snapdev=visible (#16316) If a zvol is renamed, and it has one or more snapshots, and snapdev=visible is true for the zvol, then the rename causes a kernel null pointer dereference error. This has the effect (on Linux, anyway) of killing the z_zvol taskq kthread, with locks still held; which in turn causes a variety of zvol-related operations afterward to hang indefinitely (such as udev workers, among other things). The problem occurs because of an oversight in #15486 (e36ff84c338d2f7b15aef2538f6a9507115bbf4a). As documented in dataset_kstats_create, some datasets may not actually have kstats allocated for them; and at least at the present time, this is true for snapshots. In practical terms, this means that for snapshots, dk->dk_kstats will be NULL. The dataset_kstats_rename function introduced in the patch above does not first check whether dk->dk_kstats is NULL before proceeding, unlike e.g. the nearby dataset_kstats_update_* functions. In the very particular circumstance in which a zvol is renamed, AND that zvol has one or more snapshots, AND that zvol also has snapdev=visible, zvol_rename_minors_impl will loop over not just the zvol dataset itself, but each of the zvol's snapshots as well, so that their device nodes will be renamed as well. This results in dataset_kstats_create being called for snapshots, where, as we've established, dk->dk_kstats is NULL. Fix this by simply adding a NULL check before doing anything in dataset_kstats_rename. This still allows the dataset_name kstat value for the zvol to be updated (as was the intent of the original patch), and merely blocks attempts by the code to act upon the zvol's non-kstat-having snapshots. If at some future time, kstats are added for snapshots, then things should work as intended in that case as well. Signed-off-by: Justin Gottula Reviewed-by: Rob Norris Reviewed-by: Alexander Motin Reviewed-by: Alan Somers Reviewed-by: Allan Jude Reviewed-by: Tony Hutter --- module/zfs/dataset_kstats.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/module/zfs/dataset_kstats.c b/module/zfs/dataset_kstats.c index 2ac058fd2c..5abee12434 100644 --- a/module/zfs/dataset_kstats.c +++ b/module/zfs/dataset_kstats.c @@ -201,6 +201,9 @@ dataset_kstats_destroy(dataset_kstats_t *dk) void dataset_kstats_rename(dataset_kstats_t *dk, const char *name) { + if (dk->dk_kstats == NULL) + return; + dataset_kstat_values_t *dkv = dk->dk_kstats->ks_data; char *ds_name;