DEBUG: connection: mark the closed FDs with a value that is easier to detect

Setting an FD to -1 when closed isn't the most easily noticeable thing
to do when we're chasing accidental reuse of a stale file descriptor.
Instead set it to that large a negative value that it will overflow the
fdtab and provide an analysable core at the moment the issue happens.
Care was taken to ensure it doesn't overflow nor change sign on 32-bit
machines when multiplied by fdtab, and that it also remains negative for
the various checks that exist. The value equals 0xFDDEADFD which happens
to be easily spotted in a debugger.

diff --git a/include/proto/connection.h b/include/proto/connection.h
index 397c4781f369f1ea094794fc274bcdef19a17976..8921a06fddc34fde3238464d81c431480d43e849 100644 (file)
--- a/include/proto/connection.h
+++ b/include/proto/connection.h
@@ -152,6 +152,7 @@ static inline void conn_force_close(struct connection *conn)
        if (conn_ctrl_ready(conn))
                fd_delete(conn->t.sock.fd);
 
+       conn->t.sock.fd = DEAD_FD_MAGIC;
        conn->flags &= ~(CO_FL_XPRT_READY|CO_FL_CTRL_READY);
 }
 
@@ -481,7 +482,7 @@ static inline void conn_init(struct connection *conn)
        conn->data = NULL;
        conn->owner = NULL;
        conn->send_proxy_ofs = 0;
-       conn->t.sock.fd = -1; /* just to help with debugging */
+       conn->t.sock.fd = DEAD_FD_MAGIC;
        conn->err_code = CO_ER_NONE;
        conn->target = NULL;
        conn->proxy_netns = NULL;

diff --git a/include/types/fd.h b/include/types/fd.h
index 8299a0264c1a6f4cf82d28d539354b8cd7ed64c8..7f63093548f21255135e03aaae5a22bc5d364a2d 100644 (file)
--- a/include/types/fd.h
+++ b/include/types/fd.h
@@ -76,6 +76,19 @@ enum fd_states {
        FD_ST_READY
 };
 
+
+/* This is the value used to mark a file descriptor as dead. This value is
+ * negative, this is important so that tests on fd < 0 properly match. It
+ * also has the nice property of being highly negative but not overflowing
+ * nor changing sign on 32-bit machines when multipled by sizeof(fdtab).
+ * This ensures that any unexpected dereference of such an uninitialized
+ * file descriptor will lead to so large a dereference that it will crash
+ * the process at the exact location of the bug with a clean stack trace
+ * instead of causing silent manipulation of other FDs. And it's readable
+ * when found in a dump.
+ */
+#define DEAD_FD_MAGIC 0xFDDEADFD
+
 /* info about one given fd */
 struct fdtab {
        void (*iocb)(int fd);                /* I/O handler */

diff --git a/src/dumpstats.c b/src/dumpstats.c
index 5cc09aa22de93bdb50d648d10e0e7d22e97d716c..4c3f8302738ecc98b6eec0f7202117d8e404e5b5 100644 (file)
--- a/src/dumpstats.c
+++ b/src/dumpstats.c
@@ -6088,13 +6088,15 @@ static int stats_dump_full_sess_to_buffer(struct stream_interface *si, struct st
                                      obj_type_name(conn->target),
                                      obj_base_ptr(conn->target));
 
-                       chunk_appendf(&trash,
-                                     "      flags=0x%08x fd=%d fd.state=%02x fd.cache=%d updt=%d\n",
-                                     conn->flags,
-                                     conn->t.sock.fd,
-                                     conn->t.sock.fd >= 0 ? fdtab[conn->t.sock.fd].state : 0,
-                                     conn->t.sock.fd >= 0 ? fdtab[conn->t.sock.fd].cache : 0,
-                                     conn->t.sock.fd >= 0 ? fdtab[conn->t.sock.fd].updated : 0);
+                       chunk_appendf(&trash, "      flags=0x%08x", conn->flags);
+
+                       if (conn->t.sock.fd >= 0) {
+                               chunk_appendf(&trash, " fd=%d fd.state=%02x fd.cache=%d updt=%d\n",
+                                             conn->t.sock.fd, fdtab[conn->t.sock.fd].state,
+                                             fdtab[conn->t.sock.fd].cache, fdtab[conn->t.sock.fd].updated);
+                       }
+                       else
+                               chunk_appendf(&trash, " fd=<dead>\n");
                }
                else if ((tmpctx = objt_appctx(sess->si[0].end)) != NULL) {
                        chunk_appendf(&trash,
@@ -6116,13 +6118,15 @@ static int stats_dump_full_sess_to_buffer(struct stream_interface *si, struct st
                                      obj_type_name(conn->target),
                                      obj_base_ptr(conn->target));
 
-                       chunk_appendf(&trash,
-                                     "      flags=0x%08x fd=%d fd.state=%02x fd.cache=%d updt=%d\n",
-                                     conn->flags,
-                                     conn->t.sock.fd,
-                                     conn->t.sock.fd >= 0 ? fdtab[conn->t.sock.fd].state : 0,
-                                     conn->t.sock.fd >= 0 ? fdtab[conn->t.sock.fd].cache : 0,
-                                     conn->t.sock.fd >= 0 ? fdtab[conn->t.sock.fd].updated : 0);
+                       chunk_appendf(&trash, "      flags=0x%08x", conn->flags);
+
+                       if (conn->t.sock.fd >= 0) {
+                               chunk_appendf(&trash, " fd=%d fd.state=%02x fd.cache=%d updt=%d\n",
+                                             conn->t.sock.fd, fdtab[conn->t.sock.fd].state,
+                                             fdtab[conn->t.sock.fd].cache, fdtab[conn->t.sock.fd].updated);
+                       }
+                       else
+                               chunk_appendf(&trash, " fd=<dead>\n");
                }
                else if ((tmpctx = objt_appctx(sess->si[1].end)) != NULL) {
                        chunk_appendf(&trash,
@@ -6688,7 +6692,7 @@ static int stats_dump_sess_to_buffer(struct stream_interface *si)
                                     " s0=[%d,%1xh,fd=%d,ex=%s]",
                                     curr_sess->si[0].state,
                                     curr_sess->si[0].flags,
-                                    conn ? conn->t.sock.fd : -1,
+                                    (conn && conn->t.sock.fd >= 0) ? conn->t.sock.fd : -1,
                                     curr_sess->si[0].exp ?
                                     human_time(TICKS_TO_MS(curr_sess->si[0].exp - now_ms),
                                                TICKS_TO_MS(1000)) : "");
@@ -6698,7 +6702,7 @@ static int stats_dump_sess_to_buffer(struct stream_interface *si)
                                     " s1=[%d,%1xh,fd=%d,ex=%s]",
                                     curr_sess->si[1].state,
                                     curr_sess->si[1].flags,
-                                    conn ? conn->t.sock.fd : -1,
+                                    (conn && conn->t.sock.fd >= 0) ? conn->t.sock.fd : -1,
                                     curr_sess->si[1].exp ?
                                     human_time(TICKS_TO_MS(curr_sess->si[1].exp - now_ms),
                                                TICKS_TO_MS(1000)) : "");