From eb84f74669564c999e838ade47080a1fee2e9e3c Mon Sep 17 00:00:00 2001 From: Ludvig Michaelsson Date: Thu, 21 Dec 2023 17:33:30 +0100 Subject: [PATCH 1/2] osx: add schedule_io_loop() --- src/hid_osx.c | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/src/hid_osx.c b/src/hid_osx.c index 9309762f..f2a0c54f 100644 --- a/src/hid_osx.c +++ b/src/hid_osx.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2019-2022 Yubico AB. All rights reserved. + * Copyright (c) 2019-2023 Yubico AB. All rights reserved. * Use of this source code is governed by a BSD-style * license that can be found in the LICENSE file. * SPDX-License-Identifier: BSD-2-Clause @@ -523,6 +523,21 @@ fido_hid_set_sigmask(void *handle, const fido_sigset_t *sigmask) return (FIDO_ERR_INTERNAL); } +static void +schedule_io_loop(struct hid_osx *ctx, int ms) +{ + IOHIDDeviceScheduleWithRunLoop(ctx->ref, CFRunLoopGetCurrent(), + ctx->loop_id); + + if (ms == -1) + ms = 5000; /* wait 5 seconds by default */ + + CFRunLoopRunInMode(ctx->loop_id, (double)ms/1000.0, true); + + IOHIDDeviceUnscheduleFromRunLoop(ctx->ref, CFRunLoopGetCurrent(), + ctx->loop_id); +} + int fido_hid_read(void *handle, unsigned char *buf, size_t len, int ms) { @@ -537,16 +552,7 @@ fido_hid_read(void *handle, unsigned char *buf, size_t len, int ms) return (-1); } - IOHIDDeviceScheduleWithRunLoop(ctx->ref, CFRunLoopGetCurrent(), - ctx->loop_id); - - if (ms == -1) - ms = 5000; /* wait 5 seconds by default */ - - CFRunLoopRunInMode(ctx->loop_id, (double)ms/1000.0, true); - - IOHIDDeviceUnscheduleFromRunLoop(ctx->ref, CFRunLoopGetCurrent(), - ctx->loop_id); + schedule_io_loop(ctx, ms); if ((r = read(ctx->report_pipe[0], buf, len)) == -1) { fido_log_error(errno, "%s: read", __func__); From a9cb1fc7396e0b0e108337cf8b047473e62ceed2 Mon Sep 17 00:00:00 2001 From: Ludvig Michaelsson Date: Thu, 21 Dec 2023 17:09:36 +0100 Subject: [PATCH 2/2] osx: handle report_callback() firing multiple times One invocation of CFRunLoopRunInMode() may fire report_callback() multiple times. In such a case, the next call to fido_hid_read() may block for the full duration of the timeout. We can handle this by querying the (non-blocking) pipe for any readily available data on entering fido_hid_read() and fall back to executing the run loop if it was empty. Debugged with @elibon99 and @martelletto. --- src/hid_osx.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/hid_osx.c b/src/hid_osx.c index f2a0c54f..41e9d171 100644 --- a/src/hid_osx.c +++ b/src/hid_osx.c @@ -552,11 +552,19 @@ fido_hid_read(void *handle, unsigned char *buf, size_t len, int ms) return (-1); } - schedule_io_loop(ctx, ms); - + /* check for pending frame */ if ((r = read(ctx->report_pipe[0], buf, len)) == -1) { - fido_log_error(errno, "%s: read", __func__); - return (-1); + if (errno != EAGAIN && errno != EWOULDBLOCK) { + fido_log_error(errno, "%s: read", __func__); + return (-1); + } + + schedule_io_loop(ctx, ms); + + if ((r = read(ctx->report_pipe[0], buf, len)) == -1) { + fido_log_error(errno, "%s: read", __func__); + return (-1); + } } if (r < 0 || (size_t)r != len) {