[Libguestfs] [nbdkit PATCH 1/5] retry: Don't call into closed plugin
Eric Blake
eblake at redhat.com
Mon Oct 7 14:50:12 UTC 2019
If reopen fails, calling into the plugin would end up passing NULL as
the handle. For the sh plugin, this merely results in a truncated
command line, but for other plugins it could result in a crash. The
fix is to have the retry filter track whether reopen failed, and
short-circuit any new transaction to begin with a retry when the
previous transaction ended in that state.
Add asserts to backend to ensure that the plugin's handle is non-NULL
at all points of use, now that the retry filter keeps that contract.
Fixes: f0f0ec49
Signed-off-by: Eric Blake <eblake at redhat.com>
---
server/backend.c | 18 ++++++++++++++++++
filters/retry/retry.c | 26 +++++++++++++++++---------
2 files changed, 35 insertions(+), 9 deletions(-)
diff --git a/server/backend.c b/server/backend.c
index ace61c29..641af5ff 100644
--- a/server/backend.c
+++ b/server/backend.c
@@ -265,6 +265,7 @@ backend_get_size (struct backend *b, struct connection *conn)
debug ("%s: get_size", b->name);
+ assert (h->handle);
if (h->exportsize == -1)
h->exportsize = b->get_size (b, conn, h->handle);
return h->exportsize;
@@ -277,6 +278,7 @@ backend_can_write (struct backend *b, struct connection *conn)
debug ("%s: can_write", b->name);
+ assert (h->handle);
if (h->can_write == -1)
h->can_write = b->can_write (b, conn, h->handle);
return h->can_write;
@@ -289,6 +291,7 @@ backend_can_flush (struct backend *b, struct connection *conn)
debug ("%s: can_flush", b->name);
+ assert (h->handle);
if (h->can_flush == -1)
h->can_flush = b->can_flush (b, conn, h->handle);
return h->can_flush;
@@ -301,6 +304,7 @@ backend_is_rotational (struct backend *b, struct connection *conn)
debug ("%s: is_rotational", b->name);
+ assert (h->handle);
if (h->is_rotational == -1)
h->is_rotational = b->is_rotational (b, conn, h->handle);
return h->is_rotational;
@@ -314,6 +318,7 @@ backend_can_trim (struct backend *b, struct connection *conn)
debug ("%s: can_trim", b->name);
+ assert (h->handle);
if (h->can_trim == -1) {
r = backend_can_write (b, conn);
if (r != 1) {
@@ -333,6 +338,7 @@ backend_can_zero (struct backend *b, struct connection *conn)
debug ("%s: can_zero", b->name);
+ assert (h->handle);
if (h->can_zero == -1) {
r = backend_can_write (b, conn);
if (r != 1) {
@@ -352,6 +358,7 @@ backend_can_fast_zero (struct backend *b, struct connection *conn)
debug ("%s: can_fast_zero", b->name);
+ assert (h->handle);
if (h->can_fast_zero == -1) {
r = backend_can_zero (b, conn);
if (r < NBDKIT_ZERO_EMULATE) {
@@ -370,6 +377,7 @@ backend_can_extents (struct backend *b, struct connection *conn)
debug ("%s: can_extents", b->name);
+ assert (h->handle);
if (h->can_extents == -1)
h->can_extents = b->can_extents (b, conn, h->handle);
return h->can_extents;
@@ -383,6 +391,7 @@ backend_can_fua (struct backend *b, struct connection *conn)
debug ("%s: can_fua", b->name);
+ assert (h->handle);
if (h->can_fua == -1) {
r = backend_can_write (b, conn);
if (r != 1) {
@@ -399,6 +408,7 @@ backend_can_multi_conn (struct backend *b, struct connection *conn)
{
struct b_conn_handle *h = &conn->handles[b->i];
+ assert (h->handle);
debug ("%s: can_multi_conn", b->name);
if (h->can_multi_conn == -1)
@@ -413,6 +423,7 @@ backend_can_cache (struct backend *b, struct connection *conn)
debug ("%s: can_cache", b->name);
+ assert (h->handle);
if (h->can_cache == -1)
h->can_cache = b->can_cache (b, conn, h->handle);
return h->can_cache;
@@ -426,6 +437,7 @@ backend_pread (struct backend *b, struct connection *conn,
struct b_conn_handle *h = &conn->handles[b->i];
int r;
+ assert (h->handle);
assert (backend_valid_range (b, conn, offset, count));
assert (flags == 0);
debug ("%s: pread count=%" PRIu32 " offset=%" PRIu64,
@@ -446,6 +458,7 @@ backend_pwrite (struct backend *b, struct connection *conn,
bool fua = !!(flags & NBDKIT_FLAG_FUA);
int r;
+ assert (h->handle);
assert (h->can_write == 1);
assert (backend_valid_range (b, conn, offset, count));
assert (!(flags & ~NBDKIT_FLAG_FUA));
@@ -467,6 +480,7 @@ backend_flush (struct backend *b, struct connection *conn,
struct b_conn_handle *h = &conn->handles[b->i];
int r;
+ assert (h->handle);
assert (h->can_flush == 1);
assert (flags == 0);
debug ("%s: flush", b->name);
@@ -486,6 +500,7 @@ backend_trim (struct backend *b, struct connection *conn,
bool fua = !!(flags & NBDKIT_FLAG_FUA);
int r;
+ assert (h->handle);
assert (h->can_write == 1);
assert (h->can_trim == 1);
assert (backend_valid_range (b, conn, offset, count));
@@ -511,6 +526,7 @@ backend_zero (struct backend *b, struct connection *conn,
bool fast = !!(flags & NBDKIT_FLAG_FAST_ZERO);
int r;
+ assert (h->handle);
assert (h->can_write == 1);
assert (h->can_zero > NBDKIT_ZERO_NONE);
assert (backend_valid_range (b, conn, offset, count));
@@ -541,6 +557,7 @@ backend_extents (struct backend *b, struct connection *conn,
struct b_conn_handle *h = &conn->handles[b->i];
int r;
+ assert (h->handle);
assert (h->can_extents >= 0);
assert (backend_valid_range (b, conn, offset, count));
assert (!(flags & ~NBDKIT_FLAG_REQ_ONE));
@@ -570,6 +587,7 @@ backend_cache (struct backend *b, struct connection *conn,
struct b_conn_handle *h = &conn->handles[b->i];
int r;
+ assert (h->handle);
assert (h->can_cache > NBDKIT_CACHE_NONE);
assert (backend_valid_range (b, conn, offset, count));
assert (flags == 0);
diff --git a/filters/retry/retry.c b/filters/retry/retry.c
index 0f249936..11e5313b 100644
--- a/filters/retry/retry.c
+++ b/filters/retry/retry.c
@@ -107,6 +107,7 @@ retry_config (nbdkit_next_config *next, void *nxdata,
struct retry_handle {
int readonly; /* Save original readonly setting. */
unsigned reopens;
+ bool open;
};
static void *
@@ -125,6 +126,7 @@ retry_open (nbdkit_next_open *next, void *nxdata, int readonly)
h->readonly = readonly;
h->reopens = 0;
+ h->open = true;
return h;
}
@@ -183,7 +185,8 @@ do_retry (struct retry_handle *h,
/* We could do this but it would overwrite the more important
* errno from the underlying data call.
*/
- /* *err = errno; */
+ if (*err == 0)
+ *err = errno;
return false;
}
@@ -198,8 +201,11 @@ do_retry (struct retry_handle *h,
/* If the reopen fails we treat it the same way as a command
* failing.
*/
+ h->open = false;
+ *err = ESHUTDOWN;
goto again;
}
+ h->open = true;
/* Retry the data command. */
return true;
@@ -215,7 +221,7 @@ retry_pread (struct nbdkit_next_ops *next_ops, void *nxdata,
int r;
again:
- if (! valid_range (next_ops, nxdata, count, offset, false, err))
+ if (! (h->open && valid_range (next_ops, nxdata, count, offset, false, err)))
r = -1;
else
r = next_ops->pread (nxdata, buf, count, offset, flags, err);
@@ -240,7 +246,7 @@ retry_pwrite (struct nbdkit_next_ops *next_ops, void *nxdata,
*err = EROFS;
return -1;
}
- if (! valid_range (next_ops, nxdata, count, offset, true, err))
+ if (! (h->open && valid_range (next_ops, nxdata, count, offset, true, err)))
r = -1;
else if (next_ops->can_write (nxdata) != 1) {
*err = EROFS;
@@ -274,7 +280,7 @@ retry_trim (struct nbdkit_next_ops *next_ops, void *nxdata,
*err = EROFS;
return -1;
}
- if (! valid_range (next_ops, nxdata, count, offset, true, err))
+ if (! (h->open && valid_range (next_ops, nxdata, count, offset, true, err)))
r = -1;
else if (next_ops->can_trim (nxdata) != 1) {
*err = EROFS;
@@ -303,7 +309,9 @@ retry_flush (struct nbdkit_next_ops *next_ops, void *nxdata,
int r;
again:
- if (next_ops->can_flush (nxdata) != 1) {
+ if (! h->open)
+ r = -1;
+ else if (next_ops->can_flush (nxdata) != 1) {
*err = EIO;
r = -1;
}
@@ -331,11 +339,11 @@ retry_zero (struct nbdkit_next_ops *next_ops, void *nxdata,
return -1;
}
if (flags & NBDKIT_FLAG_FAST_ZERO &&
- next_ops->can_fast_zero (nxdata) != 1) {
+ (! h->open || next_ops->can_fast_zero (nxdata) != 1)) {
*err = EOPNOTSUPP;
return -1;
}
- if (! valid_range (next_ops, nxdata, count, offset, true, err))
+ if (! (h->open && valid_range (next_ops, nxdata, count, offset, true, err)))
r = -1;
else if (next_ops->can_zero (nxdata) <= NBDKIT_ZERO_NONE) {
*err = EROFS;
@@ -367,7 +375,7 @@ retry_extents (struct nbdkit_next_ops *next_ops, void *nxdata,
size_t i;
again:
- if (! valid_range (next_ops, nxdata, count, offset, false, err))
+ if (! (h->open && valid_range (next_ops, nxdata, count, offset, false, err)))
r = -1;
else if (next_ops->can_extents (nxdata) != 1) {
*err = EIO;
@@ -412,7 +420,7 @@ retry_cache (struct nbdkit_next_ops *next_ops, void *nxdata,
int r;
again:
- if (! valid_range (next_ops, nxdata, count, offset, false, err))
+ if (! h->open && (valid_range (next_ops, nxdata, count, offset, false, err)))
r = -1;
else if (next_ops->can_cache (nxdata) <= NBDKIT_CACHE_NONE) {
*err = EIO;
--
2.21.0
More information about the Libguestfs
mailing list