From 765f2bf04fdaced4e7d7e94cfc3f743048629f31 Mon Sep 17 00:00:00 2001 From: Borislav Petkov Date: Mon, 8 Aug 2022 10:59:28 +0200 Subject: scripts/decodecode: improve faulting line determination There are cases where the IP pointer in a Code: line in an oops doesn't point at the beginning of an instruction: Code: 0f bd c2 e9 a0 cd b5 e4 48 0f bd c2 e9 97 cd b5 e4 0f 1f 80 00 00 00 00 \ e9 8b cd b5 e4 0f 1f 00 66 0f a3 d0 e9 7f cd b5 e4 0f 1f <80> 00 00 00 \ 00 0f a3 d0 e9 70 cd b5 e4 48 0f a3 d0 e9 67 cd b5 e9 7f cd b5 e4 jmp 0xffffffffe4b5cda8 0f 1f 80 00 00 00 00 nopl 0x0(%rax) ^^ and the current way of determining the faulting instruction line doesn't work because disassembled instructions are counted from the IP byte to the end and when that thing points in the middle, the trailing bytes can be interpreted as different insns: Code starting with the faulting instruction =========================================== 0: 80 00 00 addb $0x0,(%rax) 3: 00 00 add %al,(%rax) whereas, this is part of 0f 1f 80 00 00 00 00 nopl 0x0(%rax) 5: 0f a3 d0 bt %edx,%eax ... leading to: 1d: 0f 1f 00 nopl (%rax) 20: 66 0f a3 d0 bt %dx,%ax 24:* e9 7f cd b5 e4 jmp 0xffffffffe4b5cda8 <-- trapping instruction 29: 0f 1f 80 00 00 00 00 nopl 0x0(%rax) 30: 0f a3 d0 bt %edx,%eax which is the wrong faulting instruction. Change the way the faulting line number is determined by matching the opcode bytes from the beginning, leading to correct output: 1d: 0f 1f 00 nopl (%rax) 20: 66 0f a3 d0 bt %dx,%ax 24: e9 7f cd b5 e4 jmp 0xffffffffe4b5cda8 29:* 0f 1f 80 00 00 00 00 nopl 0x0(%rax) <-- trapping instruction 30: 0f a3 d0 bt %edx,%eax While at it, make decodecode use bash as the interpreter - that thing should be present on everything by now. It simplifies the code a lot too. Link: https://lkml.kernel.org/r/20220808085928.29840-1-bp@alien8.de Signed-off-by: Borislav Petkov Cc: Marc Zyngier Cc: Will Deacon Signed-off-by: Andrew Morton --- scripts/decodecode | 120 ++++++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 105 insertions(+), 15 deletions(-) (limited to 'scripts') diff --git a/scripts/decodecode b/scripts/decodecode index c711a196511c..b28fd2686561 100755 --- a/scripts/decodecode +++ b/scripts/decodecode @@ -1,4 +1,4 @@ -#!/bin/sh +#!/bin/bash # SPDX-License-Identifier: GPL-2.0 # Disassemble the Code: line in Linux oopses # usage: decodecode < oops.file @@ -8,6 +8,8 @@ # AFLAGS=--32 decodecode < 386.oops # PC=hex - the PC (program counter) the oops points to +faultlinenum=1 + cleanup() { rm -f $T $T.s $T.o $T.oo $T.aa $T.dis exit 1 @@ -102,28 +104,125 @@ disas() { grep -v "/tmp\|Disassembly\|\.text\|^$" > $t.dis 2>&1 } +# Match the maximum number of opcode bytes from @op_bytes contained within +# @opline +# +# Params: +# @op_bytes: The string of bytes from the Code: line +# @opline: The disassembled line coming from objdump +# +# Returns: +# The max number of opcode bytes from the beginning of @op_bytes which match +# the opcode bytes in the objdump line. +get_substr_opcode_bytes_num() +{ + local op_bytes=$1 + local opline=$2 + + local retval=0 + substr="" + + for opc in $op_bytes; + do + substr+="$opc" + + # return if opcode bytes do not match @opline anymore + if ! echo $opline | grep -q "$substr"; + then + break + fi + + # add trailing space + substr+=" " + retval=$((retval+1)) + done + + return $retval +} + +# Return the line number in objdump output to where the IP marker in the Code: +# line points to +# +# Params: +# @all_code: code in bytes without the marker +# @dis_file: disassembled file +# @ip_byte: The byte to which the IP points to +get_faultlinenum() +{ + local all_code="$1" + local dis_file="$2" + + # num bytes including IP byte + local num_bytes_ip=$(( $3 + 1 * $width )) + + # Add the two header lines (we're counting from 1). + local retval=3 + + # remove marker + all_code=$(echo $all_code | sed -e 's/[<>()]//g') + + while read line + do + get_substr_opcode_bytes_num "$all_code" "$line" + ate_opcodes=$? + + if ! (( $ate_opcodes )); then + continue + fi + + num_bytes_ip=$((num_bytes_ip - ($ate_opcodes * $width) )) + if (( $num_bytes_ip <= 0 )); then + break + fi + + # Delete matched opcode bytes from all_code. For that, compute + # how many chars those opcodes are represented by and include + # trailing space. + # + # a byte is 2 chars, ate_opcodes is also the number of trailing + # spaces + del_chars=$(( ($ate_opcodes * $width * 2) + $ate_opcodes )) + + all_code=$(echo $all_code | sed -e "s!^.\{$del_chars\}!!") + + let "retval+=1" + + done < $dis_file + + return $retval +} + marker=`expr index "$code" "\<"` if [ $marker -eq 0 ]; then marker=`expr index "$code" "\("` fi - touch $T.oo if [ $marker -ne 0 ]; then - # 2 opcode bytes and a single space - pc_sub=$(( $marker / 3 )) + # How many bytes to subtract from the program counter + # in order to get to the beginning virtual address of the + # Code: + pc_sub=$(( (($marker - 1) / (2 * $width + 1)) * $width )) echo All code >> $T.oo echo ======== >> $T.oo beforemark=`echo "$code"` echo -n " .$type 0x" > $T.s + echo $beforemark | sed -e 's/ /,0x/g; s/[<>()]//g' >> $T.s + disas $T $pc_sub + cat $T.dis >> $T.oo - rm -f $T.o $T.s $T.dis -# and fix code at-and-after marker + get_faultlinenum "$code" "$T.dis" $pc_sub + faultlinenum=$? + + # and fix code at-and-after marker code=`echo "$code" | cut -c$((${marker} + 1))-` + + rm -f $T.o $T.s $T.dis fi + echo Code starting with the faulting instruction > $T.aa echo =========================================== >> $T.aa code=`echo $code | sed -e 's/\r//;s/ [<(]/ /;s/[>)] / /;s/ /,0x/g; s/[>)]$//'` @@ -132,15 +231,6 @@ echo $code >> $T.s disas $T 0 cat $T.dis >> $T.aa -# (lines of whole $T.oo) - (lines of $T.aa, i.e. "Code starting") + 3, -# i.e. the title + the "===..=" line (sed is counting from 1, 0 address is -# special) -faultlinenum=$(( $(wc -l $T.oo | cut -d" " -f1) - \ - $(wc -l $T.aa | cut -d" " -f1) + 3)) - -faultline=`cat $T.dis | head -1 | cut -d":" -f2-` -faultline=`echo "$faultline" | sed -e 's/\[/\\\[/g; s/\]/\\\]/g'` - cat $T.oo | sed -e "${faultlinenum}s/^\([^:]*:\)\(.*\)/\1\*\2\t\t<-- trapping instruction/" echo cat $T.aa -- cgit v1.2.3-59-g8ed1b From defdaff15a84c68521c5f02b157fc8541e0356f3 Mon Sep 17 00:00:00 2001 From: Ira Weiny Date: Sat, 13 Aug 2022 15:00:34 -0700 Subject: checkpatch: add kmap and kmap_atomic to the deprecated list kmap() and kmap_atomic() are being deprecated in favor of kmap_local_page(). There are two main problems with kmap(): (1) It comes with an overhead as mapping space is restricted and protected by a global lock for synchronization and (2) it also requires global TLB invalidation when the kmap's pool wraps and it might block when the mapping space is fully utilized until a slot becomes available. kmap_local_page() is safe from any context and is therefore redundant with kmap_atomic() with the exception of any pagefault or preemption disable requirements. However, using kmap_atomic() for these side effects makes the code less clear. So any requirement for pagefault or preemption disable should be made explicitly. With kmap_local_page() the mappings are per thread, CPU local, can take page faults, and can be called from any context (including interrupts). It is faster than kmap() in kernels with HIGHMEM enabled. Furthermore, the tasks can be preempted and, when they are scheduled to run again, the kernel virtual addresses are restored. Link: https://lkml.kernel.org/r/20220813220034.806698-1-ira.weiny@intel.com Signed-off-by: Ira Weiny Suggested-by: Thomas Gleixner Suggested-by: Fabio M. De Francesco Reviewed-by: Chaitanya Kulkarni Cc: Joe Perches Signed-off-by: Andrew Morton --- scripts/checkpatch.pl | 2 ++ 1 file changed, 2 insertions(+) (limited to 'scripts') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 79e759aac543..9ff219e0a9d5 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -807,6 +807,8 @@ our %deprecated_apis = ( "rcu_barrier_sched" => "rcu_barrier", "get_state_synchronize_sched" => "get_state_synchronize_rcu", "cond_synchronize_sched" => "cond_synchronize_rcu", + "kmap" => "kmap_local_page", + "kmap_atomic" => "kmap_local_page", ); #Create a search pattern for all these strings to speed up a loop below -- cgit v1.2.3-59-g8ed1b From 8ea0114eda0c1c85f8f01922ac8fc1e489a61129 Mon Sep 17 00:00:00 2001 From: Mickaël Salaün Date: Fri, 2 Sep 2022 13:19:23 +0200 Subject: checkpatch: handle FILE pointer type MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When using a "FILE *" type, checkpatch considers this an error: ERROR: need consistent spacing around '*' (ctx:WxV) #32: FILE: f.c:8: +static void a(FILE *const b) ^ Fix this by explicitly defining "FILE" as a common type. This is useful for user space patches. With this patch, we now get: <_>WS( ) <_>IDENT(static) <_>WS( ) <_>DECLARE(void ) <_>FUNC(a) PAREN('(') <_>DECLARE(FILE *const ) <_>IDENT(b) <_>PAREN(')') -> V <_>WS( ) 32 > . static void a(FILE *const b) 32 > EEVVVVVVVTTTTTVNTTTTTTTTTTTTVVV 32 > ______________________________ Link: https://lkml.kernel.org/r/20220902111923.1488671-1-mic@digikod.net Link: https://lore.kernel.org/r/20220902111923.1488671-1-mic@digikod.net Signed-off-by: Mickaël Salaün Acked-by: Joe Perches Cc: Andy Whitcroft Cc: Dwaipayan Ray Cc: Lukas Bulwahn Signed-off-by: Andrew Morton --- scripts/checkpatch.pl | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'scripts') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 9ff219e0a9d5..18effbe1fe90 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -576,10 +576,14 @@ our $typeKernelTypedefs = qr{(?x: (?:__)?(?:u|s|be|le)(?:8|16|32|64)| atomic_t )}; +our $typeStdioTypedefs = qr{(?x: + FILE +)}; our $typeTypedefs = qr{(?x: $typeC99Typedefs\b| $typeOtherOSTypedefs\b| - $typeKernelTypedefs\b + $typeKernelTypedefs\b| + $typeStdioTypedefs\b )}; our $zero_initializer = qr{(?:(?:0[xX])?0+$Int_type?|NULL|false)\b}; -- cgit v1.2.3-59-g8ed1b From bd17e036b495bebbf07a5fc814c868e30e1dc131 Mon Sep 17 00:00:00 2001 From: Niklas Söderlund Date: Wed, 14 Sep 2022 12:02:55 +0200 Subject: checkpatch: warn for non-standard fixes tag style MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a warning for fixes tags that does not follow community conventions. Link: https://lkml.kernel.org/r/20220914100255.1048460-1-niklas.soderlund@corigine.com Signed-off-by: Niklas Söderlund Reviewed-by: Simon Horman Reviewed-by: Louis Peens Reviewed-by: Philippe Schenker Acked-by: Dwaipayan Ray Reviewed-by: Lukas Bulwahn Acked-by: Lukas Bulwahn Acked-by: Joe Perches Signed-off-by: Andrew Morton --- Documentation/dev-tools/checkpatch.rst | 7 ++++++ scripts/checkpatch.pl | 44 ++++++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+) (limited to 'scripts') diff --git a/Documentation/dev-tools/checkpatch.rst b/Documentation/dev-tools/checkpatch.rst index b52452bc2963..c3389c6f3838 100644 --- a/Documentation/dev-tools/checkpatch.rst +++ b/Documentation/dev-tools/checkpatch.rst @@ -612,6 +612,13 @@ Commit message See: https://www.kernel.org/doc/html/latest/process/submitting-patches.html#describe-your-changes + **BAD_FIXES_TAG** + The Fixes: tag is malformed or does not follow the community conventions. + This can occur if the tag have been split into multiple lines (e.g., when + pasted in an email program with word wrapping enabled). + + See: https://www.kernel.org/doc/html/latest/process/submitting-patches.html#describe-your-changes + Comparison style ---------------- diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 18effbe1fe90..e8e0542f29f0 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -3146,6 +3146,50 @@ sub process { } } +# Check Fixes: styles is correct + if (!$in_header_lines && + $line =~ /^\s*fixes:?\s*(?:commit\s*)?[0-9a-f]{5,}\b/i) { + my $orig_commit = ""; + my $id = "0123456789ab"; + my $title = "commit title"; + my $tag_case = 1; + my $tag_space = 1; + my $id_length = 1; + my $id_case = 1; + my $title_has_quotes = 0; + + if ($line =~ /(\s*fixes:?)\s+([0-9a-f]{5,})\s+($balanced_parens)/i) { + my $tag = $1; + $orig_commit = $2; + $title = $3; + + $tag_case = 0 if $tag eq "Fixes:"; + $tag_space = 0 if ($line =~ /^fixes:? [0-9a-f]{5,} ($balanced_parens)/i); + + $id_length = 0 if ($orig_commit =~ /^[0-9a-f]{12}$/i); + $id_case = 0 if ($orig_commit !~ /[A-F]/); + + # Always strip leading/trailing parens then double quotes if existing + $title = substr($title, 1, -1); + if ($title =~ /^".*"$/) { + $title = substr($title, 1, -1); + $title_has_quotes = 1; + } + } + + my ($cid, $ctitle) = git_commit_info($orig_commit, $id, + $title); + + if ($ctitle ne $title || $tag_case || $tag_space || + $id_length || $id_case || !$title_has_quotes) { + if (WARN("BAD_FIXES_TAG", + "Please use correct Fixes: style 'Fixes: <12 chars of sha1> (\"\")' - ie: 'Fixes: $cid (\"$ctitle\")'\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] = "Fixes: $cid (\"$ctitle\")"; + } + } + } + # Check email subject for common tools that don't need to be mentioned if ($in_header_lines && $line =~ /^Subject:.*\b(?:checkpatch|sparse|smatch)\b[^:]/i) { -- cgit v1.2.3-59-g8ed1b