BUG/MINOR: mux-h1: always make sure h1s->sd exists in h1_dump_h1s_info()
authorWilly Tarreau <w@1wt.eu>
Fri, 21 Feb 2025 10:31:36 +0000 (11:31 +0100)
committerAmaury Denoyelle <adenoyelle@haproxy.com>
Tue, 18 Mar 2025 15:12:48 +0000 (16:12 +0100)
This function may be called from a signal handler during a warning,
a panic or a show thread. We need to be more cautious about what may
or may not be dereferenced since an h1s is not necessarily fully
initialized. Loops of "show threads" sometimes manage to crash when
dereferencing a null h1s->sd, so let's guard it and add a comment
remining about the unusual call place.

This can be backported to the relevant versions.

(cherry picked from commit a56dfbdcb4cb3eb9ffd3db641efb3e5605a6c3f0)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 0ce82061f72bf7f0c1341cdca2f9e90f03437aa4)
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>

src/mux_h1.c

index 0dd0570..d47e80d 100644 (file)
@@ -5220,6 +5220,10 @@ static int h1_dump_h1c_info(struct buffer *msg, struct h1c *h1c, const char *pfx
  * <h1s> is NULL. Returns non-zero if the stream is considered suspicious. May
  * emit multiple lines, each new one being prefixed with <pfx>, if <pfx> is not
  * NULL, otherwise a single line is used.
+ *
+ * Remember that this may be called in a signal context from a "show threads"
+ * or panic dump, so the code must be careful about each data it accesses.
+ * However data are stable since the dump happens from the owner thread.
  */
 static int h1_dump_h1s_info(struct buffer *msg, const struct h1s *h1s, const char *pfx)
 {
@@ -5234,9 +5238,8 @@ static int h1_dump_h1s_info(struct buffer *msg, const struct h1s *h1s, const cha
        else
                method = "UNKNOWN";
 
-       chunk_appendf(msg, " h1s=%p h1s.flg=0x%x .sd.flg=0x%x .req.state=%s .res.state=%s",
-                     h1s, h1s->flags, se_fl_get(h1s->sd),
-                     h1m_state_str(h1s->req.state), h1m_state_str(h1s->res.state));
+       chunk_appendf(msg, " h1s=%p h1s.flg=0x%x", h1s, h1s->flags);
+       chunk_appendf(msg, " .req.state=%s .res.state=%s", h1m_state_str(h1s->req.state), h1m_state_str(h1s->res.state));
 
        if (pfx)
                chunk_appendf(msg, "\n%s", pfx);
@@ -5244,10 +5247,12 @@ static int h1_dump_h1s_info(struct buffer *msg, const struct h1s *h1s, const cha
        chunk_appendf(msg, " .meth=%s status=%d",
                      method, h1s->status);
 
-       chunk_appendf(msg, " .sd.flg=0x%08x", se_fl_get(h1s->sd));
-       if (!se_fl_test(h1s->sd, SE_FL_ORPHAN))
-               chunk_appendf(msg, " .sc.flg=0x%08x .sc.app=%p",
-                             h1s_sc(h1s)->flags, h1s_sc(h1s)->app);
+       if (h1s->sd) {
+                chunk_appendf(msg, " .sd.flg=0x%08x", se_fl_get(h1s->sd));
+                if (!se_fl_test(h1s->sd, SE_FL_ORPHAN))
+                        chunk_appendf(msg, " .sc.flg=0x%08x .sc.app=%p",
+                                      h1s_sc(h1s)->flags, h1s_sc(h1s)->app);
+        }
 
        if (pfx && h1s->subs)
                chunk_appendf(msg, "\n%s", pfx);