[Libguestfs] [libnbd PATCH v2 4/5] api: Add STRICT_FLAGS to set_strict_mode
Eric Blake
eblake at redhat.com
Fri Sep 11 21:49:55 UTC 2020
The next strict knob: allow the user to pass unknown flags across the
wire (this is different than passing a known flag at the wrong time).
It is interesting to note that NBD only permits 16 bits of flags, but
we have a signature that takes uint32_t; if we wanted, we could pack
libnbd-specific flags in the upper bits that the NBD protocol would
never see.
---
generator/API.ml | 46 ++++++++++++++++++++++++++++++++++++++--------
generator/API.mli | 1 +
generator/C.ml | 7 +++++--
lib/disconnect.c | 2 +-
lib/rw.c | 6 +++---
tests/errors.c | 22 ++++++++++++++++++++--
6 files changed, 68 insertions(+), 16 deletions(-)
diff --git a/generator/API.ml b/generator/API.ml
index aa970e6..4cd425b 100644
--- a/generator/API.ml
+++ b/generator/API.ml
@@ -87,6 +87,7 @@ and enum = {
}
and flags = {
flag_prefix : string;
+ guard : string option;
flags : (string * int) list
}
and permitted_state =
@@ -165,6 +166,7 @@ let all_enums = [ tls_enum; block_size_enum ]
(* Flags. See also Constants below. *)
let cmd_flags = {
flag_prefix = "CMD_FLAG";
+ guard = Some "((h->strict & LIBNBD_STRICT_FLAGS) || flags > UINT16_MAX)";
flags = [
"FUA", 1 lsl 0;
"NO_HOLE", 1 lsl 1;
@@ -175,6 +177,7 @@ let cmd_flags = {
}
let handshake_flags = {
flag_prefix = "HANDSHAKE_FLAG";
+ guard = None;
flags = [
"FIXED_NEWSTYLE", 1 lsl 0;
"NO_ZEROES", 1 lsl 1;
@@ -182,12 +185,15 @@ let handshake_flags = {
}
let strict_flags = {
flag_prefix = "STRICT";
+ guard = None;
flags = [
"COMMANDS", 1 lsl 0;
+ "FLAGS", 1 lsl 1;
]
}
let allow_transport_flags = {
flag_prefix = "ALLOW_TRANSPORT";
+ guard = None;
flags = [
"TCP", 1 lsl 0;
"UNIX", 1 lsl 1;
@@ -196,6 +202,7 @@ let allow_transport_flags = {
}
let shutdown_flags = {
flag_prefix = "SHUTDOWN";
+ guard = None;
flags = [
"IMMEDIATE", 1 lsl 1;
]
@@ -749,6 +756,22 @@ a read-only server, or attempting to use C<LIBNBD_CMD_FLAG_FUA> when
L<nbd_can_fua(3)> returned false). If clear, this flag relies on the
server to reject unexpected commands.
+=item C<LIBNBD_STRICT_FLAGS> = 2
+
+If set, this flag rejects client requests that attempt to set a
+command flag not recognized by libnbd (those outside of
+C<LIBNBD_CMD_FLAG_MASK>). If clear, this flag passes on unknown
+flags to the server. One possible reason to relax this strictness
+knob is to send C<LIBNBD_CMD_FLAG_FUA> on a read command (libnbd
+normally prevents that, but the NBD protocol allows it to succeed);
+however, it is also possible that sending an unknown flag may
+cause the server to change its reply in a manner that confuses
+libnbd, perhaps causing deadlock or ending the connection.
+
+Note that the NBD protocol only supports 16 bits of command flags,
+even though the libnbd API uses C<uint32_t>; bits outside of the
+range permitted by the protocol are always a client-side error.
+
=back
For convenience, the constant C<LIBNBD_STRICT_MASK> is available to
@@ -1568,9 +1591,10 @@ required into the server's replies, or if you want to use
C<LIBNBD_CMD_FLAG_DF>.
The C<flags> parameter must be C<0> for now (it exists for future NBD
-protocol extensions).";
+protocol extensions)."
+^ strict_call_description;
see_also = [Link "aio_pread"; Link "pread_structured";
- Link "get_block_size"];
+ Link "get_block_size"; Link "set_strict_mode"];
example = Some "examples/fetch-first-sector.c";
};
@@ -1646,9 +1670,11 @@ The C<flags> parameter may be C<0> for no flags, or may contain
C<LIBNBD_CMD_FLAG_DF> meaning that the server should not reply with
more than one fragment (if that is supported - some servers cannot do
this, see L<nbd_can_df(3)>). Libnbd does not validate that the server
-actually obeys the flag.";
+actually obeys the flag."
+^ strict_call_description;
see_also = [Link "can_df"; Link "pread";
- Link "aio_pread_structured"; Link "get_block_size"];
+ Link "aio_pread_structured"; Link "get_block_size";
+ Link "set_strict_mode"];
};
"pwrite", {
@@ -2123,10 +2149,12 @@ Or supply the optional C<completion_callback> which will be invoked
as described in L<libnbd(3)/Completion callbacks>.
Note that you must ensure C<buf> is valid until the command has
-completed. Other parameters behave as documented in L<nbd_pread(3)>.";
+completed. Other parameters behave as documented in L<nbd_pread(3)>."
+^ strict_call_description;
example = Some "examples/aio-connect-read.c";
see_also = [SectionLink "Issuing asynchronous commands";
- Link "aio_pread_structured"; Link "pread"];
+ Link "aio_pread_structured"; Link "pread";
+ Link "set_strict_mode"];
};
"aio_pread_structured", {
@@ -2145,9 +2173,11 @@ To check if the command completed, call L<nbd_aio_command_completed(3)>.
Or supply the optional C<completion_callback> which will be invoked
as described in L<libnbd(3)/Completion callbacks>.
-Other parameters behave as documented in L<nbd_pread_structured(3)>.";
+Other parameters behave as documented in L<nbd_pread_structured(3)>."
+^ strict_call_description;
see_also = [SectionLink "Issuing asynchronous commands";
- Link "aio_pread"; Link "pread_structured"];
+ Link "aio_pread"; Link "pread_structured";
+ Link "set_strict_mode"];
};
"aio_pwrite", {
diff --git a/generator/API.mli b/generator/API.mli
index db978ca..41e09f5 100644
--- a/generator/API.mli
+++ b/generator/API.mli
@@ -100,6 +100,7 @@ and enum = {
}
and flags = {
flag_prefix : string; (** prefix of each flag name *)
+ guard : string option; (** additional gating for checking valid flags *)
flags : (string * int) list (** flag names and their values in C *)
}
and permitted_state =
diff --git a/generator/C.ml b/generator/C.ml
index 86d9c5c..5f68b14 100644
--- a/generator/C.ml
+++ b/generator/C.ml
@@ -494,7 +494,7 @@ let generate_lib_api_c () =
);
(* Check parameters are valid. *)
- let print_flags_check n { flag_prefix; flags } subset =
+ let print_flags_check n { flag_prefix; flags; guard } subset =
let value = match errcode with
| Some value -> value
| None -> assert false in
@@ -508,7 +508,10 @@ let generate_lib_api_c () =
) flags;
sprintf "0x%x" !v
| None -> "LIBNBD_" ^ flag_prefix ^ "_MASK" in
- pr " if (unlikely ((%s & ~%s) != 0)) {\n" n mask;
+ let guard = match guard with
+ | Some value -> " && " ^ value
+ | None -> "" in
+ pr " if (unlikely ((%s & ~%s) != 0)%s) {\n" n mask guard;
pr " set_error (EINVAL, \"%%s: invalid value for flag: 0x%%x\",\n";
pr " \"%s\", %s);\n" n n;
pr " ret = %s;\n" value;
diff --git a/lib/disconnect.c b/lib/disconnect.c
index 9de1e34..f99fbd0 100644
--- a/lib/disconnect.c
+++ b/lib/disconnect.c
@@ -64,7 +64,7 @@ nbd_unlocked_aio_disconnect (struct nbd_handle *h, uint32_t flags)
{
int64_t id;
- id = nbd_internal_command_common (h, 0, NBD_CMD_DISC, 0, 0, NULL, NULL);
+ id = nbd_internal_command_common (h, flags, NBD_CMD_DISC, 0, 0, NULL, NULL);
if (id == -1)
return -1;
h->disconnect_request = true;
diff --git a/lib/rw.c b/lib/rw.c
index f49fe25..e3e80ad 100644
--- a/lib/rw.c
+++ b/lib/rw.c
@@ -281,7 +281,7 @@ nbd_unlocked_aio_pread (struct nbd_handle *h, void *buf,
struct command_cb cb = { .completion = *completion };
SET_CALLBACK_TO_NULL (*completion);
- return nbd_internal_command_common (h, 0, NBD_CMD_READ, offset, count,
+ return nbd_internal_command_common (h, flags, NBD_CMD_READ, offset, count,
buf, &cb);
}
@@ -350,7 +350,7 @@ nbd_unlocked_aio_flush (struct nbd_handle *h,
}
SET_CALLBACK_TO_NULL (*completion);
- return nbd_internal_command_common (h, 0, NBD_CMD_FLUSH, 0, 0,
+ return nbd_internal_command_common (h, flags, NBD_CMD_FLUSH, 0, 0,
NULL, &cb);
}
@@ -409,7 +409,7 @@ nbd_unlocked_aio_cache (struct nbd_handle *h,
}
SET_CALLBACK_TO_NULL (*completion);
- return nbd_internal_command_common (h, 0, NBD_CMD_CACHE, offset, count,
+ return nbd_internal_command_common (h, flags, NBD_CMD_CACHE, offset, count,
NULL, &cb);
}
diff --git a/tests/errors.c b/tests/errors.c
index 0c4151a..0cfcac5 100644
--- a/tests/errors.c
+++ b/tests/errors.c
@@ -84,7 +84,7 @@ main (int argc, char *argv[])
* which delays responding to writes until a witness file no longer
* exists.
*/
- const char *cmd[] = { "nbdkit", "-s", "--exit-with-parent", "sh",
+ const char *cmd[] = { "nbdkit", "-s", "-v", "--exit-with-parent", "sh",
script, NULL };
uint32_t strict;
@@ -244,7 +244,25 @@ main (int argc, char *argv[])
}
check (EINVAL, "nbd_pread: ");
- /* Use unknown command flags */
+ /* Use unknown command flags, client-side */
+ strict = nbd_get_strict_mode (nbd) | LIBNBD_STRICT_FLAGS;
+ if (nbd_set_strict_mode (nbd, strict) == -1) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
+ if (nbd_pread (nbd, buf, 1, 0, LIBNBD_CMD_FLAG_MASK + 1) != -1) {
+ fprintf (stderr, "%s: test failed: "
+ "nbd_pread did not fail with bogus flags\n",
+ argv[0]);
+ exit (EXIT_FAILURE);
+ }
+ check (EINVAL, "nbd_pread: ");
+ /* Use unknown command flags, server-side */
+ strict &= ~(LIBNBD_STRICT_FLAGS | LIBNBD_STRICT_COMMANDS);
+ if (nbd_set_strict_mode (nbd, strict) == -1) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
if (nbd_pread (nbd, buf, 1, 0, LIBNBD_CMD_FLAG_MASK + 1) != -1) {
fprintf (stderr, "%s: test failed: "
"nbd_pread did not fail with bogus flags\n",
--
2.28.0
More information about the Libguestfs
mailing list