From: Aurelien DARRAGON <adarragon@haproxy.com>
Date: Wed, 22 Feb 2023 13:34:28 +0000 (+0100)
Subject: MEDIUM: proto_ux: properly suspend named UNIX listeners
X-Git-Tag: v2.8-dev5~105
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=2338dba18db3a895fd394a6c07544f03c7f39167;p=thirdparty%2Fhaproxy.git

MEDIUM: proto_ux: properly suspend named UNIX listeners

When a listener is suspended, we expect that it may not process any data for
the time it is suspended.

Yet for named UNIX socket, as the suspend operation is a no-op at the proto
level, recv events on the socket may still be processed by the polling loop.

This is quite disturbing as someone may rely on a paused proxy being harmless,
which is true for all protos except for named UNIX sockets.

To fix this behavior, we explicitely disable io recv events when suspending a
named UNIX socket listener (we call disable() method on the listener).

The io recv events will automatically be restored when the listener is resumed
since the l->enable() method is called at the end of the resume() operation.

This could be backported up to 2.4 after a reasonable observation
period to make sure that this change doesn't cause unwanted side-effects.
---

diff --git a/src/proto_uxdg.c b/src/proto_uxdg.c
index 68fe207de0..0e3e1d0f34 100644
--- a/src/proto_uxdg.c
+++ b/src/proto_uxdg.c
@@ -126,17 +126,20 @@ static void uxdg_disable_listener(struct listener *l)
 }
 
 /* Suspend a receiver. Returns < 0 in case of failure, 0 if the receiver
- * was totally stopped, or > 0 if correctly suspended. Nothing is done for
- * plain unix sockets since currently it's the new process which handles
- * the renaming. Abstract sockets are completely unbound and closed so
- * there's no need to stop the poller.
+ * was totally stopped, or > 0 if correctly suspended. For plain unix sockets
+ * we only disable the listener to prevent data from being handled but nothing
+ * more is done since currently it's the new process which handles the renaming.
+ * Abstract sockets are completely unbound and closed so there's no need to stop
+ * the poller.
  */
 static int uxdg_suspend_receiver(struct receiver *rx)
 {
         struct listener *l = LIST_ELEM(rx, struct listener *, rx);
 
-        if (((struct sockaddr_un *)&rx->addr)->sun_path[0])
+        if (((struct sockaddr_un *)&rx->addr)->sun_path[0]) {
+		uxdg_disable_listener(l);
                 return 1;
+	}
 
         /* Listener's lock already held. Call lockless version of
          * unbind_listener. */
diff --git a/src/proto_uxst.c b/src/proto_uxst.c
index 08b8d27471..9678a49a8c 100644
--- a/src/proto_uxst.c
+++ b/src/proto_uxst.c
@@ -165,17 +165,20 @@ static void uxst_disable_listener(struct listener *l)
 }
 
 /* Suspend a receiver. Returns < 0 in case of failure, 0 if the receiver
- * was totally stopped, or > 0 if correctly suspended. Nothing is done for
- * plain unix sockets since currently it's the new process which handles
- * the renaming. Abstract sockets are completely unbound and closed so
- * there's no need to stop the poller.
+ * was totally stopped, or > 0 if correctly suspended. For plain unix sockets
+ * we only disable the listener to prevent data from being handled but nothing
+ * more is done since currently it's the new process which handles the renaming.
+ * Abstract sockets are completely unbound and closed so there's no need to stop
+ * the poller.
  */
 static int uxst_suspend_receiver(struct receiver *rx)
 {
 	struct listener *l = LIST_ELEM(rx, struct listener *, rx);
 
-	if (((struct sockaddr_un *)&rx->addr)->sun_path[0])
+	if (((struct sockaddr_un *)&rx->addr)->sun_path[0]) {
+		uxst_disable_listener(l);
 		return 1;
+	}
 
 	/* Listener's lock already held. Call lockless version of
 	 * unbind_listener. */