diff --git a/src/libsystemd/sd-json/sd-json.c b/src/libsystemd/sd-json/sd-json.c index b8633548c8..99d941e1ef 100644 --- a/src/libsystemd/sd-json/sd-json.c +++ b/src/libsystemd/sd-json/sd-json.c @@ -5104,7 +5104,7 @@ _public_ int sd_json_dispatch_full( } m = 0; - for (const sd_json_dispatch_field *p = table; p->name; p++) + for (const sd_json_dispatch_field *p = table; p && p->name; p++) m++; found = newa0(bool, m); @@ -5117,12 +5117,12 @@ _public_ int sd_json_dispatch_full( assert_se(key = sd_json_variant_by_index(v, i)); assert_se(value = sd_json_variant_by_index(v, i+1)); - for (p = table; p->name; p++) + for (p = table; p && p->name; p++) if (p->name == POINTER_MAX || streq_ptr(sd_json_variant_string(key), p->name)) break; - if (p->name) { /* Found a matching entry! 🙂 */ + if (p && p->name) { /* Found a matching entry! 🙂 */ sd_json_dispatch_flags_t merged_flags; merged_flags = flags | p->flags; @@ -5223,7 +5223,7 @@ _public_ int sd_json_dispatch_full( } } - for (const sd_json_dispatch_field *p = table; p->name; p++) { + for (const sd_json_dispatch_field *p = table; p && p->name; p++) { sd_json_dispatch_flags_t merged_flags = p->flags | flags; if ((merged_flags & SD_JSON_MANDATORY) && !found[p-table]) { diff --git a/src/libsystemd/sd-varlink/sd-varlink.c b/src/libsystemd/sd-varlink/sd-varlink.c index 9e8f2a66f8..8658c81f67 100644 --- a/src/libsystemd/sd-varlink/sd-varlink.c +++ b/src/libsystemd/sd-varlink/sd-varlink.c @@ -2678,10 +2678,21 @@ _public_ int sd_varlink_error_invalid_parameter_name(sd_varlink *v, const char * } _public_ int sd_varlink_error_errno(sd_varlink *v, int error) { + + /* This generates a system error return that includes the Linux error number, and error name. The + * error number is kinda Linux specific (and to some degree the error name too), hence let's indicate + * the origin of the system error. This way interpretation of the error should not leave questions + * open, even to foreign systems. */ + + error = abs(error); + const char *name = errno_to_name(error); + return sd_varlink_errorbo( v, SD_VARLINK_ERROR_SYSTEM, - SD_JSON_BUILD_PAIR("errno", SD_JSON_BUILD_INTEGER(abs(error)))); + SD_JSON_BUILD_PAIR_STRING("origin", "linux"), + SD_JSON_BUILD_PAIR_INTEGER("errno", error), + JSON_BUILD_PAIR_STRING_NON_EMPTY("errnoName", name)); } _public_ int sd_varlink_notify(sd_varlink *v, sd_json_variant *parameters) { @@ -2755,7 +2766,6 @@ _public_ int sd_varlink_dispatch(sd_varlink *v, sd_json_variant *parameters, con int r; assert_return(v, -EINVAL); - assert_return(table, -EINVAL); /* A wrapper around json_dispatch_full() that returns a nice InvalidParameter error if we hit a problem with some field. */ @@ -4194,6 +4204,8 @@ _public_ int sd_varlink_error_to_errno(const char *error, sd_json_variant *param { SD_VARLINK_ERROR_EXPECTED_MORE, -EBADE }, }; + int r; + if (!error) return 0; @@ -4201,20 +4213,46 @@ _public_ int sd_varlink_error_to_errno(const char *error, sd_json_variant *param if (streq(error, t->error)) return t->value; - if (streq(error, SD_VARLINK_ERROR_SYSTEM) && parameters) { - sd_json_variant *e; + /* This following tries to reverse the operation sd_varlink_error_errno() applies to turn errnos into + * varlink errors */ + if (!streq(error, SD_VARLINK_ERROR_SYSTEM)) + return -EBADR; - e = sd_json_variant_by_key(parameters, "errno"); - if (sd_json_variant_is_integer(e)) { - int64_t i; + if (!parameters) + return -EBADR; - i = sd_json_variant_integer(e); - if (i > 0 && i < ERRNO_MAX) - return -i; - } + /* If an origin is set, check if it's Linux, otherwise don't translate */ + sd_json_variant *e = sd_json_variant_by_key(parameters, "origin"); + if (e && (!sd_json_variant_is_string(e) || + !streq(sd_json_variant_string(e), "linux"))) + return -EBADR; + + /* If a name is specified, go by name */ + e = sd_json_variant_by_key(parameters, "errnoName"); + if (e) { + if (!sd_json_variant_is_string(e)) + return -EBADR; + + r = errno_from_name(sd_json_variant_string(e)); + if (r < 0) + return -EBADR; + + assert(r > 0); + return -r; } - return -EBADR; /* Catch-all */ + /* Finally, use the provided error number, if there is one */ + e = sd_json_variant_by_key(parameters, "errno"); + if (!e) + return -EBADR; + if (!sd_json_variant_is_integer(e)) + return -EBADR; + + int64_t i = sd_json_variant_integer(e); + if (i <= 0 || i > ERRNO_MAX) + return -EBADR; + + return (int) -i; } _public_ int sd_varlink_error_is_invalid_parameter(const char *error, sd_json_variant *parameter, const char *name) { diff --git a/src/libsystemd/sd-varlink/varlink-io.systemd.c b/src/libsystemd/sd-varlink/varlink-io.systemd.c index e2511265a3..30c9495d40 100644 --- a/src/libsystemd/sd-varlink/varlink-io.systemd.c +++ b/src/libsystemd/sd-varlink/varlink-io.systemd.c @@ -10,12 +10,21 @@ static SD_VARLINK_DEFINE_ERROR(Protocol); /* This one we invented, and use for generically propagating system errors (errno) to clients */ static SD_VARLINK_DEFINE_ERROR( System, - SD_VARLINK_DEFINE_FIELD(errno, SD_VARLINK_INT, 0)); + SD_VARLINK_FIELD_COMMENT("The origin of this system error, typically 'linux' to indicate Linux error numbers."), + SD_VARLINK_DEFINE_FIELD(origin, SD_VARLINK_STRING, SD_VARLINK_NULLABLE), + SD_VARLINK_FIELD_COMMENT("The Linux error name, i.e. ENOENT, EHWPOISON or similar."), + SD_VARLINK_DEFINE_FIELD(errnoName, SD_VARLINK_STRING, SD_VARLINK_NULLABLE), + SD_VARLINK_FIELD_COMMENT("The numeric Linux error number. Typically the name is preferable, if specified."), + SD_VARLINK_DEFINE_FIELD(errno, SD_VARLINK_STRING, SD_VARLINK_NULLABLE)); SD_VARLINK_DEFINE_INTERFACE( io_systemd, "io.systemd", + SD_VARLINK_SYMBOL_COMMENT("Local error if a Varlink connection is disconnected (this never crosses the wire and is synthesized locally only)."), &vl_error_Disconnected, + SD_VARLINK_SYMBOL_COMMENT("A method call time-out has been reached (also synthesized locally, does not cross wire)"), &vl_error_TimedOut, + SD_VARLINK_SYMBOL_COMMENT("Some form of protocol error (also synthesized locally, does not cross wire)"), &vl_error_Protocol, + SD_VARLINK_SYMBOL_COMMENT("A generic Linux system error (\"errno\"s)."), &vl_error_System); diff --git a/src/test/test-varlink.c b/src/test/test-varlink.c index fe54ec08a5..0708ec1446 100644 --- a/src/test/test-varlink.c +++ b/src/test/test-varlink.c @@ -153,6 +153,16 @@ static int method_passfd(sd_varlink *link, sd_json_variant *parameters, sd_varli return sd_varlink_reply(link, ret); } +static int method_fail_with_errno(sd_varlink *link, sd_json_variant *parameters, sd_varlink_method_flags_t flags, void *userdata) { + int r; + + r = sd_varlink_dispatch(link, parameters, NULL, NULL); + if (r != 0) + return r; + + return sd_varlink_error_errno(link, EHWPOISON); +} + static int method_done(sd_varlink *link, sd_json_variant *parameters, sd_varlink_method_flags_t flags, void *userdata) { if (++n_done == 2) @@ -315,6 +325,8 @@ static void *thread(void *arg) { ASSERT_STREQ(sd_json_variant_string(sd_json_variant_by_key(o, "method")), "io.test.IDontExist"); ASSERT_STREQ(e, SD_VARLINK_ERROR_METHOD_NOT_FOUND); + ASSERT_OK(sd_varlink_call(c, "io.test.FailWithErrno", NULL, &o, &e)); + ASSERT_ERROR(sd_varlink_error_to_errno(e, o), EHWPOISON); flood_test(arg); assert_se(sd_varlink_send(c, "io.test.Done", NULL) >= 0); @@ -368,6 +380,7 @@ TEST(chat) { assert_se(sd_varlink_server_bind_method(s, "io.test.PassFD", method_passfd) >= 0); assert_se(sd_varlink_server_bind_method(s, "io.test.DoSomething", method_something) >= 0); assert_se(sd_varlink_server_bind_method(s, "io.test.DoSomethingMore", method_something_more) >= 0); + assert_se(sd_varlink_server_bind_method(s, "io.test.FailWithErrno", method_fail_with_errno) >= 0); assert_se(sd_varlink_server_bind_method(s, "io.test.Done", method_done) >= 0); assert_se(sd_varlink_server_bind_connect(s, on_connect) >= 0); assert_se(sd_varlink_server_listen_address(s, sp, 0600) >= 0);