linux-kernel - Re: [PATCH 1/1 RESEND] checkpatch: add warning for pr_* and dev

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <m2seou3d54.fsf@posteo.net>
Date: Mon, 03 Feb 2025 21:57:11 +0000
From: Charalampos Mitrodimas <charmitro@...teo.net>
To: albankurti <kurti@...icto.ai>
Cc: Joe Perches <joe@...ches.com>,  Andrew Morton
 <akpm@...ux-foundation.org>,  Miguel Ojeda <ojeda@...nel.org>,
  linux-kernel@...r.kernel.org,  rust-for-linux@...r.kernel.org
Subject: Re: [PATCH 1/1 RESEND] checkpatch: add warning for pr_* and dev_*
 macros without a trailing newline

albankurti <kurti@...icto.ai> writes:

> 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 f431c5c581fa176f608ba3fdebb3c1051bad5774
>
> Suggested-by: Miguel Ojeda <ojeda@...nel.org>
> Link: https://github.com/Rust-for-Linux/linux/issues/1140
> Signed-off-by: albankurti <kurti@...icto.ai>
> ---
>  scripts/checkpatch.pl | 48 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 48 insertions(+)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 9eed3683ad76..86ff666f5017 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -3888,6 +3888,54 @@ sub process {
>  			}
>  		}
>  
> +# check for pr_* and dev_* logs without a newline for C and Rust files to avoid missing log messages
> +    my $macro_pattern_c = qr{
> +        \b
> +        (
> +            pr_(?:emerg|alert|crit|err|warn|notice|info|debug)
> +          | dev_(?:emerg|alert|crit|err|warn|notice|info|dbg)
> +        )
> +        \(\s*
> +        "((?:\\.|[^"])*)"
> +        \s*
> +        (?:,|\))
> +    }x;
> +
> +    my $macro_pattern_rust = qr{
> +        \b
> +        (
> +            pr_(?:emerg|alert|crit|err|warn|notice|info|debug)
> +          | dev_(?:emerg|alert|crit|err|warn|notice|info|dbg)
> +        )
> +        !\s*
> +        \(\s*
> +        "((?:\\.|[^"])*)"
> +        \s*
> +        (?:,|\))
> +    }x;
> +
> +    if ($realfile =~ /\.(?:c|h|rs)$/) {
> +      my $is_rust = ($realfile =~ /\.rs$/);
> +      my $macro_pattern = $is_rust ? $macro_pattern_rust : $macro_pattern_c;
> +
> +      if ($line =~ /^\+/) {
> +        my $cleanline = $line;
> +        $cleanline =~ s/^[+\s]+//;
> +        $cleanline =~ s/\r?$//;
> +
> +        if ($cleanline =~ /$macro_pattern/) {
> +          my $macro_call = $1;
> +          my $string_arg = $2;
> +
> +          if ($string_arg !~ /\\n$/ && $string_arg !~ /\n$/) {
> +            my $lang = $is_rust ? "Rust" : "C";
> +            WARN("${lang}_LOG_NO_NEWLINE",
> +                 "Usage of $macro_call without a trailing newline. Consider adding '\\n'.\n" . $herecurr);
> +          }

Having a comment breaks this,

  $ ./scripts/checkpatch.pl --strict -f newlines.rs
  [...]
  WARNING: Usage of pr_err without a trailing newline. Consider adding '\n'.
  #3: FILE: newlines.rs:3:
  +pr_err!("Has newline\n"); // A comment

I believe you also have to strip out comments?

> +        }
> +      }
> +    }
> +
>  # check for .L prefix local symbols in .S files
>  		if ($realfile =~ /\.S$/ &&
>  		    $line =~ /^\+\s*(?:[A-Z]+_)?SYM_[A-Z]+_(?:START|END)(?:_[A-Z_]+)?\s*\(\s*\.L/) {