From 10c1a8c3bd6a24f51616172a37f1e3ef58f8fa84 Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Wed, 20 Oct 2021 17:29:28 +0200 Subject: [PATCH] BUILD: resolvers: avoid a possible warning on null-deref Depending on the code that precedes the loop, gcc may emit this warning: src/resolvers.c: In function 'resolv_process_responses': src/resolvers.c:1009:11: warning: potential null pointer dereference [-Wnull-dereference] 1009 | if (query->type != DNS_RTYPE_SRV && flags & DNS_FLAG_TRUNCATED) { | ~~~~~^~~~~~ However after carefully checking, r_res->header.qdcount it exclusively 1 when reaching this place, which forces the for() loop to enter for at least one iteration, and to be set. Thus there's no code path leading to a null deref. It's possibly just because the assignment is too far and the compiler cannot figure that the condition is always OK. Let's just mark it to please the compiler. --- src/resolvers.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/resolvers.c b/src/resolvers.c index 15895d580..6041b43d3 100644 --- a/src/resolvers.c +++ b/src/resolvers.c @@ -961,8 +961,17 @@ static int resolv_validate_dns_response(unsigned char *resp, unsigned char *bufe reader += 2; } + /* Let's just make gcc happy. The tests above make it clear that + * qdcount==1 hence that we necessarily enter into the loop at least + * once, but gcc seems to be having difficulties following it and + * warns about the risk of NULL dereference at the next line, even + * if a BUG_ON(!query) is used. + */ + ALREADY_CHECKED(query); + /* TRUNCATED flag must be checked after we could read the query type - * because a TRUNCATED SRV query type response can still be exploited */ + * because a TRUNCATED SRV query type response can still be exploited + */ if (query->type != DNS_RTYPE_SRV && flags & DNS_FLAG_TRUNCATED) { cause = RSLV_RESP_TRUNCATED; goto return_error;