From 3252573ac733beb35c8fd6b5a9cbb0f014131c57 Mon Sep 17 00:00:00 2001 From: Richard Hughes Date: Thu, 10 Dec 2020 15:26:51 +0000 Subject: [PATCH] synaptics-prometheus: Fix flashing a fingerprint reader that is in use The fprint daemon only keeps the device open for 5 seconds and then releases it, which seems like a small window to hit. But! We're asking the user to authenticate with the same device we're about to upgrade so a different part of the stack woke up the hardware just before we're about to deploy an update onto it. Just retry a few times to make sure the device is idle. Use a flag to prevent accidentally causing regressions in other plugins. Fixes https://github.com/fwupd/fwupd/issues/2650 --- libfwupd/fwupd-enums.c | 4 + libfwupd/fwupd-enums.h | 2 + libfwupdplugin/fu-device.c | 74 ++++++++++++++++--- libfwupdplugin/fu-device.h | 7 ++ libfwupdplugin/fwupdplugin.map | 6 ++ .../synaptics-prometheus/fu-synaprom-device.c | 1 + src/fu-util-common.c | 4 + 7 files changed, 87 insertions(+), 11 deletions(-) diff --git a/libfwupd/fwupd-enums.c b/libfwupd/fwupd-enums.c index 7404b993b..c815eb61f 100644 --- a/libfwupd/fwupd-enums.c +++ b/libfwupd/fwupd-enums.c @@ -203,6 +203,8 @@ fwupd_device_flag_to_string (FwupdDeviceFlags device_flag) return "has-multiple-branches"; if (device_flag == FWUPD_DEVICE_FLAG_BACKUP_BEFORE_INSTALL) return "backup-before-install"; + if (device_flag == FWUPD_DEVICE_FLAG_RETRY_OPEN) + return "retry-open"; if (device_flag == FWUPD_DEVICE_FLAG_UNKNOWN) return "unknown"; return NULL; @@ -307,6 +309,8 @@ fwupd_device_flag_from_string (const gchar *device_flag) return FWUPD_DEVICE_FLAG_HAS_MULTIPLE_BRANCHES; if (g_strcmp0 (device_flag, "backup-before-install") == 0) return FWUPD_DEVICE_FLAG_BACKUP_BEFORE_INSTALL; + if (g_strcmp0 (device_flag, "retry-open") == 0) + return FWUPD_DEVICE_FLAG_RETRY_OPEN; return FWUPD_DEVICE_FLAG_UNKNOWN; } diff --git a/libfwupd/fwupd-enums.h b/libfwupd/fwupd-enums.h index dc5459610..540253d18 100644 --- a/libfwupd/fwupd-enums.h +++ b/libfwupd/fwupd-enums.h @@ -129,6 +129,7 @@ typedef enum { * @FWUPD_DEVICE_FLAG_HAS_MULTIPLE_BRANCHES: Device supports switching to a different stream of firmware * @FWUPD_DEVICE_FLAG_BACKUP_BEFORE_INSTALL: Device firmware should be saved before installing firmware * @FWUPD_DEVICE_FLAG_MD_SET_ICON: Set the device icon from the metadata if available + * @FWUPD_DEVICE_FLAG_RETRY_OPEN: Retry the device open up to 5 times if it fails * * The device flags. **/ @@ -175,6 +176,7 @@ typedef enum { #define FWUPD_DEVICE_FLAG_HAS_MULTIPLE_BRANCHES (1llu << 39) /* Since: 1.5.0 */ #define FWUPD_DEVICE_FLAG_BACKUP_BEFORE_INSTALL (1llu << 40) /* Since: 1.5.0 */ #define FWUPD_DEVICE_FLAG_MD_SET_ICON (1llu << 41) /* Since: 1.5.2 */ +#define FWUPD_DEVICE_FLAG_RETRY_OPEN (1llu << 42) /* Since: 1.5.5 */ #define FWUPD_DEVICE_FLAG_UNKNOWN G_MAXUINT64 /* Since: 0.7.3 */ typedef guint64 FwupdDeviceFlags; diff --git a/libfwupdplugin/fu-device.c b/libfwupdplugin/fu-device.c index 25c2cc6b9..ea52eabc2 100644 --- a/libfwupdplugin/fu-device.c +++ b/libfwupdplugin/fu-device.c @@ -20,6 +20,9 @@ #include "fwupd-common.h" #include "fwupd-device-private.h" +#define FU_DEVICE_RETRY_OPEN_COUNT 5 +#define FU_DEVICE_RETRY_OPEN_DELAY 500 /* ms */ + /** * SECTION:fu-device * @short_description: a physical or logical device @@ -233,10 +236,11 @@ fu_device_retry_set_delay (FuDevice *self, guint delay) } /** - * fu_device_retry: + * fu_device_retry_full: * @self: A #FuDevice * @func: (scope async): A function to execute * @count: The number of tries to try the function + * @delay: The delay between each try in ms * @user_data: (nullable): a helper to pass to @user_data * @error: A #GError * @@ -249,14 +253,15 @@ fu_device_retry_set_delay (FuDevice *self, guint delay) * If the reset function returns %FALSE, then the function returns straight away * without processing any pending retries. * - * Since: 1.4.0 + * Since: 1.5.5 **/ gboolean -fu_device_retry (FuDevice *self, - FuDeviceRetryFunc func, - guint count, - gpointer user_data, - GError **error) +fu_device_retry_full (FuDevice *self, + FuDeviceRetryFunc func, + guint count, + guint delay, + gpointer user_data, + GError **error) { FuDevicePrivate *priv = GET_PRIVATE (self); @@ -269,8 +274,8 @@ fu_device_retry (FuDevice *self, g_autoptr(GError) error_local = NULL; /* delay */ - if (i > 0 && priv->retry_delay > 0) - g_usleep (priv->retry_delay * 1000); + if (i > 0 && delay > 0) + g_usleep (delay * 1000); /* run function, if success return success */ if (func (self, user_data, &error_local)) @@ -323,6 +328,38 @@ fu_device_retry (FuDevice *self, return TRUE; } +/** + * fu_device_retry: + * @self: A #FuDevice + * @func: (scope async): A function to execute + * @count: The number of tries to try the function + * @user_data: (nullable): a helper to pass to @user_data + * @error: A #GError + * + * Calls a specific function a number of times, optionally handling the error + * with a reset action. + * + * If fu_device_retry_add_recovery() has not been used then all errors are + * considered non-fatal until the last try. + * + * If the reset function returns %FALSE, then the function returns straight away + * without processing any pending retries. + * + * Since: 1.4.0 + **/ +gboolean +fu_device_retry (FuDevice *self, + FuDeviceRetryFunc func, + guint count, + gpointer user_data, + GError **error) +{ + FuDevicePrivate *priv = GET_PRIVATE (self); + return fu_device_retry_full (self, func, count, + priv->retry_delay, + user_data, error); +} + /** * fu_device_poll: * @self: A #FuDevice @@ -2755,6 +2792,13 @@ fu_device_cleanup (FuDevice *self, FwupdInstallFlags flags, GError **error) return klass->cleanup (self, flags, error); } +static gboolean +fu_device_open_cb (FuDevice *self, gpointer user_data, GError **error) +{ + FuDeviceClass *klass = FU_DEVICE_GET_CLASS (self); + return klass->open (self, error); +} + /** * fu_device_open: * @self: A #FuDevice @@ -2796,8 +2840,16 @@ fu_device_open (FuDevice *self, GError **error) /* subclassed */ if (klass->open != NULL) { - if (!klass->open (self, error)) - return FALSE; + if (fu_device_has_flag (self, FWUPD_DEVICE_FLAG_RETRY_OPEN)) { + if (!fu_device_retry_full (self, fu_device_open_cb, + FU_DEVICE_RETRY_OPEN_COUNT, + FU_DEVICE_RETRY_OPEN_DELAY, + NULL, error)) + return FALSE; + } else { + if (!klass->open (self, error)) + return FALSE; + } } /* setup */ diff --git a/libfwupdplugin/fu-device.h b/libfwupdplugin/fu-device.h index 5707f939f..35e7b7343 100644 --- a/libfwupdplugin/fu-device.h +++ b/libfwupdplugin/fu-device.h @@ -374,6 +374,13 @@ gboolean fu_device_retry (FuDevice *self, gpointer user_data, GError **error) G_GNUC_WARN_UNUSED_RESULT; +gboolean fu_device_retry_full (FuDevice *self, + FuDeviceRetryFunc func, + guint count, + guint delay, + gpointer user_data, + GError **error) + G_GNUC_WARN_UNUSED_RESULT; gboolean fu_device_bind_driver (FuDevice *self, const gchar *subsystem, const gchar *driver, diff --git a/libfwupdplugin/fwupdplugin.map b/libfwupdplugin/fwupdplugin.map index e3d5440b5..e48345fe6 100644 --- a/libfwupdplugin/fwupdplugin.map +++ b/libfwupdplugin/fwupdplugin.map @@ -699,3 +699,9 @@ LIBFWUPDPLUGIN_1.5.4 { fu_common_bytes_new_offset; local: *; } LIBFWUPDPLUGIN_1.5.3; + +LIBFWUPDPLUGIN_1.5.5 { + global: + fu_device_retry_full; + local: *; +} LIBFWUPDPLUGIN_1.5.4; diff --git a/plugins/synaptics-prometheus/fu-synaprom-device.c b/plugins/synaptics-prometheus/fu-synaprom-device.c index 26ba80ead..e1320e509 100644 --- a/plugins/synaptics-prometheus/fu-synaprom-device.c +++ b/plugins/synaptics-prometheus/fu-synaprom-device.c @@ -440,6 +440,7 @@ fu_synaprom_device_init (FuSynapromDevice *self) { fu_device_add_flag (FU_DEVICE (self), FWUPD_DEVICE_FLAG_UPDATABLE); fu_device_add_flag (FU_DEVICE (self), FWUPD_DEVICE_FLAG_CAN_VERIFY); + fu_device_add_flag (FU_DEVICE (self), FWUPD_DEVICE_FLAG_RETRY_OPEN); fu_device_set_version_format (FU_DEVICE (self), FWUPD_VERSION_FORMAT_TRIPLET); fu_device_set_protocol (FU_DEVICE (self), "com.synaptics.prometheus"); fu_device_set_remove_delay (FU_DEVICE (self), FU_DEVICE_REMOVE_DELAY_RE_ENUMERATE); diff --git a/src/fu-util-common.c b/src/fu-util-common.c index 3cef59c82..9b1cdd780 100644 --- a/src/fu-util-common.c +++ b/src/fu-util-common.c @@ -1120,6 +1120,10 @@ fu_util_device_flag_to_string (guint64 device_flag) /* skip */ return NULL; } + if (device_flag == FWUPD_DEVICE_FLAG_RETRY_OPEN) { + /* skip */ + return NULL; + } if (device_flag == FWUPD_DEVICE_FLAG_UNKNOWN) { return NULL; }