Skip to content

Commit

Permalink
Fix null ptr deref when renaming a zvol with snaps and snapdev=visible (
Browse files Browse the repository at this point in the history
openzfs#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 openzfs#15486
(e36ff84). 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 <justin@jgottula.com>
Reviewed-by: Rob Norris <robn@despairlabs.com>
Reviewed-by: Alexander Motin <mav@FreeBSD.org>
Reviewed-by: Alan Somers <asomers@gmail.com>
Reviewed-by: Allan Jude <allan@klarasystems.com>
Reviewed-by: Tony Hutter <hutter2@llnl.gov>
  • Loading branch information
jgottula authored and lundman committed Sep 4, 2024
1 parent 05dbd54 commit ce2a076
Showing 1 changed file with 3 additions and 0 deletions.
3 changes: 3 additions & 0 deletions module/zfs/dataset_kstats.c
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,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;

Expand Down

0 comments on commit ce2a076

Please sign in to comment.