From 2da02ae8b2ce60b9106fa893dc02917c26734c9c Mon Sep 17 00:00:00 2001 From: Christopher Faulet Date: Thu, 24 Feb 2022 13:45:27 +0100 Subject: [PATCH] BUILD: tree-wide: Avoid warnings about undefined entities retrieved from a CS Since recent changes related to the conn-stream/stream-interface refactoring, GCC reports potential null pointer dereferences when we get the appctx, the stream or the stream-interface from the conn-strem. Of course, depending on the time, these entities may be null. But at many places, we know they are defined and it is safe to get them without any check. Thus, we use ALREADY_CHECKED() macro to silent these warnings. Note that the refactoring is unfinished, so it is not a real issue for now. --- include/haproxy/applet.h | 2 ++ include/haproxy/conn_stream.h | 2 +- include/haproxy/stream_interface.h | 25 ++++++++++++++++++++----- src/hlua.c | 6 +++++- src/http_ana.c | 1 + src/mux_pt.c | 5 ++++- src/proxy.c | 5 ++++- src/stats.c | 8 ++++++++ src/stream.c | 6 ++++-- 9 files changed, 49 insertions(+), 11 deletions(-) diff --git a/include/haproxy/applet.h b/include/haproxy/applet.h index 97b9c347b..b4c33ea26 100644 --- a/include/haproxy/applet.h +++ b/include/haproxy/applet.h @@ -113,6 +113,8 @@ static inline void appctx_free(struct appctx *appctx) /* wakes up an applet when conditions have changed */ static inline void appctx_wakeup(struct appctx *appctx) { + ALREADY_CHECKED(appctx); + task_wakeup(appctx->t, TASK_WOKEN_OTHER); } diff --git a/include/haproxy/conn_stream.h b/include/haproxy/conn_stream.h index 517d6d2f6..5dc9cf265 100644 --- a/include/haproxy/conn_stream.h +++ b/include/haproxy/conn_stream.h @@ -93,7 +93,7 @@ static inline struct check *cs_check(const struct conn_stream *cs) static inline struct stream_interface *cs_si(const struct conn_stream *cs) { - return (cs_strm(cs) ? cs->si : NULL); + return cs->si; } static inline const char *cs_get_data_name(const struct conn_stream *cs) diff --git a/include/haproxy/stream_interface.h b/include/haproxy/stream_interface.h index 8119a4233..88fc16f83 100644 --- a/include/haproxy/stream_interface.h +++ b/include/haproxy/stream_interface.h @@ -55,13 +55,19 @@ void si_sync_send(struct stream_interface *si); /* returns the channel which receives data from this stream interface (input channel) */ static inline struct channel *si_ic(struct stream_interface *si) { - return ((si->flags & SI_FL_ISBACK) ? &(cs_strm(si->cs)->res) : &(cs_strm(si->cs)->req)); + struct stream *strm = cs_strm(si->cs); + + ALREADY_CHECKED(strm); + return ((si->flags & SI_FL_ISBACK) ? &(strm->res) : &(strm->req)); } /* returns the channel which feeds data to this stream interface (output channel) */ static inline struct channel *si_oc(struct stream_interface *si) { - return ((si->flags & SI_FL_ISBACK) ? &(cs_strm(si->cs)->req) : &(cs_strm(si->cs)->res)); + struct stream *strm = cs_strm(si->cs); + + ALREADY_CHECKED(strm); + return ((si->flags & SI_FL_ISBACK) ? &(strm->req) : &(strm->res)); } /* returns the buffer which receives data from this stream interface (input channel's buffer) */ @@ -79,19 +85,28 @@ static inline struct buffer *si_ob(struct stream_interface *si) /* returns the stream associated to a stream interface */ static inline struct stream *si_strm(struct stream_interface *si) { - return cs_strm(si->cs); + struct stream *strm = cs_strm(si->cs); + + ALREADY_CHECKED(strm); + return strm; } /* returns the task associated to this stream interface */ static inline struct task *si_task(struct stream_interface *si) { - return cs_strm(si->cs)->task; + struct stream *strm = cs_strm(si->cs); + + ALREADY_CHECKED(strm); + return strm->task; } /* returns the stream interface on the other side. Used during forwarding. */ static inline struct stream_interface *si_opposite(struct stream_interface *si) { - return ((si->flags & SI_FL_ISBACK) ? cs_strm(si->cs)->csf->si : cs_strm(si->cs)->csb->si); + struct stream *strm = cs_strm(si->cs); + + ALREADY_CHECKED(strm); + return ((si->flags & SI_FL_ISBACK) ? strm->csf->si : strm->csb->si); } /* initializes a stream interface in the SI_ST_INI state and create the event diff --git a/src/hlua.c b/src/hlua.c index 3b2bd73ef..8b42746d8 100644 --- a/src/hlua.c +++ b/src/hlua.c @@ -2685,6 +2685,7 @@ __LJMP static int hlua_socket_connect_yield(struct lua_State *L, int status, lua } appctx = cs_appctx(s->csf); + ALREADY_CHECKED(appctx); /* Check for connection established. */ if (appctx->ctx.hlua_cosocket.connected) { @@ -4272,7 +4273,10 @@ static int hlua_applet_tcp_new(lua_State *L, struct appctx *ctx) struct hlua_appctx *luactx; struct stream_interface *si = cs_si(ctx->owner); struct stream *s = si_strm(si); - struct proxy *p = s->be; + struct proxy *p; + + ALREADY_CHECKED(s); + p = s->be; /* Check stack size. */ if (!lua_checkstack(L, 3)) diff --git a/src/http_ana.c b/src/http_ana.c index a3fd15cb7..ff98a7bdd 100644 --- a/src/http_ana.c +++ b/src/http_ana.c @@ -3883,6 +3883,7 @@ static int http_handle_stats(struct stream *s, struct channel *req) struct htx_sl *sl; appctx = cs_appctx(s->csb); + ALREADY_CHECKED(appctx); memset(&appctx->ctx.stats, 0, sizeof(appctx->ctx.stats)); appctx->st1 = appctx->st2 = 0; appctx->ctx.stats.st_code = STAT_STATUS_INIT; diff --git a/src/mux_pt.c b/src/mux_pt.c index 9f1aaa877..1a706270f 100644 --- a/src/mux_pt.c +++ b/src/mux_pt.c @@ -407,7 +407,10 @@ static void mux_pt_destroy_meth(void *ctx) static void mux_pt_detach(struct conn_stream *cs) { struct connection *conn = cs_conn(cs); - struct mux_pt_ctx *ctx = conn->ctx; + struct mux_pt_ctx *ctx; + + ALREADY_CHECKED(conn); + ctx = conn->ctx; TRACE_ENTER(PT_EV_STRM_END, conn, cs); diff --git a/src/proxy.c b/src/proxy.c index c392b4c6c..c486376a5 100644 --- a/src/proxy.c +++ b/src/proxy.c @@ -2613,7 +2613,7 @@ static void dump_server_addr(const struct sockaddr_storage *addr, char *addr_str static int dump_servers_state(struct stream_interface *si) { struct appctx *appctx = cs_appctx(si->cs); - struct proxy *px = appctx->ctx.cli.p0; + struct proxy *px; struct server *srv; char srv_addr[INET6_ADDRSTRLEN + 1]; char srv_agent_addr[INET6_ADDRSTRLEN + 1]; @@ -2622,6 +2622,9 @@ static int dump_servers_state(struct stream_interface *si) int bk_f_forced_id, srv_f_forced_id; char *srvrecord; + ALREADY_CHECKED(appctx); + px = appctx->ctx.cli.p0; + if (!appctx->ctx.cli.p1) appctx->ctx.cli.p1 = px->srv; diff --git a/src/stats.c b/src/stats.c index bffd3e395..854515b26 100644 --- a/src/stats.c +++ b/src/stats.c @@ -1984,6 +1984,8 @@ static int stats_dump_li_stats(struct stream_interface *si, struct proxy *px, st struct stats_module *mod; size_t stats_count = ST_F_TOTAL_FIELDS; + ALREADY_CHECKED(appctx); + memset(stats, 0, sizeof(struct field) * stat_count[STATS_DOMAIN_PROXY]); if (!stats_fill_li_stats(px, l, appctx->ctx.stats.flags, stats, @@ -2495,6 +2497,8 @@ static int stats_dump_sv_stats(struct stream_interface *si, struct proxy *px, st struct field *stats = stat_l[STATS_DOMAIN_PROXY]; size_t stats_count = ST_F_TOTAL_FIELDS; + ALREADY_CHECKED(appctx); + memset(stats, 0, sizeof(struct field) * stat_count[STATS_DOMAIN_PROXY]); if (!stats_fill_sv_stats(px, sv, appctx->ctx.stats.flags, stats, @@ -4205,6 +4209,8 @@ static int stats_send_http_redirect(struct stream_interface *si, struct htx *htx struct htx_sl *sl; unsigned int flags; + ALREADY_CHECKED(appctx); + /* scope_txt = search pattern + search query, appctx->ctx.stats.scope_len is always <= STAT_SCOPE_TXT_MAXLEN */ scope_txt[0] = 0; if (appctx->ctx.stats.scope_len) { @@ -4518,6 +4524,8 @@ static int stats_dump_info_to_buffer(struct stream_interface *si) { struct appctx *appctx = cs_appctx(si->cs); + ALREADY_CHECKED(appctx); + if (!stats_fill_info(info, INF_TOTAL_FIELDS, appctx->ctx.stats.flags)) return 0; diff --git a/src/stream.c b/src/stream.c index 935fce42d..37cd79378 100644 --- a/src/stream.c +++ b/src/stream.c @@ -980,11 +980,11 @@ enum act_return process_use_service(struct act_rule *rule, struct proxy *px, if (flags & ACT_OPT_FIRST) { /* Register applet. this function schedules the applet. */ s->target = &rule->applet.obj_type; - if (unlikely(!si_register_handler(cs_si(s->csb), objt_applet(s->target)))) + appctx = si_register_handler(cs_si(s->csb), objt_applet(s->target)); + if (unlikely(!appctx)) return ACT_RET_ERR; /* Initialise the context. */ - appctx = cs_appctx(s->csb); memset(&appctx->ctx, 0, sizeof(appctx->ctx)); appctx->rule = rule; } @@ -3101,6 +3101,8 @@ static int stats_dump_full_strm_to_buffer(struct stream_interface *si, struct st struct connection *conn; struct appctx *tmpctx; + ALREADY_CHECKED(appctx); + chunk_reset(&trash); if (appctx->ctx.sess.section > 0 && appctx->ctx.sess.uid != strm->uniq_id) {