| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <20250207-checkpatch-newline-v3-1-20d8774f16ea@invicto.ai>
Date: Fri, 07 Feb 2025 18:18:18 +0000 (UTC)
From: Alban Kurti <kurti@...icto.ai>
To: Andy Whitcroft <apw@...onical.com>, Joe Perches <joe@...ches.com>,
Dwaipayan Ray <dwaipayanray1@...il.com>,
Lukas Bulwahn <lukas.bulwahn@...il.com>, Miguel Ojeda <ojeda@...nel.org>,
Alex Gaynor <alex.gaynor@...il.com>, Boqun Feng <boqun.feng@...il.com>,
Gary Guo <gary@...yguo.net>,
Björn Roy Baron <bjorn3_gh@...tonmail.com>,
Benno Lossin <benno.lossin@...ton.me>,
Andreas Hindborg <a.hindborg@...nel.org>, Alice Ryhl <aliceryhl@...gle.com>,
Trevor Gross <tmgross@...ch.edu>
Cc: Alban Kurti <kurti@...icto.ai>, linux-kernel@...r.kernel.org,
rust-for-linux@...r.kernel.org
Subject: [PATCH v3] checkpatch: add warning for pr_* and dev_* macros
without a trailing newline
Add a new check in scripts/checkpatch.pl to detect usage of pr_(level)
and dev_(level) macros (for both C and Rust) when the string literal
does not end with '\n'. Missing trailing newlines can lead to incomplete
log lines that do not appear properly in dmesg or in console output.
To show an example of this working after applying the patch we can run
the script on the commit that likely motivated this need/issue:
./scripts/checkpatch.pl --strict -g "f431c5c581fa1"
Also, the patch is able to handle correctly if there is a printing call
without a newline which then has a newline printed via pr_cont for
both Rust and C alike. If there is no newline printed and the patch
ends or there is another pr_* call before a newline with pr_cont is
printed it will show a warning. Not implemented for dev_cont because it
is not clear to me if that is used at all.
One false warning that will be generated due to this change is in case
we have a patch that modifies a `pr_* call without a newline` which has a
pr_cont with a newline following it. In this case there will be a
warning but because the patch does not include the following pr_cont it
will warn there is nothing creating a newline. I have modified the
warning to be softer due to this known problem.
I have tested with comments, whitespace, differen orders of pr_* calls
and pr_cont and the only case that I suspect to be a problem is the one
outlined above.
Suggested-by: Miguel Ojeda <ojeda@...nel.org>
Closes: https://github.com/Rust-for-Linux/linux/issues/1140
Signed-off-by: Alban Kurti <kurti@...icto.ai>
---
Changes since v2:
- Added pending_log functionality to account for pr_cont usage with a
newline
- Better formatted cover and patch
- Link to v2: https://lore.kernel.org/all/20250205120540.387447-1-kurti@invicto.ai/
---
scripts/checkpatch.pl | 96 +++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 96 insertions(+)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 9eed3683ad76caffbbb2418e5dbea7551d374406..e7494920dea47986af0ba668145a19fcc21a3c9b 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -77,6 +77,8 @@ my ${CONFIG_} = "CONFIG_";
my %maybe_linker_symbol; # for externs in c exceptions, when seen in *vmlinux.lds.h
+my $pending_log = undef;
+
sub help {
my ($exitcode) = @_;
@@ -3898,6 +3900,91 @@ sub process {
# check we are in a valid source file C or perl if not then ignore this hunk
next if ($realfile !~ /\.(h|c|pl|dtsi|dts)$/);
+# check for pr_* and dev_* logs without a newline for C and Rust files to avoid missing log messages
+ my $pr_cont_pattern = qr{
+ \b
+ pr_cont!?
+ \s*
+ \(
+ \s*
+ "([^"]*)"
+ [^)]*
+ \)
+ }x;
+ my $log_macro_pattern = qr{
+ \b
+ (
+ pr_(?:emerg|alert|crit|err|warn|notice|info|debug)
+ | dev_(?:emerg|alert|crit|err|warn|notice|info|dbg)
+ )
+ (!?)
+ \s*
+ \(
+ \s*
+ "([^"]*)"
+ }x;
+
+ if ($realfile =~ /\.(?:c|h|rs)$/) {
+ if ($rawline =~ /^\+/) {
+ my $cleanline = $rawline;
+ $cleanline =~ s/^[+\s]+//;
+ $cleanline =~ s/\r?$//;
+ $cleanline =~ s{/\*.*?\*/}{}g;
+ $cleanline =~ s{//.*}{}g;
+
+ if ($pending_log) {
+ if ($cleanline =~ /$pr_cont_pattern/) {
+ my $cont_string_arg = $1;
+ if ($cont_string_arg =~ /\\n$/) {
+ $pending_log = undef;
+ }
+ } elsif ($cleanline =~ /$log_macro_pattern/) {
+ WARN($pending_log->{lang} . "_LOG_NO_NEWLINE",
+ "Possible usage of $pending_log->{macro_call} without a trailing newline.\n" .
+ $pending_log->{herecurr});
+
+ $pending_log = undef;
+
+ my $macro_call = $1;
+ my $maybe_excl = $2;
+ my $string_arg = $3;
+ $string_arg =~ s/\s+$//;
+
+ if ($realfile =~ /\.rs$/ && $maybe_excl ne '!') {
+ return;
+ }
+
+ if ($string_arg !~ /\\n$/ && $string_arg !~ /\n$/) {
+ $pending_log = {
+ macro_call => $macro_call,
+ herecurr => $herecurr,
+ lang => ($realfile =~ /\.rs$/) ? "Rust" : "C",
+ };
+ }
+ }
+ } else {
+ if ($cleanline =~ /$log_macro_pattern/) {
+ my $macro_call = $1;
+ my $maybe_excl = $2;
+ my $string_arg = $3;
+ $string_arg =~ s/\s+$//;
+
+ if ($realfile =~ /\.rs$/ && $maybe_excl ne '!') {
+ return;
+ }
+
+ if ($string_arg !~ /\\n$/ && $string_arg !~ /\n$/) {
+ $pending_log = {
+ macro_call => $macro_call,
+ herecurr => $herecurr,
+ lang => ($realfile =~ /\.rs$/) ? "Rust" : "C",
+ };
+ }
+ }
+ }
+ }
+ }
+
# at the beginning of a line any tabs must come first and anything
# more than $tabsize must use tabs.
if ($rawline =~ /^\+\s* \t\s*\S/ ||
@@ -7678,6 +7765,15 @@ sub process {
}
}
+# pending log means a pr_* without an ending newline has not
+# been followed by a pr_cont call with a newline at the end
+ if ($pending_log) {
+ WARN($pending_log->{lang} . "_LOG_NO_NEWLINE",
+ "Usage of $pending_log->{macro_call} without a trailing newline.\n" .
+ $pending_log->{herecurr});
+ $pending_log = undef;
+ }
+
# If we have no input at all, then there is nothing to report on
# so just keep quiet.
if ($#rawlines == -1) {
---
base-commit: ceff0757f5dafb5be5205988171809c877b1d3e3
change-id: 20250207-checkpatch-newline-09b515077b71
Best regards,
--
Alban Kurti <kurti@...icto.ai>
Powered by blists - more mailing lists