[Libguestfs] [nbdkit PATCH] delay: Avoid numeric overflow
Attempting delay-read=1000 results in no delay whatsoever: 1000
seconds, scaled to milliseconds, stored in int, then subjected to
.tv_nsec = (ms * 1000000) % 1000000000
results in .tv_nsec being set to 567587328 thanks to 32-bit overflow,
but that in turn results in instant EINVAL failure of nanosleep().
Fix it by diagnosing failure to fit in an int during config, and avoid
math that overflows during delay(). Forbid negative delays while at it.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
I was trying to test what happens when a non-fatal signal is sent to
nbdkit (by adding a no-op SIGUSR1 handler). Most of the time, sending
SIGUSR1 to the process as a whole ended up cutting the poll() in a
different thread short (and not the nanosleep()), but by directing
SIGUSR1 to the thread stuck in nanosleep, I confirmed that nanosleep()
indeed returns early with EINTR, and we end up with insufficient
delay. But getting to that point in my testing required that I first
diagnosed why my attempt at a 1000-second sleep acted like no delay at
all.
I plan on adding an nbdkit_sleep() wrapper function, which will resume
the sleep on unrelated EINTR (fixing the theoretical problem of not
sleeping long enough - theoretical since it only matters if we handle
a signal but want to continue execution, but all our current handled
signals instead request process termination) as well as the problem of
keeping nbdkit alive too long (by specifically returning an error to
let the delay filter know that it is time to exit, rather than
finishing the sleep, whether it was a signal that interrupted a
different thread or merely detection of EOF from the client).
filters/delay/delay.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/filters/delay/delay.c b/filters/delay/delay.c
index 11681c88..80eb33b0 100644
--- a/filters/delay/delay.c
+++ b/filters/delay/delay.c
@@ -32,6 +32,7 @@
#include <config.h>
+#include <limits.h>
#include <stdio.h>
#include <stdlib.h>
#include <stdint.h>
@@ -54,7 +55,7 @@ parse_delay (const char *key, const char *value)
int r;
if (len > 2 && strcmp (&value[len-2], "ms") == 0) {
- if (sscanf (value, "%d", &r) == 1)
+ if (sscanf (value, "%d", &r) == 1 && r >= 0)
return r;
else {
nbdkit_error ("cannot parse %s in milliseconds parameter: %s",
@@ -63,8 +64,14 @@ parse_delay (const char *key, const char *value)
}
}
else {
- if (sscanf (value, "%d", &r) == 1)
+ if (sscanf (value, "%d", &r) == 1 && r >= 0) {
+ if (r * 1000LL > INT_MAX) {
+ nbdkit_error ("seconds parameter %s is too large: %s",
+ key, value);
+ return -1;
+ }
return r * 1000;
+ }
else {
nbdkit_error ("cannot parse %s in seconds parameter: %s",
key, value);
@@ -79,7 +86,7 @@ delay (int ms)
if (ms > 0) {
const struct timespec ts = {
.tv_sec = ms / 1000,
- .tv_nsec = (ms * 1000000) % 1000000000
+ .tv_nsec = (ms % 1000) * 1000000,
};
nanosleep (&ts, NULL);
}
--
2.20.1