From 4279c5e9063890986b0a23671f9500bf90eff2c4 Mon Sep 17 00:00:00 2001 From: Tomas Bzatek Date: Thu, 9 Feb 2023 08:20:52 -0500 Subject: [PATCH] udiskslinuxdevice: Fix dm-multipath ATA drives handling The dm-multipath devices are actually capable of tunneling ioctls down to the drive through an active path. It might be actually dangerous to approach the particular paths directly, in parallel or async. This change refrains from any probing on ATA devices which are part of a dm-multipath device. The actual dm-multipath device is probed instead. However the dm-multipath device carries only a small number of udev attributes that still need to be retrieved from the particular path. --- doc/udisks2-sections.txt.daemon.sections.in | 2 + src/udiskslinuxdevice.c | 73 ++++++++++++++++++++- src/udiskslinuxdevice.h | 7 +- src/udiskslinuxdrive.c | 2 +- src/udiskslinuxdriveata.c | 61 +++++++++++++---- src/udiskslinuxdriveobject.c | 15 +---- src/udiskslinuxprovider.c | 4 +- 7 files changed, 132 insertions(+), 32 deletions(-) diff --git a/doc/udisks2-sections.txt.daemon.sections.in b/doc/udisks2-sections.txt.daemon.sections.in index f35a104f2d..458258783f 100644 --- a/doc/udisks2-sections.txt.daemon.sections.in +++ b/doc/udisks2-sections.txt.daemon.sections.in @@ -283,6 +283,8 @@ udisks_linux_device_read_sysfs_attr_as_int udisks_linux_device_read_sysfs_attr_as_uint64 udisks_linux_device_subsystem_is_nvme udisks_linux_device_nvme_is_fabrics +udisks_linux_device_is_dm_multipath +udisks_linux_device_is_mpath_device_path UDISKS_TYPE_LINUX_DEVICE UDISKS_LINUX_DEVICE diff --git a/src/udiskslinuxdevice.c b/src/udiskslinuxdevice.c index 8e098cbbcb..916302abba 100644 --- a/src/udiskslinuxdevice.c +++ b/src/udiskslinuxdevice.c @@ -99,6 +99,7 @@ static gboolean probe_ata (UDisksLinuxDevice *device, /** * udisks_linux_device_new_sync: * @udev_device: A #GUdevDevice. + * @udev_client: A #GUdevClient. * * Creates a new #UDisksLinuxDevice from @udev_device which includes * probing the device for more information, if applicable. @@ -109,7 +110,7 @@ static gboolean probe_ata (UDisksLinuxDevice *device, * Returns: A #UDisksLinuxDevice. */ UDisksLinuxDevice * -udisks_linux_device_new_sync (GUdevDevice *udev_device) +udisks_linux_device_new_sync (GUdevDevice *udev_device, GUdevClient *udev_client) { UDisksLinuxDevice *device; GError *error = NULL; @@ -122,7 +123,7 @@ udisks_linux_device_new_sync (GUdevDevice *udev_device) /* No point in probing on remove events */ if (!(g_strcmp0 (g_udev_device_get_action (udev_device), "remove") == 0)) { - if (!udisks_linux_device_reprobe_sync (device, NULL, &error)) + if (!udisks_linux_device_reprobe_sync (device, udev_client, NULL, &error)) goto out; } @@ -142,6 +143,7 @@ udisks_linux_device_new_sync (GUdevDevice *udev_device) /** * udisks_linux_device_reprobe_sync: * @device: A #UDisksLinuxDevice. + * @udev_client: A #GUdevClient. * @cancellable: (allow-none): A #GCancellable or %NULL. * @error: Return location for error or %NULL. * @@ -149,10 +151,14 @@ udisks_linux_device_new_sync (GUdevDevice *udev_device) * blocked for a non-trivial amount of time while the probing is * underway. * + * Probing is dm-multipath aware in which case an active path + * is looked up and udev attributes are fetched from there. + * * Returns: %TRUE if reprobing succeeded, %FALSE otherwise. */ gboolean udisks_linux_device_reprobe_sync (UDisksLinuxDevice *device, + GUdevClient *udev_client, GCancellable *cancellable, GError **error) { @@ -167,7 +173,8 @@ udisks_linux_device_reprobe_sync (UDisksLinuxDevice *device, g_udev_device_get_property_as_boolean (device->udev_device, "ID_ATA") && !g_udev_device_has_property (device->udev_device, "ID_USB_TYPE") && !g_udev_device_has_property (device->udev_device, "ID_USB_DRIVER") && - !g_udev_device_has_property (device->udev_device, "ID_USB_MODEL")) + !g_udev_device_has_property (device->udev_device, "ID_USB_MODEL") && + !udisks_linux_device_is_mpath_device_path (device)) { if (!probe_ata (device, cancellable, error)) goto out; @@ -218,6 +225,34 @@ udisks_linux_device_reprobe_sync (UDisksLinuxDevice *device, if (!device->nvme_ns_info) goto out; } + else + /* Probe the dm-multipath devices */ + if (g_strcmp0 (g_udev_device_get_subsystem (device->udev_device), "block") == 0 && + g_strcmp0 (g_udev_device_get_devtype (device->udev_device), "disk") == 0 && + udisks_linux_device_is_dm_multipath (device)) + { + gboolean is_ata = FALSE; + gchar **slaves; + guint n; + + slaves = udisks_daemon_util_resolve_links (g_udev_device_get_sysfs_path (device->udev_device), "slaves"); + for (n = 0; slaves[n] != NULL; n++) + { + GUdevDevice *slave; + + slave = g_udev_client_query_by_sysfs_path (udev_client, slaves[n]); + if (slave != NULL) + { + is_ata |= g_udev_device_get_property_as_boolean (slave, "ID_ATA"); + g_object_unref (slave); + } + if (is_ata) + break; + } + g_strfreev (slaves); + if (is_ata && !probe_ata (device, cancellable, error)) + goto out; + } ret = TRUE; @@ -463,3 +498,35 @@ udisks_linux_device_nvme_is_fabrics (UDisksLinuxDevice *device) return FALSE; } + +/** + * udisks_linux_device_is_dm_multipath: + * @device: A #UDisksLinuxDevice. + * + * Returns: %TRUE if @device represents a dm-multipath block device, + * %FALSE otherwise. + */ +gboolean +udisks_linux_device_is_dm_multipath (UDisksLinuxDevice *device) +{ + const gchar *dm_uuid; + + if (g_udev_device_get_property_as_int (device->udev_device, "MPATH_DEVICE_READY") == 1) + return TRUE; + + dm_uuid = g_udev_device_get_sysfs_attr (device->udev_device, "dm/uuid"); + return dm_uuid != NULL && g_str_has_prefix (dm_uuid, "mpath-"); +} + +/** + * udisks_linux_device_is_mpath_device_path: + * @device: A #UDisksLinuxDevice. + * + * Returns: %TRUE if @device is an I/O path that is part of a dm-multipath + * device, %FALSE otherwise. + */ +gboolean +udisks_linux_device_is_mpath_device_path (UDisksLinuxDevice *device) +{ + return g_udev_device_get_property_as_int (device->udev_device, "DM_MULTIPATH_DEVICE_PATH") == 1; +} diff --git a/src/udiskslinuxdevice.h b/src/udiskslinuxdevice.h index 43e7d9701a..70a134afd1 100644 --- a/src/udiskslinuxdevice.h +++ b/src/udiskslinuxdevice.h @@ -57,8 +57,10 @@ struct _UDisksLinuxDevice }; GType udisks_linux_device_get_type (void) G_GNUC_CONST; -UDisksLinuxDevice *udisks_linux_device_new_sync (GUdevDevice *udev_device); +UDisksLinuxDevice *udisks_linux_device_new_sync (GUdevDevice *udev_device, + GUdevClient *udev_client); gboolean udisks_linux_device_reprobe_sync (UDisksLinuxDevice *device, + GUdevClient *udev_client, GCancellable *cancellable, GError **error); @@ -75,6 +77,9 @@ guint64 udisks_linux_device_read_sysfs_attr_as_uint64 (UDisksLinuxDev gboolean udisks_linux_device_subsystem_is_nvme (UDisksLinuxDevice *device); gboolean udisks_linux_device_nvme_is_fabrics (UDisksLinuxDevice *device); +gboolean udisks_linux_device_is_dm_multipath (UDisksLinuxDevice *device); +gboolean udisks_linux_device_is_mpath_device_path (UDisksLinuxDevice *device); + G_END_DECLS #endif /* __UDISKS_LINUX_DEVICE_H__ */ diff --git a/src/udiskslinuxdrive.c b/src/udiskslinuxdrive.c index 92d8234ab1..e584e5b969 100644 --- a/src/udiskslinuxdrive.c +++ b/src/udiskslinuxdrive.c @@ -1630,7 +1630,7 @@ handle_power_off (UDisksDrive *_drive, } fd = -1; - device = udisks_linux_drive_object_get_device (object, TRUE /* get_hw */); + device = udisks_linux_drive_object_get_device (object, FALSE /* get_hw */); if (device == NULL) { g_dbus_method_invocation_return_error (invocation, diff --git a/src/udiskslinuxdriveata.c b/src/udiskslinuxdriveata.c index e74f2e1218..24e9b2dc23 100644 --- a/src/udiskslinuxdriveata.c +++ b/src/udiskslinuxdriveata.c @@ -372,7 +372,7 @@ udisks_linux_drive_ata_update (UDisksLinuxDriveAta *drive, UDisksLinuxDriveObject *object) { UDisksLinuxDevice *device; - device = udisks_linux_drive_object_get_device (object, TRUE /* get_hw */); + device = udisks_linux_drive_object_get_device (object, FALSE /* get_hw */); if (device == NULL) goto out; @@ -423,6 +423,24 @@ static gboolean update_io_stats (UDisksLinuxDriveAta *drive, UDisksLinuxDevice * return noio; } +static BDExtraArg ** +build_smart_extra_args (UDisksLinuxDevice *device) +{ + BDExtraArg **extra; + + if (!udisks_linux_device_is_dm_multipath (device)) + return NULL; + + /* Now that we're in UDisksLinuxDriveAta it means the device already passed + * earlier selection for hardware type. The most likely scenario here are + * SATA drives connected to a SAS HBA, so hardcode the SCSI to ATA Translation (SAT) + * protocol for ATA PASS THROUGH SCSI. + */ + extra = g_new0 (BDExtraArg *, 2); + extra[0] = bd_extra_arg_new ("--device=sat,auto", NULL); + return extra; +} + /** * udisks_linux_drive_ata_refresh_smart_sync: * @drive: The #UDisksLinuxDriveAta to refresh. @@ -469,7 +487,7 @@ udisks_linux_drive_ata_refresh_smart_sync (UDisksLinuxDriveAta *drive, goto out; } - device = udisks_linux_drive_object_get_device (object, TRUE /* get_hw */); + device = udisks_linux_drive_object_get_device (object, FALSE /* get_hw */); if (device == NULL) { g_set_error_literal (error, UDISKS_ERROR, UDISKS_ERROR_FAILED, @@ -499,6 +517,7 @@ udisks_linux_drive_ata_refresh_smart_sync (UDisksLinuxDriveAta *drive, } else { + BDExtraArg **extra; guchar count; gboolean noio = FALSE; gboolean awake; @@ -518,8 +537,11 @@ udisks_linux_drive_ata_refresh_smart_sync (UDisksLinuxDriveAta *drive, goto out_io; } + extra = build_smart_extra_args (device); data = bd_smart_ata_get_info (g_udev_device_get_device_file (device->udev_device), + (const BDExtraArg **) extra, &l_error); + bd_extra_arg_list_free (extra); } if (data == NULL) @@ -583,12 +605,13 @@ udisks_linux_drive_ata_smart_selftest_sync (UDisksLinuxDriveAta *drive, GError *l_error = NULL; gboolean ret = FALSE; BDSmartSelfTestOp op; + BDExtraArg **extra = NULL; object = udisks_daemon_util_dup_object (drive, error); if (object == NULL) goto out; - device = udisks_linux_drive_object_get_device (object, TRUE /* get_hw */); + device = udisks_linux_drive_object_get_device (object, FALSE /* get_hw */); if (device == NULL) { g_set_error_literal (error, UDISKS_ERROR, UDISKS_ERROR_FAILED, @@ -615,8 +638,11 @@ udisks_linux_drive_ata_smart_selftest_sync (UDisksLinuxDriveAta *drive, goto out; } + extra = build_smart_extra_args (device); if (!bd_smart_device_self_test (g_udev_device_get_device_file (device->udev_device), - op, &l_error)) + op, + (const BDExtraArg **) extra, + &l_error)) { g_set_error_literal (error, UDISKS_ERROR, UDISKS_ERROR_FAILED, @@ -628,6 +654,7 @@ udisks_linux_drive_ata_smart_selftest_sync (UDisksLinuxDriveAta *drive, ret = TRUE; out: + bd_extra_arg_list_free (extra); g_clear_object (&device); g_clear_object (&object); return ret; @@ -661,7 +688,7 @@ handle_smart_update (UDisksDriveAta *_drive, } daemon = udisks_linux_drive_object_get_daemon (object); - block_object = udisks_linux_drive_object_get_block (object, TRUE); + block_object = udisks_linux_drive_object_get_block (object, FALSE); if (block_object == NULL) { g_dbus_method_invocation_return_error (invocation, @@ -818,7 +845,7 @@ handle_smart_selftest_abort (UDisksDriveAta *_drive, } daemon = udisks_linux_drive_object_get_daemon (object); - block_object = udisks_linux_drive_object_get_block (object, TRUE); + block_object = udisks_linux_drive_object_get_block (object, FALSE); if (block_object == NULL) { g_dbus_method_invocation_return_error (invocation, @@ -1042,7 +1069,7 @@ handle_smart_selftest_start (UDisksDriveAta *_drive, } daemon = udisks_linux_drive_object_get_daemon (object); - block_object = udisks_linux_drive_object_get_block (object, TRUE); + block_object = udisks_linux_drive_object_get_block (object, FALSE); if (block_object == NULL) { g_dbus_method_invocation_return_error (invocation, @@ -1145,11 +1172,13 @@ handle_smart_set_enabled (UDisksDriveAta *_drive, UDisksLinuxDriveObject *object = NULL; UDisksLinuxBlockObject *block_object = NULL; UDisksDaemon *daemon; + UDisksLinuxProvider *provider; GError *error = NULL; UDisksLinuxDevice *device = NULL; const gchar *message; const gchar *action_id; uid_t caller_uid; + BDExtraArg **extra = NULL; object = udisks_daemon_util_dup_object (drive, &error); if (object == NULL) @@ -1169,6 +1198,7 @@ handle_smart_set_enabled (UDisksDriveAta *_drive, } daemon = udisks_linux_drive_object_get_daemon (object); + provider = udisks_daemon_get_linux_provider (daemon); if (!udisks_daemon_util_get_caller_uid_sync (daemon, invocation, NULL /* GCancellable */, @@ -1219,8 +1249,11 @@ handle_smart_set_enabled (UDisksDriveAta *_drive, goto out; } + extra = build_smart_extra_args (device); if (!bd_smart_set_enabled (g_udev_device_get_device_file (device->udev_device), - value, &error)) + value, + (const BDExtraArg **) extra, + &error)) { if (g_error_matches (error, BD_SMART_ERROR, BD_SMART_ERROR_TECH_UNAVAIL)) { @@ -1274,7 +1307,9 @@ handle_smart_set_enabled (UDisksDriveAta *_drive, } /* Reread new IDENTIFY data */ - if (!udisks_linux_device_reprobe_sync (device, NULL, &error)) + if (!udisks_linux_device_reprobe_sync (device, + udisks_linux_provider_get_udev_client (provider), + NULL, &error)) { g_prefix_error (&error, "Error reprobing device: "); g_dbus_method_invocation_take_error (invocation, error); @@ -1305,6 +1340,7 @@ handle_smart_set_enabled (UDisksDriveAta *_drive, udisks_drive_ata_complete_smart_set_enabled (_drive, invocation); out: + bd_extra_arg_list_free (extra); g_clear_object (&device); g_clear_object (&block_object); g_clear_object (&object); @@ -1369,7 +1405,7 @@ udisks_linux_drive_ata_get_pm_state (UDisksLinuxDriveAta *drive, /* TODO: some SSD controllers may block for considerable time when trimming large amount of blocks */ - device = udisks_linux_drive_object_get_device (object, TRUE /* get_hw */); + device = udisks_linux_drive_object_get_device (object, FALSE /* get_hw */); if (device == NULL) { g_set_error_literal (error, UDISKS_ERROR, UDISKS_ERROR_FAILED, @@ -1529,7 +1565,7 @@ handle_pm_standby_wakeup (UDisksDriveAta *_drive, invocation)) goto out; - device = udisks_linux_drive_object_get_device (object, TRUE /* get_hw */); + device = udisks_linux_drive_object_get_device (object, FALSE /* get_hw */); if (device == NULL) { g_dbus_method_invocation_return_error (invocation, UDISKS_ERROR, UDISKS_ERROR_FAILED, @@ -2030,7 +2066,7 @@ udisks_linux_drive_ata_secure_erase_sync (UDisksLinuxDriveAta *drive, daemon = udisks_linux_drive_object_get_daemon (object); - device = udisks_linux_drive_object_get_device (object, TRUE /* get_hw */); + device = udisks_linux_drive_object_get_device (object, FALSE /* get_hw */); if (device == NULL) { g_set_error (&local_error, UDISKS_ERROR, UDISKS_ERROR_FAILED, @@ -2405,7 +2441,6 @@ handle_security_erase_unit (UDisksDriveAta *_drive, /* ---------------------------------------------------------------------------------------------------- */ - static void drive_ata_iface_init (UDisksDriveAtaIface *iface) { diff --git a/src/udiskslinuxdriveobject.c b/src/udiskslinuxdriveobject.c index 16227a0d2c..3535604b71 100644 --- a/src/udiskslinuxdriveobject.c +++ b/src/udiskslinuxdriveobject.c @@ -234,15 +234,6 @@ strip_and_replace_with_uscore (gchar *s) ; } -static gboolean -is_dm_multipath (UDisksLinuxDevice *device) -{ - const gchar *dm_uuid; - - dm_uuid = g_udev_device_get_sysfs_attr (device->udev_device, "dm/uuid"); - return dm_uuid != NULL && g_str_has_prefix (dm_uuid, "mpath-"); -} - static void udisks_linux_drive_object_constructed (GObject *_object) { @@ -437,7 +428,7 @@ udisks_linux_drive_object_get_device (UDisksLinuxDriveObject *object, g_mutex_lock (&object->devices_mutex); for (devices = object->devices; devices; devices = devices->next) { - if (!get_hw || !is_dm_multipath (UDISKS_LINUX_DEVICE (devices->data))) + if (!get_hw || !udisks_linux_device_is_dm_multipath (UDISKS_LINUX_DEVICE (devices->data))) { ret = devices->data; break; @@ -486,7 +477,7 @@ udisks_linux_drive_object_get_block (UDisksLinuxDriveObject *object, device = udisks_linux_block_object_get_device (UDISKS_LINUX_BLOCK_OBJECT (iter_object)); skip = (g_strcmp0 (g_udev_device_get_devtype (device->udev_device), "disk") != 0 - || (get_hw && is_dm_multipath (device))); + || (get_hw && udisks_linux_device_is_dm_multipath (device))); g_object_unref (device); if (skip) @@ -1049,7 +1040,7 @@ udisks_linux_drive_object_should_include_device (GUdevClient *client, } /* dm-multipath */ - if (is_dm_multipath (device)) + if (udisks_linux_device_is_dm_multipath (device)) { gchar **slaves; guint n; diff --git a/src/udiskslinuxprovider.c b/src/udiskslinuxprovider.c index 293a85e34f..483f4060e4 100644 --- a/src/udiskslinuxprovider.c +++ b/src/udiskslinuxprovider.c @@ -327,7 +327,7 @@ probe_request_thread_func (gpointer user_data) continue; /* probe the device - this may take a while */ - request->udisks_device = udisks_linux_device_new_sync (request->udev_device); + request->udisks_device = udisks_linux_device_new_sync (request->udev_device, provider->gudev_client); /* now that we've probed the device, post the request back to the main thread */ g_idle_add (on_idle_with_probed_uevent, request); @@ -565,7 +565,7 @@ get_udisks_devices (UDisksLinuxProvider *provider) GUdevDevice *device = G_UDEV_DEVICE (l->data); if (!g_udev_device_get_is_initialized (device)) continue; - udisks_devices = g_list_prepend (udisks_devices, udisks_linux_device_new_sync (device)); + udisks_devices = g_list_prepend (udisks_devices, udisks_linux_device_new_sync (device, provider->gudev_client)); } udisks_devices = g_list_reverse (udisks_devices); g_list_free_full (devices, g_object_unref);