From: Yaping Li <202858510+YapingLi04@users.noreply.github.com>
Date: Tue, 28 Apr 2026 22:45:05 +0000 (-0700)
Subject: sd-varlink: skip IDL validation on synthetic empty terminator
X-Git-Tag: v261-rc2~25^2~5
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=4d2bc332f338db8bb85947ab1259f70e4e3f8c82;p=thirdparty%2Fsystemd.git

sd-varlink: skip IDL validation on synthetic empty terminator

When a streaming method's callback returns 0 with no replies enqueued and
sd_varlink_set_sentinel() was passed NULL (POINTER_MAX), the framework
emits an empty parameters reply via sd_varlink_reply(v, NULL) as a clean
stream terminator. That call validated the empty parameters against the
method's output IDL — which always failed when the IDL declared any
mandatory output field, generating a spurious 'didn't pass validation'
warning even though the empty terminator is correct by construction.

Move the body of sd_varlink_reply() into a static varlink_reply_internal()
that takes a skip_validation flag. The public sd_varlink_reply() keeps
its existing behavior (validation enabled). Add a static
varlink_reply_terminator() helper that wraps
varlink_reply_internal(v, NULL, /* skip_validation= */ true), and route
varlink_dispatch_sentinel() through it for the synthetic empty terminator,
so both the synchronous dispatch path (varlink_dispatch_method()) and the
fiber path (varlink_fiber_entry()) skip validation in one place.

Without this, callers who want 'streaming method, possibly empty result'
semantics had to either fire a logically wrong 'NotFound'-style error
sentinel on empty (sysext.List, BootControl.ListBootEntries pattern) or
mark every output field NULLABLE just to placate the validator (the
io.systemd.Manager.EnqueueMarkedJobs approach), misrepresenting the
contract — exactly the tradeoff the FIXME in pcrlock.c flagged. With
this change, the empty terminator is invisible to the IDL validator and
the FIXME is no longer needed.

Co-developed-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---

diff --git a/src/libsystemd/sd-varlink/sd-varlink.c b/src/libsystemd/sd-varlink/sd-varlink.c
index af9dd30973b..1b0efa39920 100644
--- a/src/libsystemd/sd-varlink/sd-varlink.c
+++ b/src/libsystemd/sd-varlink/sd-varlink.c
@@ -73,6 +73,7 @@ static const char* const varlink_state_table[_VARLINK_STATE_MAX] = {
 DEFINE_PRIVATE_STRING_TABLE_LOOKUP_TO_STRING(varlink_state, VarlinkState);
 
 static void varlink_server_test_exit_on_idle(sd_varlink_server *s);
+static int varlink_reply_terminator(sd_varlink *v);
 
 static void varlink_set_state(sd_varlink *v, VarlinkState state) {
         assert(v);
@@ -989,7 +990,9 @@ static int varlink_dispatch_sentinel(sd_varlink *v) {
         /* Propagate the sentinel to the client if one was configured and no replies were enqueued by
          * the callback. */
         if (sentinel == POINTER_MAX)
-                r = sd_varlink_reply(v, NULL);
+                /* Synthetic empty terminator. Skip IDL validation since the empty parameters wouldn't
+                 * satisfy any mandatory output fields the method declares. */
+                r = varlink_reply_terminator(v);
         else {
                 r = sd_varlink_error(v, sentinel, NULL);
                 /* sd_varlink_error() deliberately returns a negative
@@ -2298,10 +2301,10 @@ _public_ int sd_varlink_collectb(
         return sd_varlink_collect_full(v, method, parameters, ret_parameters, ret_error_id, NULL);
 }
 
-_public_ int sd_varlink_reply(sd_varlink *v, sd_json_variant *parameters) {
+static int varlink_reply_internal(sd_varlink *v, sd_json_variant *parameters, bool skip_validation) {
         int r;
 
-        assert_return(v, -EINVAL);
+        assert(v);
 
         if (v->state == VARLINK_DISCONNECTED)
                 return varlink_log_errno(v, SYNTHETIC_ERRNO(ENOTCONN), "Not connected.");
@@ -2313,8 +2316,9 @@ _public_ int sd_varlink_reply(sd_varlink *v, sd_json_variant *parameters) {
 
         bool more = IN_SET(v->state, VARLINK_PROCESSING_METHOD_MORE, VARLINK_PENDING_METHOD_MORE);
 
-        /* Validate parameters BEFORE sanitization */
-        if (v->current_method) {
+        /* Validate parameters BEFORE sanitization. Validation should be skipped for the synthetic
+         * empty terminator. */
+        if (!skip_validation && v->current_method) {
                 const char *bad_field = NULL;
 
                 r = varlink_idl_validate_method_reply(v->current_method, parameters, more && v->sentinel ? SD_VARLINK_REPLY_CONTINUES : 0, &bad_field);
@@ -2373,6 +2377,16 @@ _public_ int sd_varlink_reply(sd_varlink *v, sd_json_variant *parameters) {
         return 1;
 }
 
+static int varlink_reply_terminator(sd_varlink *v) {
+        return varlink_reply_internal(v, /* parameters= */ NULL, /* skip_validation= */ true);
+}
+
+_public_ int sd_varlink_reply(sd_varlink *v, sd_json_variant *parameters) {
+        assert_return(v, -EINVAL);
+
+        return varlink_reply_internal(v, parameters, /* skip_validation= */ false);
+}
+
 _public_ int sd_varlink_replyb(sd_varlink *v, ...) {
         _cleanup_(sd_json_variant_unrefp) sd_json_variant *parameters = NULL;
         va_list ap;
diff --git a/src/pcrlock/pcrlock.c b/src/pcrlock/pcrlock.c
index a0d415209ce..247c36cca15 100644
--- a/src/pcrlock/pcrlock.c
+++ b/src/pcrlock/pcrlock.c
@@ -5406,7 +5406,6 @@ static int vl_method_read_event_log(sd_varlink *link, sd_json_variant *parameter
         if (r < 0)
                 return r;
 
-        // FIXME: We can't use a NULL sentinel here because the output fields in the IDL are non-nullable.
         r = sd_varlink_set_sentinel(link, NULL);
         if (r < 0)
                 return r;