From: Thomas Haller <thaller@redhat.com>
To: NetFilter <netfilter-devel@vger.kernel.org>
Cc: Thomas Haller <thaller@redhat.com>
Subject: [PATCH nft v2 5/8] src: rework SNPRINTF_BUFFER_SIZE() and handle truncation
Date: Tue, 29 Aug 2023 14:53:34 +0200 [thread overview]
Message-ID: <20230829125809.232318-6-thaller@redhat.com> (raw)
In-Reply-To: <20230829125809.232318-1-thaller@redhat.com>
Before, the macro asserts against truncation. This is despite the
callers still checked for truncation and tried to handle it. Probably
for good reason. With stmt_evaluate_log_prefix() it's not clear that the
code ensures that truncation cannot happen, so we must not assert
against it, but handle it.
Also,
- wrap the macro in "do { ... } while(0)" to make it more
function-like.
- evaluate macro arguments exactly once, to make it more function-like.
- take pointers to the arguments that are being modified.
- use assert() instead of abort().
- use size_t type for arguments related to the buffer size.
- drop "size". It was mostly redundant to "offset". We can know
everything we want based on "len" and "offset" alone.
- "offset" previously was incremented before checking for truncation.
So it would point somewhere past the buffer. This behavior does not
seem useful. Instead, on truncation "len" will be zero (as before) and
"offset" will point one past the buffer (one past the terminating
NUL).
Thereby, also fix a warning from clang:
evaluate.c:4134:9: error: variable 'size' set but not used [-Werror,-Wunused-but-set-variable]
size_t size = 0;
^
meta.c:1006:9: error: variable 'size' set but not used [-Werror,-Wunused-but-set-variable]
size_t size;
^
Signed-off-by: Thomas Haller <thaller@redhat.com>
---
include/utils.h | 35 ++++++++++++++++++++++++++---------
src/evaluate.c | 8 +++++---
src/meta.c | 11 ++++++-----
3 files changed, 37 insertions(+), 17 deletions(-)
diff --git a/include/utils.h b/include/utils.h
index cee1e5c1e8ae..efc8dec013a1 100644
--- a/include/utils.h
+++ b/include/utils.h
@@ -72,15 +72,32 @@
#define max(_x, _y) ({ \
_x > _y ? _x : _y; })
-#define SNPRINTF_BUFFER_SIZE(ret, size, len, offset) \
- if (ret < 0) \
- abort(); \
- offset += ret; \
- assert(ret < len); \
- if (ret > len) \
- ret = len; \
- size += ret; \
- len -= ret;
+#define SNPRINTF_BUFFER_SIZE(ret, len, offset) \
+ ({ \
+ const int _ret = (ret); \
+ size_t *const _len = (len); \
+ size_t *const _offset = (offset); \
+ bool _not_truncated = true; \
+ size_t _ret2; \
+ \
+ assert(_ret >= 0); \
+ \
+ if ((size_t) _ret >= *_len) { \
+ /* Truncated.
+ *
+ * We will leave "len" at zero and increment
+ * "offset" to point one byte after the buffer
+ * (after the terminating NUL byte). */ \
+ _ret2 = *_len; \
+ _not_truncated = false; \
+ } else \
+ _ret2 = (size_t) _ret; \
+ \
+ *_offset += _ret2; \
+ *_len -= _ret2; \
+ \
+ _not_truncated; \
+ })
#define MSEC_PER_SEC 1000L
diff --git a/src/evaluate.c b/src/evaluate.c
index 77e3838e2692..71e94a51cc2f 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -4129,14 +4129,16 @@ static int stmt_evaluate_queue(struct eval_ctx *ctx, struct stmt *stmt)
static int stmt_evaluate_log_prefix(struct eval_ctx *ctx, struct stmt *stmt)
{
char prefix[NF_LOG_PREFIXLEN] = {}, tmp[NF_LOG_PREFIXLEN] = {};
- int len = sizeof(prefix), offset = 0, ret;
+ size_t len = sizeof(prefix);
+ size_t offset = 0;
struct expr *expr;
- size_t size = 0;
if (stmt->log.prefix->etype != EXPR_LIST)
return 0;
list_for_each_entry(expr, &stmt->log.prefix->expressions, list) {
+ int ret;
+
switch (expr->etype) {
case EXPR_VALUE:
expr_to_string(expr, tmp);
@@ -4150,7 +4152,7 @@ static int stmt_evaluate_log_prefix(struct eval_ctx *ctx, struct stmt *stmt)
BUG("unknown expression type %s\n", expr_name(expr));
break;
}
- SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+ SNPRINTF_BUFFER_SIZE(ret, &len, &offset);
}
if (len == 0)
diff --git a/src/meta.c b/src/meta.c
index 4f383269d032..ea00f2396b99 100644
--- a/src/meta.c
+++ b/src/meta.c
@@ -999,11 +999,11 @@ struct error_record *meta_key_parse(const struct location *loc,
const char *str,
unsigned int *value)
{
- int ret, len, offset = 0;
const char *sep = "";
+ size_t offset = 0;
unsigned int i;
char buf[1024];
- size_t size;
+ size_t len;
for (i = 0; i < array_size(meta_templates); i++) {
if (!meta_templates[i].token || strcmp(meta_templates[i].token, str))
@@ -1026,9 +1026,10 @@ struct error_record *meta_key_parse(const struct location *loc,
}
len = (int)sizeof(buf);
- size = sizeof(buf);
for (i = 0; i < array_size(meta_templates); i++) {
+ int ret;
+
if (!meta_templates[i].token)
continue;
@@ -1036,8 +1037,8 @@ struct error_record *meta_key_parse(const struct location *loc,
sep = ", ";
ret = snprintf(buf+offset, len, "%s%s", sep, meta_templates[i].token);
- SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
- assert(offset < (int)sizeof(buf));
+ SNPRINTF_BUFFER_SIZE(ret, &len, &offset);
+ assert(len > 0);
}
return error(loc, "syntax error, unexpected %s, known keys are %s", str, buf);
--
2.41.0
next prev parent reply other threads:[~2023-08-29 13:00 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-08-29 12:53 [PATCH nft v2 0/8] fix compiler warnings with clang Thomas Haller
2023-08-29 12:53 ` [PATCH nft v2 1/8] netlink: avoid "-Wenum-conversion" warning in dtype_map_from_kernel() Thomas Haller
2023-08-29 12:53 ` [PATCH nft v2 2/8] netlink: avoid "-Wenum-conversion" warning in parser_bison.y Thomas Haller
2023-08-29 12:53 ` [PATCH nft v2 3/8] datatype: avoid cast-align warning with struct sockaddr result from getaddrinfo() Thomas Haller
2023-08-29 12:53 ` [PATCH nft v2 4/8] evaluate: fix check for truncation in stmt_evaluate_log_prefix() Thomas Haller
2023-08-29 12:53 ` Thomas Haller [this message]
2023-08-29 12:53 ` [PATCH nft v2 6/8] evaluate: don't needlessly clear full string buffer " Thomas Haller
2023-08-29 18:08 ` Pablo Neira Ayuso
2023-08-29 18:55 ` Thomas Haller
2023-08-29 12:53 ` [PATCH nft v2 7/8] src: suppress "-Wunused-but-set-variable" warning with "parser_bison.c" Thomas Haller
2023-08-29 12:53 ` [PATCH nft v2 8/8] include: drop "format" attribute from nft_gmp_print() Thomas Haller
2023-08-29 15:00 ` [PATCH nft v2 0/8] fix compiler warnings with clang Pablo Neira Ayuso
2023-08-29 17:52 ` Thomas Haller
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230829125809.232318-6-thaller@redhat.com \
--to=thaller@redhat.com \
--cc=netfilter-devel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).