[PATCH v1] tools/ocaml: xenbus - Fix handling of requests with len = 0 for socket connections

Andrii Sultanov posted 1 patch 2 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/1783598431.8631fc262581453bbf619ec5b2062170.19f46c03b15000edb5@vates.tech
tools/ocaml/libs/xb/xb.ml | 25 +++++++++++++++++++------
1 file changed, 19 insertions(+), 6 deletions(-)
[PATCH v1] tools/ocaml: xenbus - Fix handling of requests with len = 0 for socket connections
Posted by Andrii Sultanov 2 weeks ago
Following the report at https://lore.kernel.org/xen-devel/CAFLBxZaeTMcF4tcV45MJdCVx4A6qbzQdjKei_Quh_iLrtARVFA@mail.gmail.com/

Without this fix, the backend would be stuck waiting on the zero-sized
body until the next request comes in.

Instead return the request immediately after reading the header, there's
no need to wait for another call to .has_more_input and .input

Reported-by: George Dunlap <dunlapg@umich.edu>
Signed-off-by: Andrii Sultanov <andriy.sultanov@vates.tech>
---
 tools/ocaml/libs/xb/xb.ml | 25 +++++++++++++++++++------
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/tools/ocaml/libs/xb/xb.ml b/tools/ocaml/libs/xb/xb.ml
index 3e3ef2b29d..12ddb05c6d 100644
--- a/tools/ocaml/libs/xb/xb.ml
+++ b/tools/ocaml/libs/xb/xb.ml
@@ -249,6 +249,11 @@ let can_input con = Queue.can_push con.pkt_out CommandReply
 
 (* NB: can throw Reconnect *)
 let input con =
+  let reset_and_return partial_pkt =
+    let pkt = Packet.of_partialpkt partial_pkt in
+    con.partial_in <- init_partial_in ();
+    Some pkt
+  in
   if not (can_input con) then None
   else
     let to_read = to_read con in
@@ -264,17 +269,25 @@ let input con =
         if sz > 0 then
           Partial.append partial_pkt (Bytes.to_string b) sz;
         if Partial.to_complete partial_pkt = 0 then (
-          let pkt = Packet.of_partialpkt partial_pkt in
-          con.partial_in <- init_partial_in ();
-          Some pkt
+          reset_and_return partial_pkt
         ) else None
       | NoHdr (i, buf)      ->
         (* we complete the partial header *)
         if sz > 0 then
           Bytes.blit b 0 buf (Partial.header_size () - i) sz;
-        con.partial_in <- if sz = i then
-            HaveHdr (Partial.of_string (Bytes.to_string buf)) else NoHdr (i - sz, buf);
-        None
+        if sz = i then
+          let partial_pkt = Partial.of_string (Bytes.to_string buf) in
+          (* If there is no body, we can return the full request immediately *)
+          if Partial.to_complete partial_pkt = 0 then
+            reset_and_return partial_pkt
+          else (
+            con.partial_in <- HaveHdr partial_pkt;
+            None
+          )
+        else (
+          con.partial_in <- NoHdr (i - sz, buf);
+          None
+        )
     )
 
 let classify t =
-- 
2.54.0
Re: [PATCH for-4.22 v1] tools/ocaml: xenbus - Fix handling of requests with len = 0 for socket connections
Posted by Andrew Cooper 1 week, 2 days ago
On 09/07/2026 1:00 pm, Andrii Sultanov wrote:
> Following the report at https://lore.kernel.org/xen-devel/CAFLBxZaeTMcF4tcV45MJdCVx4A6qbzQdjKei_Quh_iLrtARVFA@mail.gmail.com/
>
> Without this fix, the backend would be stuck waiting on the zero-sized
> body until the next request comes in.
>
> Instead return the request immediately after reading the header, there's
> no need to wait for another call to .has_more_input and .input
>
> Reported-by: George Dunlap <dunlapg@umich.edu>
> Signed-off-by: Andrii Sultanov <andriy.sultanov@vates.tech>

Oleksii: I'm requesting a release ack on this patch.  It has been
reviewed
(1783604323.8631fc262581453bbf619ec5b2062170.19f471a22b0000edb5@vates.tech
but on a disconnected thread).

There have been two bug reports so far; George (on list), and Benny (on
Matrix).

It is logically a regression vs 4.21.  A new feature in 4.22 causes `xl
list -l`, `xl migrate`, etc to hang when using the oxenstored rather
than (C)xenstored.

There was no reply at first because the security team were investigating
a potential security angle, but we've concluded that there isn't one.

Technically a form of this bug exists in 4.21 also, but it's not used
automatically on any path.  It's triggerable by `xl list -x` and nothing
else we're aware of.

Anyway, oxenstored is the default xenstored if you have Ocaml tools in
the build, so this is a critical bug in 4.22.

~Andrew

Re: [PATCH for-4.22 v1] tools/ocaml: xenbus - Fix handling of requests with len = 0 for socket connections
Posted by Oleksii Kurochko 1 week, 2 days ago

On 7/14/26 12:26 PM, Andrew Cooper wrote:
> On 09/07/2026 1:00 pm, Andrii Sultanov wrote:
>> Following the report at https://lore.kernel.org/xen-devel/CAFLBxZaeTMcF4tcV45MJdCVx4A6qbzQdjKei_Quh_iLrtARVFA@mail.gmail.com/
>>
>> Without this fix, the backend would be stuck waiting on the zero-sized
>> body until the next request comes in.
>>
>> Instead return the request immediately after reading the header, there's
>> no need to wait for another call to .has_more_input and .input
>>
>> Reported-by: George Dunlap <dunlapg@umich.edu>
>> Signed-off-by: Andrii Sultanov <andriy.sultanov@vates.tech>
> 
> Oleksii: I'm requesting a release ack on this patch.  It has been
> reviewed
> (1783604323.8631fc262581453bbf619ec5b2062170.19f471a22b0000edb5@vates.tech
> but on a disconnected thread).
> 
> There have been two bug reports so far; George (on list), and Benny (on
> Matrix).
> 
> It is logically a regression vs 4.21.  A new feature in 4.22 causes `xl
> list -l`, `xl migrate`, etc to hang when using the oxenstored rather
> than (C)xenstored.
> 
> There was no reply at first because the security team were investigating
> a potential security angle, but we've concluded that there isn't one.
> 
> Technically a form of this bug exists in 4.21 also, but it's not used
> automatically on any path.  It's triggerable by `xl list -x` and nothing
> else we're aware of.
> 
> Anyway, oxenstored is the default xenstored if you have Ocaml tools in
> the build, so this is a critical bug in 4.22.

I think this could reasonably be considered a critical bug. It affects a 
default Xen configuration (when OCaml tools are enabled and oxenstored 
is used) and causes core `xl` operations such as `xl list -l` and `xl 
migrate` to hang indefinitely. So:

Release-Acked-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>

Thanks.

~ Oleksii