* [PATCH RESEND 1/4] parse: initial parsing of __attribute__((format))
2025-10-16 11:04 Add printf/scanf -Wformat checking Ben Dooks
@ 2025-10-16 11:04 ` Ben Dooks
2025-10-16 11:04 ` [PATCH RESEND 2/4] add -Wformat Ben Dooks
` (3 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: Ben Dooks @ 2025-10-16 11:04 UTC (permalink / raw)
To: linux-sparse, Luc Van Oostenryck, Chris Li; +Cc: Ben Dooks
Add code to parse the __attribute__((format)) used to indicate that
a variadic function takes a printf-style format string and where
those are. Save the data in ctype ready for checking when such an
function is encoutered.
Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
---
parse.c | 81 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
symbol.h | 9 ++++++-
2 files changed, 88 insertions(+), 2 deletions(-)
diff --git a/parse.c b/parse.c
index 3f67451e..af4e5b50 100644
--- a/parse.c
+++ b/parse.c
@@ -86,7 +86,7 @@ static attr_t
attribute_cleanup,
attribute_designated_init,
attribute_transparent_union, ignore_attribute,
- attribute_mode, attribute_force;
+ attribute_mode, attribute_force, attribute_format;
typedef struct symbol *to_mode_t(struct symbol *);
@@ -121,6 +121,12 @@ static void asm_modifier(struct token *token, unsigned long *mods, unsigned long
*mods |= mod;
}
+/* the types of formatting from __attribute__((format)) */
+enum {
+ FMT_PRINTF = 0,
+ FMT_SCANF,
+};
+
static struct symbol_op typedef_op = {
.type = KW_MODIFIER,
.declarator = storage_specifier,
@@ -382,6 +388,10 @@ static struct symbol_op attr_force_op = {
.attribute = attribute_force,
};
+static struct symbol_op attr_format_op = {
+ .attribute = attribute_format,
+};
+
static struct symbol_op address_space_op = {
.attribute = attribute_address_space,
};
@@ -441,6 +451,16 @@ static struct symbol_op mode_word_op = {
.to_mode = to_word_mode
};
+static struct symbol_op attr_printf_op = {
+ .type = KW_FORMAT,
+ .class = FMT_PRINTF,
+};
+
+static struct symbol_op attr_scanf_op = {
+ .type = KW_FORMAT,
+ .class = FMT_SCANF,
+};
+
/*
* Define the keyword and their effects.
* The entries in the 'typedef' and put in NS_TYPEDEF and
@@ -557,6 +577,9 @@ static struct init_keyword {
D("pure", &attr_fun_op, .mods = MOD_PURE),
A("const", &attr_fun_op, .mods = MOD_PURE),
D("gnu_inline", &attr_fun_op, .mods = MOD_GNU_INLINE),
+ D("format", &attr_format_op),
+ D("printf", &attr_printf_op),
+ D("scanf", &attr_scanf_op),
/* Modes */
D("mode", &mode_op),
@@ -1217,6 +1240,60 @@ static struct token *attribute_address_space(struct token *token, struct symbol
return token;
}
+static int invalid_format_args(long long start, long long at)
+{
+ return start < 0 || at < 0 || start > USHRT_MAX || at > USHRT_MAX ||
+ (start == at && start > 0) ||
+ (start == 0 && at == 0);
+}
+
+static struct token *attribute_format(struct token *token, struct symbol *attr, struct decl_state *ctx)
+{
+ struct expression *args[3];
+ struct symbol *fmt_sym = NULL;
+
+ /* expecting format ( type, start, va_args at) */
+
+ token = expect(token, '(', "after format attribute");
+ if (token_type(token) == TOKEN_IDENT)
+ fmt_sym = lookup_keyword(token->ident, NS_KEYWORD);
+ if (fmt_sym)
+ if (!fmt_sym->op || fmt_sym->op->type != KW_FORMAT)
+ fmt_sym = NULL;
+
+ token = conditional_expression(token, &args[0]);
+ token = expect(token, ',', "format attribute type");
+ token = conditional_expression(token, &args[1]);
+ token = expect(token, ',', "format attribute type position");
+ token = conditional_expression(token, &args[2]);
+ token = expect(token, ')', "format attribute arg position");
+
+ if (!fmt_sym || !args[0] || !args[1] || !args[2]) {
+ warning(token->pos, "incorrect format attribute");
+ } else if (fmt_sym->op->class != FMT_PRINTF) {
+ /* skip anything that isn't printf for the moment */
+ warning(token->pos, "only printf format attribute supported");
+ } else {
+ long long start, at;
+
+ start = get_expression_value(args[2]);
+ at = get_expression_value(args[1]);
+
+ if (invalid_format_args(start, at)) {
+ warning(token->pos, "bad format positions");
+ } else if (start == 0) {
+ /* nothing to do here, is va_list function */
+ } else if (start < at) {
+ warning(token->pos, "format cannot be after va_args");
+ } else {
+ ctx->ctype.format.index = at;
+ ctx->ctype.format.first = start;
+ }
+ }
+
+ return token;
+}
+
static struct symbol *to_QI_mode(struct symbol *ctype)
{
if (ctype->ctype.base_type != &int_type)
@@ -3007,6 +3084,8 @@ struct token *external_declaration(struct token *token, struct symbol_list **lis
if (!(decl->ctype.modifiers & MOD_STATIC))
decl->ctype.modifiers |= MOD_EXTERN;
+
+ base_type->ctype.format = decl->ctype.format;
} else if (base_type == &void_ctype && !(decl->ctype.modifiers & MOD_EXTERN)) {
sparse_error(token->pos, "void declaration");
}
diff --git a/symbol.h b/symbol.h
index 88130c15..0ea46da8 100644
--- a/symbol.h
+++ b/symbol.h
@@ -82,8 +82,9 @@ enum keyword {
KW_ASM = 1 << 5,
KW_MODE = 1 << 6,
KW_STATIC = 1 << 7,
- // KW UNUSED = 1 << 8,
+ // KW_UNUSED = 1 << 8.
KW_EXACT = 1 << 9,
+ KW_FORMAT = 1 << 10,
};
struct context {
@@ -95,12 +96,18 @@ extern struct context *alloc_context(void);
DECLARE_PTR_LIST(context_list, struct context);
+struct attr_format {
+ unsigned short index; /* index in argument list for format string */
+ unsigned short first; /* where first variadic argument is */
+};
+
struct ctype {
struct symbol *base_type;
unsigned long modifiers;
unsigned long alignment;
struct context_list *contexts;
struct ident *as;
+ struct attr_format format;
};
struct decl_state {
--
2.37.2.352.g3c44437643
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH RESEND 2/4] add -Wformat
2025-10-16 11:04 Add printf/scanf -Wformat checking Ben Dooks
2025-10-16 11:04 ` [PATCH RESEND 1/4] parse: initial parsing of __attribute__((format)) Ben Dooks
@ 2025-10-16 11:04 ` Ben Dooks
2025-10-16 11:04 ` [PATCH RESEND 3/4] evaluate: check variadic argument types against formatting info Ben Dooks
` (2 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: Ben Dooks @ 2025-10-16 11:04 UTC (permalink / raw)
To: linux-sparse, Luc Van Oostenryck, Chris Li; +Cc: Ben Dooks
Add option to enable/disable format checking (and default it to off)
Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
---
options.c | 2 ++
options.h | 1 +
sparse.1 | 8 ++++++++
3 files changed, 11 insertions(+)
diff --git a/options.c b/options.c
index 6ee4d878..54ac00b4 100644
--- a/options.c
+++ b/options.c
@@ -106,6 +106,7 @@ int Wflexible_array_array = 1;
int Wflexible_array_nested = 0;
int Wflexible_array_sizeof = 0;
int Wflexible_array_union = 0;
+int Wformat = 0;
int Wimplicit_int = 1;
int Winit_cstring = 0;
int Wint_to_pointer_cast = 1;
@@ -865,6 +866,7 @@ static const struct flag warnings[] = {
{ "flexible-array-nested", &Wflexible_array_nested },
{ "flexible-array-sizeof", &Wflexible_array_sizeof },
{ "flexible-array-union", &Wflexible_array_union },
+ { "format", &Wformat },
{ "implicit-int", &Wimplicit_int },
{ "init-cstring", &Winit_cstring },
{ "int-to-pointer-cast", &Wint_to_pointer_cast },
diff --git a/options.h b/options.h
index c2a9551a..105c45d0 100644
--- a/options.h
+++ b/options.h
@@ -106,6 +106,7 @@ extern int Wflexible_array_array;
extern int Wflexible_array_nested;
extern int Wflexible_array_sizeof;
extern int Wflexible_array_union;
+extern int Wformat;
extern int Wimplicit_int;
extern int Winit_cstring;
extern int Wint_to_pointer_cast;
diff --git a/sparse.1 b/sparse.1
index 2fba7e7a..64b0571e 100644
--- a/sparse.1
+++ b/sparse.1
@@ -285,6 +285,14 @@ To have any effect, at least one of \fB-Wflexible-array-array\fR,
be enabled.
Sparse does issue these warnings by default.
+.B \-Wformat
+Warn about parameter mismatch to any variadic function which specifies
+where the format string is specified with the
+.BI __attribute__((format( type, message, va_start )))
+attribute.
+
+Sparse does not issue these warnings by default. To turn them on, use
+\fB\-W-format\fR.
.
.TP
.B \-Winit\-cstring
--
2.37.2.352.g3c44437643
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH RESEND 3/4] evaluate: check variadic argument types against formatting info
2025-10-16 11:04 Add printf/scanf -Wformat checking Ben Dooks
2025-10-16 11:04 ` [PATCH RESEND 1/4] parse: initial parsing of __attribute__((format)) Ben Dooks
2025-10-16 11:04 ` [PATCH RESEND 2/4] add -Wformat Ben Dooks
@ 2025-10-16 11:04 ` Ben Dooks
2025-10-16 11:04 ` [PATCH RESEND 4/4] tests: add varargs printf format tests Ben Dooks
2025-10-16 17:12 ` Add printf/scanf -Wformat checking Linus Torvalds
4 siblings, 0 replies; 9+ messages in thread
From: Ben Dooks @ 2025-10-16 11:04 UTC (permalink / raw)
To: linux-sparse, Luc Van Oostenryck, Chris Li; +Cc: Ben Dooks
The variadic argumebt code did not check any of the variadic arguments
as it did not previously know the possible type. Now we have the possible
formatting information stored in the ctype, we can do some checks on the
printf formatting types.
Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
---
Makefile | 1 +
builtin.c | 4 +-
evaluate.c | 14 +-
evaluate.h | 10 +-
verify-format.c | 504 ++++++++++++++++++++++++++++++++++++++++++++++++
verify-format.h | 6 +
6 files changed, 532 insertions(+), 7 deletions(-)
create mode 100644 verify-format.c
create mode 100644 verify-format.h
diff --git a/Makefile b/Makefile
index e172758b..670e95aa 100644
--- a/Makefile
+++ b/Makefile
@@ -90,6 +90,7 @@ LIB_OBJS += tokenize.o
LIB_OBJS += unssa.o
LIB_OBJS += utils.o
LIB_OBJS += version.o
+LIB_OBJS += verify-format.o
PROGRAMS :=
PROGRAMS += compile
diff --git a/builtin.c b/builtin.c
index 3a29c3ae..e4751445 100644
--- a/builtin.c
+++ b/builtin.c
@@ -438,7 +438,7 @@ static int evaluate_generic_int_op(struct expression *expr)
NEXT_PTR_LIST(t);
} END_FOR_EACH_PTR(arg);
FINISH_PTR_LIST(t);
- return evaluate_arguments(types, expr->args);
+ return evaluate_arguments(NULL, types, expr->args);
err:
sparse_error(arg->pos, "non-integer type for argument %d:", n);
@@ -502,7 +502,7 @@ static int eval_atomic_common(struct expression *expr)
if (!expr->ctype) // set the return type, if needed
expr->ctype = ctype;
- return evaluate_arguments(types, expr->args);
+ return evaluate_arguments(NULL, types, expr->args);
err:
sparse_error(arg->pos, "invalid type for argument %d:", n);
diff --git a/evaluate.c b/evaluate.c
index fe716f63..4ffbba73 100644
--- a/evaluate.c
+++ b/evaluate.c
@@ -42,6 +42,7 @@
#include "symbol.h"
#include "target.h"
#include "expression.h"
+#include "verify-format.h"
struct symbol *current_fn;
@@ -1386,8 +1387,8 @@ static int whitelist_pointers(struct symbol *t1, struct symbol *t2)
return !Wtypesign;
}
-static int check_assignment_types(struct symbol *target, struct expression **rp,
- const char **typediff)
+int check_assignment_types(struct symbol *target, struct expression **rp,
+ const char **typediff)
{
struct symbol *source = degenerate(*rp);
struct symbol *t, *s;
@@ -2324,7 +2325,8 @@ static struct symbol *evaluate_alignof(struct expression *expr)
return size_t_ctype;
}
-int evaluate_arguments(struct symbol_list *argtypes, struct expression_list *head)
+int evaluate_arguments(struct symbol *fn, struct symbol_list *argtypes,
+ struct expression_list *head)
{
struct expression *expr;
struct symbol *argtype;
@@ -2365,6 +2367,10 @@ int evaluate_arguments(struct symbol_list *argtypes, struct expression_list *hea
NEXT_PTR_LIST(argtype);
} END_FOR_EACH_PTR(expr);
FINISH_PTR_LIST(argtype);
+
+ if (fn && Wformat)
+ verify_format_attribute(fn, head);
+
return 1;
}
@@ -3191,7 +3197,7 @@ static struct symbol *evaluate_call(struct expression *expr)
if (!sym->op->args(expr))
return NULL;
} else {
- if (!evaluate_arguments(ctype->arguments, arglist))
+ if (!evaluate_arguments(ctype, ctype->arguments, arglist))
return NULL;
args = expression_list_size(expr->args);
fnargs = symbol_list_size(ctype->arguments);
diff --git a/evaluate.h b/evaluate.h
index a16e9703..3f51129d 100644
--- a/evaluate.h
+++ b/evaluate.h
@@ -28,8 +28,16 @@ void evaluate_symbol_list(struct symbol_list *list);
///
// evaluate the arguments of a function
+// @fn: the symbol of the prototype
// @argtypes: the list of the types in the prototype
// @args: the list of the effective arguments
-int evaluate_arguments(struct symbol_list *argtypes, struct expression_list *args);
+int evaluate_arguments(struct symbol *fn, struct symbol_list *argtypes, struct expression_list *args);
+///
+// check if assignment types are compatible
+// @target: the target assignment
+// @rp: the expression
+// @typediff: the resulant message if different type
+int check_assignment_types(struct symbol *target, struct expression **rp,
+ const char **typediff);
#endif
diff --git a/verify-format.c b/verify-format.c
new file mode 100644
index 00000000..979729bf
--- /dev/null
+++ b/verify-format.c
@@ -0,0 +1,504 @@
+/*
+ * sparse/verify-format.c
+ *
+ * Copyright (C) 2019 Codethink Ltd.
+ * Written by Ben Dooks <ben.dooks@codethink.co.uk>
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ *
+ * Verification code for format-attributes (currently printf)
+ */
+#include <stdlib.h>
+#include <stdarg.h>
+#include <stddef.h>
+#include <stdio.h>
+#include <string.h>
+#include <ctype.h>
+#include <unistd.h>
+#include <fcntl.h>
+#include <limits.h>
+
+#include "evaluate.h"
+#include "lib.h"
+#include "allocate.h"
+#include "parse.h"
+#include "token.h"
+#include "symbol.h"
+#include "target.h"
+#include "expression.h"
+#include "verify-format.h"
+
+struct format_type {
+ const char *format;
+ int (*test)(struct format_type *fmt,
+ struct expression **expr,
+ struct symbol *ctype,
+ struct symbol **target,
+ const char **typediff);
+ struct symbol *data;
+};
+
+struct format_state {
+ struct expression *expr;
+ unsigned int first;
+ unsigned int fmt_index;
+ unsigned int arg_index;
+ unsigned int used_position: 1;
+};
+
+static int printf_fmt_numtype(struct format_type *fmt,
+ struct expression **expr,
+ struct symbol *ctype,
+ struct symbol **target, const char **typediff)
+{
+ struct symbol *type = fmt->data;
+ *target = type;
+ return check_assignment_types(*target, expr, typediff);
+}
+
+static int printf_fmt_string(struct format_type *fmt,
+ struct expression **expr,
+ struct symbol *ctype,
+ struct symbol **target, const char **typediff)
+{
+ *target = &const_string_ctype;
+ return check_assignment_types(*target, expr, typediff);
+}
+
+static int printf_fmt_pointer(struct format_type *fmt,
+ struct expression **expr,
+ struct symbol *ctype,
+ struct symbol **target, const char **typediff)
+{
+ *target = &const_ptr_ctype;
+ return check_assignment_types(*target, expr, typediff);
+}
+
+static int printf_fmt_print_pointer(struct format_type *fmt,
+ struct expression **expr,
+ struct symbol *ctype,
+ struct symbol **target,
+ const char **typediff)
+{
+ int ret;
+ *target = &const_ptr_ctype;
+ ret = check_assignment_types(*target, expr, typediff);
+ if (ret == 0) {
+ /* if just printing, ignore address-space mismatches */
+ if (strcmp(*typediff, "different address spaces") == 0)
+ ret = 1;
+ }
+ return ret;
+}
+
+static struct format_type printf_fmt_ptr_ref = {
+ .format = "p",
+ .test = printf_fmt_pointer,
+};
+
+static struct expression *get_nth_expression(struct expression_list *args, int nr)
+{
+ return ptr_list_nth_entry((struct ptr_list *)args, nr);
+}
+
+static int is_float_spec(char t)
+{
+ return t == 'f' || t == 'g' || t == 'F' || t == 'G';
+}
+
+static struct format_type *parse_printf_get_fmt(struct format_type *type,
+ const char *msg,
+ const char **msgout)
+{
+ const char *ptr = msg;
+ int szmod=0;
+
+ type->test = NULL;
+ *msgout = ptr;
+
+ if (*ptr == 's') {
+ ptr++;
+ type->test = printf_fmt_string;
+ } else if (*ptr == 'c') {
+ ptr++;
+ type->test = printf_fmt_numtype;
+ type->data = &char_ctype;
+ } else if (*ptr == 'p') {
+ ptr++;
+ type->test = printf_fmt_print_pointer;
+ /* check for pointer being printed as hex explicitly */
+ if (*ptr == 'x' || *ptr == 'X') {
+ ptr++;
+ } else if (isalpha(*ptr)) {
+ /* probably some extra specifiers after %p */
+ ptr++;
+ type->test = printf_fmt_pointer;
+ }
+ } else if (*ptr == 'z') {
+ // todo - we should construct pointer to int/etc //
+
+ ptr++;
+ if (*ptr == 'd' || *ptr == 'i') {
+ ptr++;
+ type->test = printf_fmt_numtype;
+ type->data = ssize_t_ctype;
+ } else if (*ptr == 'u' || *ptr == 'x' || *ptr == 'X' ||
+ *ptr == 'o') {
+ ptr++;
+ type->test = printf_fmt_numtype;
+ type->data = size_t_ctype;
+ }
+ } else {
+ if (*ptr == 'l') {
+ szmod++;
+ ptr++;
+ if (*ptr == 'l') {
+ szmod++;
+ ptr++;
+ }
+ } else {
+ if (*ptr == 'h') { // short/char to int
+ szmod = -1;
+ ptr++;
+ if (*ptr == 'h') // promotion from char
+ ptr++;
+ }
+ if (*ptr == 't') { // ptrdiff_t
+ szmod = 2;
+ ptr++;
+ }
+ if (*ptr == 'j') { // intmax_t
+ szmod = 3;
+ ptr++;
+ }
+ }
+
+ if (*ptr == 'x' || *ptr == 'X' || *ptr == 'u' || *ptr == 'o') {
+ ptr++;
+ type->test = printf_fmt_numtype;
+ switch (szmod) {
+ case -1:
+ type->data = &ushort_ctype;
+ break;
+ case 0:
+ type->data = &uint_ctype;
+ break;
+ case 1:
+ type->data = &ulong_ctype;
+ break;
+ case 2:
+ type->data = &ullong_ctype;
+ break;
+ case 3:
+ type->data = uintmax_ctype;
+ break;
+ default:
+ type->test = NULL;
+ }
+ } else if (*ptr == 'i' || *ptr == 'd') {
+ ptr++;
+ type->test = printf_fmt_numtype;
+ switch (szmod) {
+ case -1:
+ type->data = &short_ctype;
+ break;
+ case 0:
+ type->data = &int_ctype;
+ break;
+ case 1:
+ type->data = &long_ctype;
+ break;
+ case 2:
+ type->data = &llong_ctype;
+ break;
+ case 3:
+ type->data = intmax_ctype;
+ break;
+ default:
+ type->test = NULL;
+ }
+ } else if (*ptr == 'L' && is_float_spec(ptr[1])) {
+ type->test = printf_fmt_numtype;
+ type->data = &ldouble_ctype;
+ ptr += 2;
+ } else if (is_float_spec(*ptr)) {
+ type->test = printf_fmt_numtype;
+ type->data = szmod == 1 ? &ldouble_ctype : &double_ctype;
+ ptr++;
+ } else if (*ptr == 'n') {
+ /* pointer to an de-referenced int/etc */
+
+ /* todo - we should construct pointer to int/etc
+ * also should not have any flags or widths for this
+ */
+ type->test = printf_fmt_pointer;
+ ptr++;
+ }
+ }
+
+ if (type->test == NULL)
+ return NULL;
+
+ *msgout = ptr;
+ return type;
+}
+
+static int is_printf_flag(char ch)
+{
+ return ch == '0' || ch == '+' || ch == '-' || ch == ' ' || ch == '#';
+}
+
+static int printf_check_position(const char **fmt)
+{
+ const char *ptr= *fmt;
+
+ if (!isdigit(*ptr))
+ return -1;
+ while (isdigit(*ptr))
+ ptr++;
+ if (*ptr == '$') {
+ const char *pos = *fmt;
+ *fmt = ptr+1;
+ return strtoul(pos, NULL, 10);
+ }
+ return -1;
+}
+
+static void parse_format_printf_checkpos(struct format_state *state,
+ const char *which)
+{
+ if (state->used_position) {
+ warning(state->expr->pos,
+ "format %d: %s: no position specified",
+ state->arg_index-1, which);
+ }
+}
+
+static int parse_format_printf_argfield(const char **fmtptr,
+ struct format_state *state,
+ struct expression_list *args,
+ int *pos, const char *which)
+{
+ struct expression *expr;
+ struct symbol *ctype;
+ const char *fmt = *fmtptr;
+ int argpos = -1;
+
+ /* check for simple digit-string width/precision specifier first */
+ if (*fmt != '*') {
+ while (isdigit(*fmt))
+ fmt++;
+ *fmtptr = fmt;
+ return 0;
+ }
+
+ fmt++;
+ argpos = printf_check_position(&fmt);
+
+ if (argpos > 0) {
+ argpos += state->first - 1;
+ state->used_position = 1;
+ } else {
+ argpos = (*pos)++;
+ state->arg_index++;
+ parse_format_printf_checkpos(state, which);
+ }
+
+ *fmtptr = fmt;
+ expr = get_nth_expression(args, argpos-1);
+ if (!expr) {
+ warning(state->expr->pos, "%s: no argument at position %d",
+ which, argpos);
+ return 1;
+ }
+
+ /* check the value we got was int/uint type */
+ ctype = expr->ctype;
+ if (ctype) {
+ struct symbol *target = &int_ctype;
+
+ if (ctype != &int_ctype && ctype != &uint_ctype) {
+ warning(expr->pos, "incorrect type for %s argument %d", which, argpos);
+ info(expr->pos, " expected %s", show_typename(target));
+ info(expr->pos, " got %s", show_typename(ctype));
+ }
+ }
+
+ return 0;
+}
+
+/*
+ * printf format parsing code
+ *
+ * this code currently does not:
+ * - check castable types (such as int vs long vs long long)
+ * - validate all arguments specified are also used...
+ */
+static int parse_format_printf(const char **fmtstring,
+ struct format_state *state,
+ struct expression_list *args)
+{
+ struct format_type ftype; /* temp storage for format info */
+ struct format_type *type; /* type found from the parse */
+ struct expression *expr;
+ const char *fmt = *fmtstring; /* pointer to parse position */
+ const char *fmtpost = NULL; /* moved to end of the parsed format */
+ int pos = state->arg_index; /* position of the argument */
+ int error = 0;
+ int ret;
+
+ if (!fmt) {
+ warning(state->expr->pos, "no format string passed");
+ return -1;
+ }
+
+ /* trivial check for %% */
+ fmt++;
+ if (fmt[0] == '%') {
+ *fmtstring = fmt+1;
+ return 0;
+ }
+
+ state->arg_index++;
+ state->fmt_index++;
+
+ ret = printf_check_position(&fmt);
+ if (ret == 0) {
+ /* we got an invalid position argument */
+ error++;
+ } else if (ret < 0) {
+ parse_format_printf_checkpos(state, "position");
+ } else {
+ state->used_position = 1;
+ pos = ret + state->first - 1;
+ }
+
+ /* get rid of any formatting flag bits */
+ while (is_printf_flag(*fmt))
+ fmt++;
+
+ /* now there is the posibility of a width specifier */
+ if (parse_format_printf_argfield(&fmt, state, args, &pos, "width"))
+ error++;
+
+ /* now we might have the precision specifier */
+ if (*fmt == '.') {
+ fmt++;
+ if (parse_format_printf_argfield(&fmt, state, args, &pos, "position"))
+ error++;
+ }
+
+ type = parse_printf_get_fmt(&ftype, fmt, &fmtpost);
+
+ if (!type && fmt[0] == 'p')
+ type = &printf_fmt_ptr_ref; /* probably some extension */
+
+ if (type) {
+ struct symbol *ctype, *target = NULL;
+ const char *typediff = "different types";
+ int ret;
+
+ *fmtstring = fmtpost;
+ expr = get_nth_expression(args, pos-1);
+ if (!expr) {
+ /* no argument, but otherwise valid argument string */
+ warning(state->expr->pos, "no argument at position '%d'", pos);
+ return 0;
+ }
+
+ ctype = expr->ctype;
+ if (!ctype)
+ return -3;
+
+ ret = type->test(type, &expr, ctype, &target, &typediff);
+ if (!target) /* shouldn't happen, but catch anyway */
+ return -4;
+
+ if (ret == 0) {
+ warning(expr->pos, "incorrect type in argument %d (%s)", pos, typediff);
+ info(expr->pos, " expected %s", show_typename(target));
+ info(expr->pos, " got %s", show_typename(ctype));
+ }
+ } else {
+ /* try and find the end of this format string by looking for a space*/
+ fmtpost = *fmtstring;
+ while (*fmtpost > ' ')
+ fmtpost++;
+ warning(state->expr->pos, "cannot evaluate type '%.*s'",
+ (int)(fmtpost - *fmtstring), *fmtstring);
+ *fmtstring += 1;
+ return -1;
+ }
+
+ return 1;
+}
+
+/*
+ * attempt to run through a printf format string and work out the types
+ * it specifies. The format is parsed from the __attribute__(format())
+ * in the parser code which stores the positions of the message and arg
+ * start in the ctype.
+ */
+void verify_format_attribute(struct symbol *fn, struct expression_list *args)
+{
+ struct format_state state = { };
+ struct expression *expr;
+ struct expression *init;
+ const char *fmt_string;
+
+ if (!fn->ctype.format.index)
+ return;
+
+ expr = get_nth_expression(args, fn->ctype.format.index-1);
+ if (!expr)
+ return;
+
+ if (expr->type != EXPR_SYMBOL || expr->symbol->ident)
+ return; // not a literal
+ init = expr->symbol->initializer;
+ if (!init || init->type != EXPR_STRING)
+ return; // not a string
+ fmt_string = init->string->data;
+
+ state.expr = expr;
+ state.first = fn->ctype.format.first;
+ state.arg_index = fn->ctype.format.first;
+
+ if (!fmt_string) {
+ warning(expr->pos, "not a format string?");
+ } else {
+ const char *string = fmt_string;
+ int fail = 0;
+
+ while (string[0]) {
+ if (string[0] != '%') {
+ /* strip anything before the '%' */
+ string++;
+ continue;
+ }
+
+ if (parse_format_printf(&string, &state, args) < 0)
+ fail++;
+ }
+
+ if (fail > 0)
+ /* format string may have '\n' etc embedded in it */
+ warning(expr->pos, "cannot evaluate format string");
+ }
+}
diff --git a/verify-format.h b/verify-format.h
new file mode 100644
index 00000000..4a7ef79d
--- /dev/null
+++ b/verify-format.h
@@ -0,0 +1,6 @@
+#ifndef VERIFY_FORMAT_H
+#define VERIFY_FORMAT_H
+
+void verify_format_attribute(struct symbol *fn, struct expression_list *args);
+
+#endif
--
2.37.2.352.g3c44437643
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH RESEND 4/4] tests: add varargs printf format tests
2025-10-16 11:04 Add printf/scanf -Wformat checking Ben Dooks
` (2 preceding siblings ...)
2025-10-16 11:04 ` [PATCH RESEND 3/4] evaluate: check variadic argument types against formatting info Ben Dooks
@ 2025-10-16 11:04 ` Ben Dooks
2025-10-16 17:12 ` Add printf/scanf -Wformat checking Linus Torvalds
4 siblings, 0 replies; 9+ messages in thread
From: Ben Dooks @ 2025-10-16 11:04 UTC (permalink / raw)
To: linux-sparse, Luc Van Oostenryck, Chris Li; +Cc: Ben Dooks
Add some tests for the new printf format checking code.
Note, these do not all pass yet.
Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
---
validation/varargs-format-addrspace1.c | 36 ++++++++
validation/varargs-format-bad.c | 18 ++++
validation/varargs-format-checking.c | 21 +++++
validation/varargs-format-position.c | 32 +++++++
validation/varargs-format-prefix.c | 19 ++++
validation/varargs-format-tests.c | 55 ++++++++++++
validation/varargs-type-formattest.c | 117 +++++++++++++++++++++++++
7 files changed, 298 insertions(+)
create mode 100644 validation/varargs-format-addrspace1.c
create mode 100644 validation/varargs-format-bad.c
create mode 100644 validation/varargs-format-checking.c
create mode 100644 validation/varargs-format-position.c
create mode 100644 validation/varargs-format-prefix.c
create mode 100644 validation/varargs-format-tests.c
create mode 100644 validation/varargs-type-formattest.c
diff --git a/validation/varargs-format-addrspace1.c b/validation/varargs-format-addrspace1.c
new file mode 100644
index 00000000..3370ac67
--- /dev/null
+++ b/validation/varargs-format-addrspace1.c
@@ -0,0 +1,36 @@
+
+extern int variadic(char *msg, ...) __attribute__((format (printf, 1, 2)));
+extern int variadic2(char *msg, int , ...) __attribute__((format (printf, 1, 3)));
+extern int variadic3(int, char *msg, ...) __attribute__((format (printf, 2, 3)));
+
+static void test(void) {
+ void __attribute__((noderef, address_space(1))) *a;
+ void *b;
+
+ variadic("%s\n", a);
+ variadic("%s\n", b);
+ variadic("%s %s\n", b, a);
+ variadic2("%s %s\n", 1, b, a);
+ variadic3(1, "%s %s\n", b, a);
+ variadic3(1, "%s %p\n", b, a);
+}
+
+/*
+ * check-name: variadic formatting test with address-space to %s
+ * check-command: sparse -Wformat $file
+ *
+ * check-error-start
+varargs-format-addrspace1.c:10:26: warning: incorrect type in argument 2 (different address spaces)
+varargs-format-addrspace1.c:10:26: expected char const *
+varargs-format-addrspace1.c:10:26: got void [noderef] <asn:1> *a
+varargs-format-addrspace1.c:12:32: warning: incorrect type in argument 3 (different address spaces)
+varargs-format-addrspace1.c:12:32: expected char const *
+varargs-format-addrspace1.c:12:32: got void [noderef] <asn:1> *a
+varargs-format-addrspace1.c:13:36: warning: incorrect type in argument 4 (different address spaces)
+varargs-format-addrspace1.c:13:36: expected char const *
+varargs-format-addrspace1.c:13:36: got void [noderef] <asn:1> *a
+varargs-format-addrspace1.c:14:36: warning: incorrect type in argument 4 (different address spaces)
+varargs-format-addrspace1.c:14:36: expected char const *
+varargs-format-addrspace1.c:14:36: got void [noderef] <asn:1> *a
+ * check-error-end
+ */
diff --git a/validation/varargs-format-bad.c b/validation/varargs-format-bad.c
new file mode 100644
index 00000000..82ae357c
--- /dev/null
+++ b/validation/varargs-format-bad.c
@@ -0,0 +1,18 @@
+
+extern int variadic(char *msg, ...) __attribute__((format (printf, 0, 0)));
+extern int variadic2(char *msg, int , ...) __attribute__((format (printf, 2, 2)));
+extern int variadic3(char *msg, int , ...) __attribute__((format (printf, 2, 1)));
+
+static void test(void) {
+}
+
+/*
+ * check-name: variadic formatting test with bad formatting parameters
+ * check-command: sparse -Wformat $file
+ *
+ * check-error-start
+varargs-format-bad.c:2:73: warning: bad format positions
+varargs-format-bad.c:3:80: warning: bad format positions
+varargs-format-bad.c:4:80: warning: format cannot be after va_args
+* check-error-end
+ */
diff --git a/validation/varargs-format-checking.c b/validation/varargs-format-checking.c
new file mode 100644
index 00000000..9f3e5ac2
--- /dev/null
+++ b/validation/varargs-format-checking.c
@@ -0,0 +1,21 @@
+
+extern void pf(char *msg, ...) __attribute__((format (printf, 1, 2)));
+
+static void test(void) {
+ pf("%u %lu %llu\n", 1U, 1UL, 1ULL);
+ pf("%d %ld %lld\n", 1, 1L, 1LL);
+ pf("%x %lx %llx\n", 1U, 1UL, 1ULL);
+ pf("%d %ld %lld\n", 1, 1L, 1L);
+}
+
+/*
+ * check-name: variadic formatting test type checking
+ * check-command: sparse -Wformat $file
+ * check-known-to-fail
+ *
+ * check-error-start
+varargs-format-checking.c:8:36: warning: incorrect type in argument 4 (different types)
+varargs-format-checking.c:8:36: expected long long
+varargs-format-checking.c:8:36: got long
+ * check-error-end
+ */
diff --git a/validation/varargs-format-position.c b/validation/varargs-format-position.c
new file mode 100644
index 00000000..88a4dbc2
--- /dev/null
+++ b/validation/varargs-format-position.c
@@ -0,0 +1,32 @@
+
+extern void pf(char *msg, ...) __attribute__((format (printf, 1, 2)));
+
+static void test(void) {
+ pf("%2$d %u\n", 1U, 1L);
+ pf("%3$d %2$u\n", 1U, 1);
+ pf("%1$d %2$d\n", 1L, 1);
+}
+
+/*
+ * check-name: variadic formatting test position checking
+ * check-command: sparse -Wformat $file
+ * check-known-to-fail
+ *
+ * check-error-start
+varargs-format-position.c:5:29: warning: incorrect type in argument 3 (different types)
+varargs-format-position.c:5:29: expected int
+varargs-format-position.c:5:29: got long
+varargs-format-position.c:5:12: warning: format 3: position: no position specified
+varargs-format-position.c:5:29: warning: incorrect type in argument 3 (different types)
+varargs-format-position.c:5:29: expected unsigned int
+varargs-format-position.c:5:29: got long
+varargs-format-position.c:6:12: warning: no argument at position '4'
+varargs-format-position.c:6:31: warning: incorrect type in argument 3 (different types)
+varargs-format-position.c:6:31: expected unsigned int
+varargs-format-position.c:6:31: got int
+varargs-format-position.c:7:27: warning: incorrect type in argument 2 (different types)
+varargs-format-position.c:7:27: expected int
+varargs-format-position.c:7:27: got long
+ * check-error-end
+ *
+ */
diff --git a/validation/varargs-format-prefix.c b/validation/varargs-format-prefix.c
new file mode 100644
index 00000000..8e2456e6
--- /dev/null
+++ b/validation/varargs-format-prefix.c
@@ -0,0 +1,19 @@
+
+extern int __attribute__((format (printf, 1, 2))) variadic(char *msg, ...);
+
+static int test(void) {
+ void __attribute__((noderef, address_space(1))) *a;
+
+ variadic("%s\n", a);
+}
+
+/*
+ * check-name: variadic formatting test prefix based __attribute__
+ * check-command: sparse -Wformat $file
+ *
+ * check-error-start
+varargs-format-prefix.c:7:26: warning: incorrect type in argument 2 (different address spaces)
+varargs-format-prefix.c:7:26: expected char const *
+varargs-format-prefix.c:7:26: got void [noderef] <asn:1> *a
+ * check-error-end
+ */
diff --git a/validation/varargs-format-tests.c b/validation/varargs-format-tests.c
new file mode 100644
index 00000000..659bbe94
--- /dev/null
+++ b/validation/varargs-format-tests.c
@@ -0,0 +1,55 @@
+
+extern void pf(char *msg, ...) __attribute__((format (printf, 1, 2)));
+
+static int test(void)
+{
+ pf("%*d\n", 5, 10); /* value 10, print width is 5 */
+ pf("%2$*1$d\n", 5, 10); /* value 10, print width is 5 */
+ pf("%3$*2$d\n", 1, 5, 10); /* ok, skipping the '1' */
+ pf("%3$-*2$d\n", 1, 5, 10); /* ok, skipping the '1' */
+ pf("%3$*2$-d\n", 1, 5, 10); /* bad, the "-" shouldn't be before the 'd' */
+ pf("%3$ *2$d\n", 1, 5, 10); /* ok, skipping the '1' */
+ pf("%3$+*2$d\n", 1, 5, 10); /* ok, skipping the '1' */
+ pf("%3$0+*2$d\n", 1, 5, 10); /* ok, skipping the '1' */
+ pf("%3$+0*2$d\n", 1, 5, 10); /* ok, skipping the '1' */
+ pf("%3$+#*2$d\n", 1, 5, 10); /* ok, skipping the '1' */
+ pf("%3$+#*2$.5d\n", 1, 5, 10); /* ok, skipping the '1' */
+
+ /* go with some precision as well as width strings */
+ pf("%2$+*1$.6d\n", 5, 10); /* ok */
+ pf("%2$+*1$.*3$d\n", 5, 10, 6); /* ok */
+ pf("%2$+*3$.*1$d\n", 6, 10, 5); /* ok */
+ pf("%2$+*1$.*d\n", 5, 10, 6); /* not ok */
+
+ pf("%s", "msg");
+ return 0;
+}
+
+static void test2(int x, int y, const void *p)
+{
+ pf("%02x%02x %8p\n", x, y, p);
+}
+
+static inline void fn(int x) { pf("%08x\n", x); }
+static void test3(int x)
+{
+ fn;
+ fn(x);
+}
+
+static void test4(int i, unsigned int u)
+{
+ pf("%d\n", i);
+ pf("%x\n", u);
+}
+
+/*
+ * check-name: variadic formatting tests for width/precisions
+ * check-command: sparse -Wformat $file
+ *
+ * check-error-start
+varargs-format-tests.c:10:12: warning: cannot evaluate type '%3$*2$-d'
+varargs-format-tests.c:10:12: warning: cannot evaluate format string
+varargs-format-tests.c:22:12: warning: format 3: position: no position specified
+ * check-error-end
+ */
diff --git a/validation/varargs-type-formattest.c b/validation/varargs-type-formattest.c
new file mode 100644
index 00000000..f01c6d89
--- /dev/null
+++ b/validation/varargs-type-formattest.c
@@ -0,0 +1,117 @@
+
+extern void pf1(char *msg, ...) __attribute__((format (printf, 1, 2)));
+extern void pf2(int m, char *msg, ...) __attribute__((format (printf, 2, 3)));
+
+/* run all the tests with both of these printf formatted types */
+#define pf(x...) do { pf1(x); pf2(1, x); } while(0);
+
+static void test(void) {
+ /* first two are valid */
+ pf("%*d", 5, 10); /* value 10, print width is 5 */
+ pf("%2$*1$d", 5, 10); /* value 10, print width is 5 */
+ pf("%2$*3$d", 5, 10); /* value 10, print width is ?? */
+
+ pf("%*d", 5, 10); /* value 10, print width is 5 */
+ pf("%*d", 5, 10L); /* value 10, print width is 5 (bad type) */
+ pf("%*d", 5UL, 10L); /* value 10, print width is 5 (bad type) */
+
+ pf("%3$*2$d", 1, 5, 10); /* ok, skipping the '1' */
+ pf("%3$*2$d", 1, 5, 10L); /* bad print type */
+ pf("%2$*3$d", 1UL, 10, 5); /* ok, try with swapping width/val */
+ pf("%2$*3$d", 1UL, 10L, 5); /* bad, try with swapping width/val */
+
+ /* and now try with precision specifiers */
+
+ pf("%*.6d", 5, 10); /* value 10, print width is 5 */
+ pf("%*.6d", 5, 10L); /* value 10, print width is 5 (bad type) */
+ pf("%*.6d", 5UL, 10L); /* value 10, print width is 5 (bad type) */
+
+ pf("%*.*d", 5, 6, 10); /* value 10, print width is 5 */
+ pf("%*.*d", 5, 6, 10L); /* value 10, print width is 5 (bad type) */
+ pf("%*.*d", 5UL, 6, 10L); /* value 10, print width is 5 (bad type) */
+ pf("%*.*d", 5, 6UL, 10); /* value 10, print width is 5 (bad type) */
+}
+
+/*
+ * check-name: variadic formatting test position checking types
+ * check-command: sparse -Wformat $file
+ * check-known-to-fail
+ *
+ * check-error-start
+varargs-type-formattest.c:12:9: warning: width: no argument at position 4
+varargs-type-formattest.c:12:9: warning: width: no argument at position 5
+varargs-type-formattest.c:15:9: warning: incorrect type in argument 3 (different types)
+varargs-type-formattest.c:15:9: expected int
+varargs-type-formattest.c:15:9: got long
+varargs-type-formattest.c:15:9: warning: incorrect type in argument 4 (different types)
+varargs-type-formattest.c:15:9: expected int
+varargs-type-formattest.c:15:9: got long
+varargs-type-formattest.c:16:9: warning: incorrect type for width argument 2
+varargs-type-formattest.c:16:9: expected int
+varargs-type-formattest.c:16:9: got unsigned long
+varargs-type-formattest.c:16:9: warning: incorrect type in argument 3 (different types)
+varargs-type-formattest.c:16:9: expected int
+varargs-type-formattest.c:16:9: got long
+varargs-type-formattest.c:16:9: warning: incorrect type for width argument 3
+varargs-type-formattest.c:16:9: expected int
+varargs-type-formattest.c:16:9: got unsigned long
+varargs-type-formattest.c:16:9: warning: incorrect type in argument 4 (different types)
+varargs-type-formattest.c:16:9: expected int
+varargs-type-formattest.c:16:9: got long
+varargs-type-formattest.c:19:9: warning: incorrect type in argument 4 (different types)
+varargs-type-formattest.c:19:9: expected int
+varargs-type-formattest.c:19:9: got long
+varargs-type-formattest.c:19:9: warning: incorrect type in argument 5 (different types)
+varargs-type-formattest.c:19:9: expected int
+varargs-type-formattest.c:19:9: got long
+varargs-type-formattest.c:21:9: warning: incorrect type in argument 3 (different types)
+varargs-type-formattest.c:21:9: expected int
+varargs-type-formattest.c:21:9: got long
+varargs-type-formattest.c:21:9: warning: incorrect type in argument 4 (different types)
+varargs-type-formattest.c:21:9: expected int
+varargs-type-formattest.c:21:9: got long
+varargs-type-formattest.c:26:9: warning: incorrect type in argument 3 (different types)
+varargs-type-formattest.c:26:9: expected int
+varargs-type-formattest.c:26:9: got long
+varargs-type-formattest.c:26:9: warning: incorrect type in argument 4 (different types)
+varargs-type-formattest.c:26:9: expected int
+varargs-type-formattest.c:26:9: got long
+varargs-type-formattest.c:27:9: warning: incorrect type for width argument 2
+varargs-type-formattest.c:27:9: expected int
+varargs-type-formattest.c:27:9: got unsigned long
+varargs-type-formattest.c:27:9: warning: incorrect type in argument 3 (different types)
+varargs-type-formattest.c:27:9: expected int
+varargs-type-formattest.c:27:9: got long
+varargs-type-formattest.c:27:9: warning: incorrect type for width argument 3
+varargs-type-formattest.c:27:9: expected int
+varargs-type-formattest.c:27:9: got unsigned long
+varargs-type-formattest.c:27:9: warning: incorrect type in argument 4 (different types)
+varargs-type-formattest.c:27:9: expected int
+varargs-type-formattest.c:27:9: got long
+varargs-type-formattest.c:30:9: warning: incorrect type in argument 4 (different types)
+varargs-type-formattest.c:30:9: expected int
+varargs-type-formattest.c:30:9: got long
+varargs-type-formattest.c:30:9: warning: incorrect type in argument 5 (different types)
+varargs-type-formattest.c:30:9: expected int
+varargs-type-formattest.c:30:9: got long
+varargs-type-formattest.c:31:9: warning: incorrect type for width argument 2
+varargs-type-formattest.c:31:9: expected int
+varargs-type-formattest.c:31:9: got unsigned long
+varargs-type-formattest.c:31:9: warning: incorrect type in argument 4 (different types)
+varargs-type-formattest.c:31:9: expected int
+varargs-type-formattest.c:31:9: got long
+varargs-type-formattest.c:31:9: warning: incorrect type for width argument 3
+varargs-type-formattest.c:31:9: expected int
+varargs-type-formattest.c:31:9: got unsigned long
+varargs-type-formattest.c:31:9: warning: incorrect type in argument 5 (different types)
+varargs-type-formattest.c:31:9: expected int
+varargs-type-formattest.c:31:9: got long
+varargs-type-formattest.c:32:9: warning: incorrect type for position argument 3
+varargs-type-formattest.c:32:9: expected int
+varargs-type-formattest.c:32:9: got unsigned long
+varargs-type-formattest.c:32:9: warning: incorrect type for position argument 4
+varargs-type-formattest.c:32:9: expected int
+varargs-type-formattest.c:32:9: got unsigned long
+ * check-error-end
+ *
+ */
--
2.37.2.352.g3c44437643
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: Add printf/scanf -Wformat checking
2025-10-16 11:04 Add printf/scanf -Wformat checking Ben Dooks
` (3 preceding siblings ...)
2025-10-16 11:04 ` [PATCH RESEND 4/4] tests: add varargs printf format tests Ben Dooks
@ 2025-10-16 17:12 ` Linus Torvalds
2025-10-17 7:47 ` Ben Dooks
2025-10-20 10:01 ` Ben Dooks
4 siblings, 2 replies; 9+ messages in thread
From: Linus Torvalds @ 2025-10-16 17:12 UTC (permalink / raw)
To: Ben Dooks; +Cc: linux-sparse, Luc Van Oostenryck, Chris Li
On Thu, Oct 16, 2025 at 4:04 AM Ben Dooks <ben.dooks@codethink.co.uk> wrote:
>
> [re-send as I can't type sparse and messages didn't get to the list]
You may need to re-send again after fixing your mailer setup, because
while it made it to the list, I suspect a lot of people didn't
actually get it, because it fails DKIM checks and that then tends to
mean that it gets marked as spam.
At least it did for me - but I check my spam folder daily, so I noticed.
The reason it fails DKIM is that your codethink DKIM setup is scrogged.
The DKIM message headers look like this:
DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed;
d=codethink.co.uk; s=imap4-20230908;
so far, all good. You have DKIM, it uses relaxed/relaxed, all great.
But then it continues with the list of headers that are hashed:
h=Sender:Content-Transfer-Encoding:
MIME-Version:Message-Id:Date:Subject:To:From:Reply-To:Cc:
All still good so far, and those are the right headers. But they are
pretty much the *only* right headers, because the list of headers then
goes on with
Content-Type:
Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender:
Resent-To:Resent-Cc:Resent-Message-ID:In-Reply-To:References:List-Id:
List-Help:List-Unsubscribe:List-Subscribe:List-Post:List-Owner:List-Archive;
and that's just bogus.
Having email list headers in the DKIM header hashing means that any
well-behaved mailing list that adds its own subscription headers will
now result in a different hash, and DKIM will fail, and anybody who
reads the list with a sane MUA will not see the emails you sent.
So by all means keep the "In-Reply-To" and "References" headers in the
DKIM list, but get rid of all the "List-Help" etc headers, because
they are actively wrong and bad.
I've seen this before, and I have been told that there's some Debian
DKIM help page for DKIM that lists those headers. I don't know the
details, but that help page is really actively wrong and should be
fixed.
Typically, you should really only list the headers you really care
about: things that *matter*. Things that make it clear that it was
from you, and what the subject and thread was.
Other headers shouldn't be hashed, exactly because there are valid
reasons why those headers may then be added by intermediate mailers).
Anyway, what this results in is that when I look at the headers, I see this:
ARC-Authentication-Results: i=1; smtp.subspace.kernel.org;
dmarc=pass (p=reject dis=none) header.from=codethink.co.uk; spf=pass
smtp.mailfrom=codethink.com; dkim=pass (2048-bit key)
header.d=codethink.co.uk header.i=@codethink.co.uk header.b=x51xpGj+;
arc=none smtp.client-ip=188.40.203.114
Authentication-Results: smtp.subspace.kernel.org; dmarc=pass
(p=reject dis=none) header.from=codethink.co.uk
ie the list receiver is happy, because at the time the list gets the
email directly from you, the DKIM hash matches, and so DMARC rules at
that time say "all good".
But then when the list re-sends the email out to list subscribers,
they will get something like this:
Authentication-Results: mx.google.com;
dkim=fail header.i=@codethink.co.uk header.s=imap4-20230908
header.b=x51xpGj+;
because the list expander has added headers
List-Id: <linux-sparse.vger.kernel.org>
List-Subscribe: <mailto:linux-sparse+subscribe@vger.kernel.org>
List-Unsubscribe: <mailto:linux-sparse+unsubscribe@vger.kernel.org>
(and possibly others) so now the header hash no longer matches,
because you've told dkim to hash those headers. They obviously weren't
there in your original email, so now the hash that codethink computed
is no longer matching the headers that the list sends out.
And this is a well-behaved mailing list that does what mailing lists
*should* do, in adding those headers. It's really your DKIM that is
actively wrong.
If you remember where you got the bad list of headers from, maybe you
can report this problem upstream too?
Linus
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: Add printf/scanf -Wformat checking
2025-10-16 17:12 ` Add printf/scanf -Wformat checking Linus Torvalds
@ 2025-10-17 7:47 ` Ben Dooks
2025-10-20 10:01 ` Ben Dooks
1 sibling, 0 replies; 9+ messages in thread
From: Ben Dooks @ 2025-10-17 7:47 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linux-sparse
On 16/10/2025 18:12, Linus Torvalds wrote:
> On Thu, Oct 16, 2025 at 4:04 AM Ben Dooks <ben.dooks@codethink.co.uk> wrote:
>>
>> [re-send as I can't type sparse and messages didn't get to the list]
>
> You may need to re-send again after fixing your mailer setup, because
> while it made it to the list, I suspect a lot of people didn't
> actually get it, because it fails DKIM checks and that then tends to
> mean that it gets marked as spam.
>
> At least it did for me - but I check my spam folder daily, so I noticed.
>
> The reason it fails DKIM is that your codethink DKIM setup is scrogged.
Thanks, passed this on to our IT support.
--
Ben Dooks http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius
https://www.codethink.co.uk/privacy.html
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Add printf/scanf -Wformat checking
2025-10-16 17:12 ` Add printf/scanf -Wformat checking Linus Torvalds
2025-10-17 7:47 ` Ben Dooks
@ 2025-10-20 10:01 ` Ben Dooks
1 sibling, 0 replies; 9+ messages in thread
From: Ben Dooks @ 2025-10-20 10:01 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linux-sparse, Luc Van Oostenryck, Chris Li
On 16/10/2025 18:12, Linus Torvalds wrote:
> On Thu, Oct 16, 2025 at 4:04 AM Ben Dooks <ben.dooks@codethink.co.uk> wrote:
>>
>> [re-send as I can't type sparse and messages didn't get to the list]
>
> You may need to re-send again after fixing your mailer setup, because
> while it made it to the list, I suspect a lot of people didn't
> actually get it, because it fails DKIM checks and that then tends to
> mean that it gets marked as spam.
>
> At least it did for me - but I check my spam folder daily, so I noticed.
>
> The reason it fails DKIM is that your codethink DKIM setup is scrogged.
>
> The DKIM message headers look like this:
>
> DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed;
> d=codethink.co.uk; s=imap4-20230908;
>
> so far, all good. You have DKIM, it uses relaxed/relaxed, all great.
> But then it continues with the list of headers that are hashed:
>
> h=Sender:Content-Transfer-Encoding:
> MIME-Version:Message-Id:Date:Subject:To:From:Reply-To:Cc:
>
> All still good so far, and those are the right headers. But they are
> pretty much the *only* right headers, because the list of headers then
> goes on with
>
> Content-Type:
> Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender:
> Resent-To:Resent-Cc:Resent-Message-ID:In-Reply-To:References:List-Id:
> List-Help:List-Unsubscribe:List-Subscribe:List-Post:List-Owner:List-Archive;
>
> and that's just bogus.
>
> Having email list headers in the DKIM header hashing means that any
> well-behaved mailing list that adds its own subscription headers will
> now result in a different hash, and DKIM will fail, and anybody who
> reads the list with a sane MUA will not see the emails you sent.
>
> So by all means keep the "In-Reply-To" and "References" headers in the
> DKIM list, but get rid of all the "List-Help" etc headers, because
> they are actively wrong and bad.
>
> I've seen this before, and I have been told that there's some Debian
> DKIM help page for DKIM that lists those headers. I don't know the
> details, but that help page is really actively wrong and should be
> fixed.
>
> Typically, you should really only list the headers you really care
> about: things that *matter*. Things that make it clear that it was
> from you, and what the subject and thread was.
>
> Other headers shouldn't be hashed, exactly because there are valid
> reasons why those headers may then be added by intermediate mailers).
>
> Anyway, what this results in is that when I look at the headers, I see this:
>
> ARC-Authentication-Results: i=1; smtp.subspace.kernel.org;
> dmarc=pass (p=reject dis=none) header.from=codethink.co.uk; spf=pass
> smtp.mailfrom=codethink.com; dkim=pass (2048-bit key)
> header.d=codethink.co.uk header.i=@codethink.co.uk header.b=x51xpGj+;
> arc=none smtp.client-ip=188.40.203.114
> Authentication-Results: smtp.subspace.kernel.org; dmarc=pass
> (p=reject dis=none) header.from=codethink.co.uk
>
> ie the list receiver is happy, because at the time the list gets the
> email directly from you, the DKIM hash matches, and so DMARC rules at
> that time say "all good".
>
> But then when the list re-sends the email out to list subscribers,
> they will get something like this:
>
> Authentication-Results: mx.google.com;
> dkim=fail header.i=@codethink.co.uk header.s=imap4-20230908
> header.b=x51xpGj+;
>
> because the list expander has added headers
>
> List-Id: <linux-sparse.vger.kernel.org>
> List-Subscribe: <mailto:linux-sparse+subscribe@vger.kernel.org>
> List-Unsubscribe: <mailto:linux-sparse+unsubscribe@vger.kernel.org>
>
> (and possibly others) so now the header hash no longer matches,
> because you've told dkim to hash those headers. They obviously weren't
> there in your original email, so now the hash that codethink computed
> is no longer matching the headers that the list sends out.
>
> And this is a well-behaved mailing list that does what mailing lists
> *should* do, in adding those headers. It's really your DKIM that is
> actively wrong.
>
> If you remember where you got the bad list of headers from, maybe you
> can report this problem upstream too?
So our operations people dug into this, and it looks to be the default
for Debian and possibly the upstream Debian too. From the ticket:
> We don't explicitly set dkim_sign_headers in our exim.conf, so
according to the docs, this defaults to the macro _DKIM_SIGN_HEADERS
> pulling the source code from the Debian packlage for this, _DKIM_SIGN_HEADERS is set by the macro PDKIM_DEFAULT_SIGN_HEADERS:
>
> #define PDKIM_DEFAULT_SIGN_HEADERS "From:Sender:Reply-To:Subject:Date:"\
> "Message-ID:To:Cc:MIME-Version:Content-Type:"\
> "Content-Transfer-Encoding:Content-ID:"\
> "Content-Description:Resent-Date:Resent-From:"\
> "Resent-Sender:Resent-To:Resent-Cc:"\
> "Resent-Message-ID:In-Reply-To:References:"\
> "List-Id:List-Help:List-Unsubscribe:"\
> "List-Subscribe:List-Post:List-Owner:List-Archive"
>
> looking upstream @ https://code.exim.org/exim/exim.git - ./src/src/miscmods/pdkim/pdkim.h on the master branch
>
> #define PDKIM_DEFAULT_SIGN_HEADERS "From:Sender:Reply-To:Subject:Date:"\
> "Message-ID:To:Cc:MIME-Version:Content-Type:"\
> "Content-Transfer-Encoding:Content-ID:"\
> "Content-Description:Resent-Date:Resent-From:"\
> "Resent-Sender:Resent-To:Resent-Cc:"\
> "Resent-Message-ID:In-Reply-To:References:"\
> "List-Id:List-Help:List-Unsubscribe:"\
> "List-Subscribe:List-Post:List-Owner:List-Archive"
>
> So yeah, this is just the default exim config...
I think this is now fixed, will re-send later.
--
Ben Dooks http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius
https://www.codethink.co.uk/privacy.html
^ permalink raw reply [flat|nested] 9+ messages in thread