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
This commit is contained in:
Richard Hughes 2020-12-10 15:26:51 +00:00
parent 23cd466ef1
commit 3252573ac7
7 changed files with 87 additions and 11 deletions

View File

@ -203,6 +203,8 @@ fwupd_device_flag_to_string (FwupdDeviceFlags device_flag)
return "has-multiple-branches"; return "has-multiple-branches";
if (device_flag == FWUPD_DEVICE_FLAG_BACKUP_BEFORE_INSTALL) if (device_flag == FWUPD_DEVICE_FLAG_BACKUP_BEFORE_INSTALL)
return "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) if (device_flag == FWUPD_DEVICE_FLAG_UNKNOWN)
return "unknown"; return "unknown";
return NULL; return NULL;
@ -307,6 +309,8 @@ fwupd_device_flag_from_string (const gchar *device_flag)
return FWUPD_DEVICE_FLAG_HAS_MULTIPLE_BRANCHES; return FWUPD_DEVICE_FLAG_HAS_MULTIPLE_BRANCHES;
if (g_strcmp0 (device_flag, "backup-before-install") == 0) if (g_strcmp0 (device_flag, "backup-before-install") == 0)
return FWUPD_DEVICE_FLAG_BACKUP_BEFORE_INSTALL; 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; return FWUPD_DEVICE_FLAG_UNKNOWN;
} }

View File

@ -129,6 +129,7 @@ typedef enum {
* @FWUPD_DEVICE_FLAG_HAS_MULTIPLE_BRANCHES: Device supports switching to a different stream of firmware * @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_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_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. * 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_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_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_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 */ #define FWUPD_DEVICE_FLAG_UNKNOWN G_MAXUINT64 /* Since: 0.7.3 */
typedef guint64 FwupdDeviceFlags; typedef guint64 FwupdDeviceFlags;

View File

@ -20,6 +20,9 @@
#include "fwupd-common.h" #include "fwupd-common.h"
#include "fwupd-device-private.h" #include "fwupd-device-private.h"
#define FU_DEVICE_RETRY_OPEN_COUNT 5
#define FU_DEVICE_RETRY_OPEN_DELAY 500 /* ms */
/** /**
* SECTION:fu-device * SECTION:fu-device
* @short_description: a physical or logical 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 * @self: A #FuDevice
* @func: (scope async): A function to execute * @func: (scope async): A function to execute
* @count: The number of tries to try the function * @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 * @user_data: (nullable): a helper to pass to @user_data
* @error: A #GError * @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 * If the reset function returns %FALSE, then the function returns straight away
* without processing any pending retries. * without processing any pending retries.
* *
* Since: 1.4.0 * Since: 1.5.5
**/ **/
gboolean gboolean
fu_device_retry (FuDevice *self, fu_device_retry_full (FuDevice *self,
FuDeviceRetryFunc func, FuDeviceRetryFunc func,
guint count, guint count,
gpointer user_data, guint delay,
GError **error) gpointer user_data,
GError **error)
{ {
FuDevicePrivate *priv = GET_PRIVATE (self); FuDevicePrivate *priv = GET_PRIVATE (self);
@ -269,8 +274,8 @@ fu_device_retry (FuDevice *self,
g_autoptr(GError) error_local = NULL; g_autoptr(GError) error_local = NULL;
/* delay */ /* delay */
if (i > 0 && priv->retry_delay > 0) if (i > 0 && delay > 0)
g_usleep (priv->retry_delay * 1000); g_usleep (delay * 1000);
/* run function, if success return success */ /* run function, if success return success */
if (func (self, user_data, &error_local)) if (func (self, user_data, &error_local))
@ -323,6 +328,38 @@ fu_device_retry (FuDevice *self,
return TRUE; 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: * fu_device_poll:
* @self: A #FuDevice * @self: A #FuDevice
@ -2755,6 +2792,13 @@ fu_device_cleanup (FuDevice *self, FwupdInstallFlags flags, GError **error)
return klass->cleanup (self, flags, 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: * fu_device_open:
* @self: A #FuDevice * @self: A #FuDevice
@ -2796,8 +2840,16 @@ fu_device_open (FuDevice *self, GError **error)
/* subclassed */ /* subclassed */
if (klass->open != NULL) { if (klass->open != NULL) {
if (!klass->open (self, error)) if (fu_device_has_flag (self, FWUPD_DEVICE_FLAG_RETRY_OPEN)) {
return FALSE; 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 */ /* setup */

View File

@ -374,6 +374,13 @@ gboolean fu_device_retry (FuDevice *self,
gpointer user_data, gpointer user_data,
GError **error) GError **error)
G_GNUC_WARN_UNUSED_RESULT; 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, gboolean fu_device_bind_driver (FuDevice *self,
const gchar *subsystem, const gchar *subsystem,
const gchar *driver, const gchar *driver,

View File

@ -699,3 +699,9 @@ LIBFWUPDPLUGIN_1.5.4 {
fu_common_bytes_new_offset; fu_common_bytes_new_offset;
local: *; local: *;
} LIBFWUPDPLUGIN_1.5.3; } LIBFWUPDPLUGIN_1.5.3;
LIBFWUPDPLUGIN_1.5.5 {
global:
fu_device_retry_full;
local: *;
} LIBFWUPDPLUGIN_1.5.4;

View File

@ -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_UPDATABLE);
fu_device_add_flag (FU_DEVICE (self), FWUPD_DEVICE_FLAG_CAN_VERIFY); 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_version_format (FU_DEVICE (self), FWUPD_VERSION_FORMAT_TRIPLET);
fu_device_set_protocol (FU_DEVICE (self), "com.synaptics.prometheus"); fu_device_set_protocol (FU_DEVICE (self), "com.synaptics.prometheus");
fu_device_set_remove_delay (FU_DEVICE (self), FU_DEVICE_REMOVE_DELAY_RE_ENUMERATE); fu_device_set_remove_delay (FU_DEVICE (self), FU_DEVICE_REMOVE_DELAY_RE_ENUMERATE);

View File

@ -1120,6 +1120,10 @@ fu_util_device_flag_to_string (guint64 device_flag)
/* skip */ /* skip */
return NULL; return NULL;
} }
if (device_flag == FWUPD_DEVICE_FLAG_RETRY_OPEN) {
/* skip */
return NULL;
}
if (device_flag == FWUPD_DEVICE_FLAG_UNKNOWN) { if (device_flag == FWUPD_DEVICE_FLAG_UNKNOWN) {
return NULL; return NULL;
} }