From cef572ad9bd7f85035ba8272e5352040e8be0152 Mon Sep 17 00:00:00 2001 From: Li Bin Date: Sat, 28 Oct 2017 11:07:28 +0800 Subject: [PATCH 1/5] workqueue: Fix NULL pointer dereference When queue_work() is used in irq (not in task context), there is a potential case that trigger NULL pointer dereference. ---------------------------------------------------------------- worker_thread() |-spin_lock_irq() |-process_one_work() |-worker->current_pwq = pwq |-spin_unlock_irq() |-worker->current_func(work) |-spin_lock_irq() |-worker->current_pwq = NULL |-spin_unlock_irq() //interrupt here |-irq_handler |-__queue_work() //assuming that the wq is draining |-is_chained_work(wq) |-current_wq_worker() //Here, 'current' is the interrupted worker! |-current->current_pwq is NULL here! |-schedule() ---------------------------------------------------------------- Avoid it by checking for task context in current_wq_worker(), and if not in task context, we shouldn't use the 'current' to check the condition. Reported-by: Xiaofei Tan Signed-off-by: Li Bin Reviewed-by: Lai Jiangshan Signed-off-by: Tejun Heo Fixes: 8d03ecfe4718 ("workqueue: reimplement is_chained_work() using current_wq_worker()") Cc: stable@vger.kernel.org # v3.9+ --- kernel/workqueue_internal.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/kernel/workqueue_internal.h b/kernel/workqueue_internal.h index 8635417c587b7a..29fa81f0f51a72 100644 --- a/kernel/workqueue_internal.h +++ b/kernel/workqueue_internal.h @@ -9,6 +9,7 @@ #include #include +#include struct worker_pool; @@ -59,7 +60,7 @@ struct worker { */ static inline struct worker *current_wq_worker(void) { - if (current->flags & PF_WQ_WORKER) + if (in_task() && (current->flags & PF_WQ_WORKER)) return kthread_data(current); return NULL; } From 5dfeaac15f2b1abb5a53c9146041c7235eb9aa04 Mon Sep 17 00:00:00 2001 From: Andrey Ryabinin Date: Mon, 16 Oct 2017 18:51:30 +0300 Subject: [PATCH 2/5] crypto: x86/sha256-mb - fix panic due to unaligned access struct sha256_ctx_mgr allocated in sha256_mb_mod_init() via kzalloc() and later passed in sha256_mb_flusher_mgr_flush_avx2() function where instructions vmovdqa used to access the struct. vmovdqa requires 16-bytes aligned argument, but nothing guarantees that struct sha256_ctx_mgr will have that alignment. Unaligned vmovdqa will generate GP fault. Fix this by replacing vmovdqa with vmovdqu which doesn't have alignment requirements. Fixes: a377c6b1876e ("crypto: sha256-mb - submit/flush routines for AVX2") Reported-by: Josh Poimboeuf Signed-off-by: Andrey Ryabinin Cc: Acked-by: Tim Chen Signed-off-by: Herbert Xu --- arch/x86/crypto/sha256-mb/sha256_mb_mgr_flush_avx2.S | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/arch/x86/crypto/sha256-mb/sha256_mb_mgr_flush_avx2.S b/arch/x86/crypto/sha256-mb/sha256_mb_mgr_flush_avx2.S index 8fe6338bcc8457..16c4ccb1f15488 100644 --- a/arch/x86/crypto/sha256-mb/sha256_mb_mgr_flush_avx2.S +++ b/arch/x86/crypto/sha256-mb/sha256_mb_mgr_flush_avx2.S @@ -155,8 +155,8 @@ LABEL skip_ %I .endr # Find min length - vmovdqa _lens+0*16(state), %xmm0 - vmovdqa _lens+1*16(state), %xmm1 + vmovdqu _lens+0*16(state), %xmm0 + vmovdqu _lens+1*16(state), %xmm1 vpminud %xmm1, %xmm0, %xmm2 # xmm2 has {D,C,B,A} vpalignr $8, %xmm2, %xmm3, %xmm3 # xmm3 has {x,x,D,C} @@ -176,8 +176,8 @@ LABEL skip_ %I vpsubd %xmm2, %xmm0, %xmm0 vpsubd %xmm2, %xmm1, %xmm1 - vmovdqa %xmm0, _lens+0*16(state) - vmovdqa %xmm1, _lens+1*16(state) + vmovdqu %xmm0, _lens+0*16(state) + vmovdqu %xmm1, _lens+1*16(state) # "state" and "args" are the same address, arg1 # len is arg2 @@ -234,8 +234,8 @@ ENTRY(sha256_mb_mgr_get_comp_job_avx2) jc .return_null # Find min length - vmovdqa _lens(state), %xmm0 - vmovdqa _lens+1*16(state), %xmm1 + vmovdqu _lens(state), %xmm0 + vmovdqu _lens+1*16(state), %xmm1 vpminud %xmm1, %xmm0, %xmm2 # xmm2 has {D,C,B,A} vpalignr $8, %xmm2, %xmm3, %xmm3 # xmm3 has {x,x,D,C} From d041b557792c85677f17e08eee535eafbd6b9aa2 Mon Sep 17 00:00:00 2001 From: Andrey Ryabinin Date: Mon, 16 Oct 2017 18:51:31 +0300 Subject: [PATCH 3/5] crypto: x86/sha1-mb - fix panic due to unaligned access struct sha1_ctx_mgr allocated in sha1_mb_mod_init() via kzalloc() and later passed in sha1_mb_flusher_mgr_flush_avx2() function where instructions vmovdqa used to access the struct. vmovdqa requires 16-bytes aligned argument, but nothing guarantees that struct sha1_ctx_mgr will have that alignment. Unaligned vmovdqa will generate GP fault. Fix this by replacing vmovdqa with vmovdqu which doesn't have alignment requirements. Fixes: 2249cbb53ead ("crypto: sha-mb - SHA1 multibuffer submit and flush routines for AVX2") Signed-off-by: Andrey Ryabinin Cc: Signed-off-by: Herbert Xu --- arch/x86/crypto/sha1-mb/sha1_mb_mgr_flush_avx2.S | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/arch/x86/crypto/sha1-mb/sha1_mb_mgr_flush_avx2.S b/arch/x86/crypto/sha1-mb/sha1_mb_mgr_flush_avx2.S index 93b945597ecfb7..7cfba738f104f5 100644 --- a/arch/x86/crypto/sha1-mb/sha1_mb_mgr_flush_avx2.S +++ b/arch/x86/crypto/sha1-mb/sha1_mb_mgr_flush_avx2.S @@ -157,8 +157,8 @@ LABEL skip_ %I .endr # Find min length - vmovdqa _lens+0*16(state), %xmm0 - vmovdqa _lens+1*16(state), %xmm1 + vmovdqu _lens+0*16(state), %xmm0 + vmovdqu _lens+1*16(state), %xmm1 vpminud %xmm1, %xmm0, %xmm2 # xmm2 has {D,C,B,A} vpalignr $8, %xmm2, %xmm3, %xmm3 # xmm3 has {x,x,D,C} @@ -178,8 +178,8 @@ LABEL skip_ %I vpsubd %xmm2, %xmm0, %xmm0 vpsubd %xmm2, %xmm1, %xmm1 - vmovdqa %xmm0, _lens+0*16(state) - vmovdqa %xmm1, _lens+1*16(state) + vmovdqu %xmm0, _lens+0*16(state) + vmovdqu %xmm1, _lens+1*16(state) # "state" and "args" are the same address, arg1 # len is arg2 @@ -235,8 +235,8 @@ ENTRY(sha1_mb_mgr_get_comp_job_avx2) jc .return_null # Find min length - vmovdqa _lens(state), %xmm0 - vmovdqa _lens+1*16(state), %xmm1 + vmovdqu _lens(state), %xmm0 + vmovdqu _lens+1*16(state), %xmm1 vpminud %xmm1, %xmm0, %xmm2 # xmm2 has {D,C,B,A} vpalignr $8, %xmm2, %xmm3, %xmm3 # xmm3 has {x,x,D,C} From 441f99c90497e15aa3ad1dbabd56187e29614348 Mon Sep 17 00:00:00 2001 From: Romain Izard Date: Tue, 31 Oct 2017 15:42:35 +0100 Subject: [PATCH 4/5] crypto: ccm - preserve the IV buffer The IV buffer used during CCM operations is used twice, during both the hashing step and the ciphering step. When using a hardware accelerator that updates the contents of the IV buffer at the end of ciphering operations, the value will be modified. In the decryption case, the subsequent setup of the hashing algorithm will interpret the updated IV instead of the original value, which can lead to out-of-bounds writes. Reuse the idata buffer, only used in the hashing step, to preserve the IV's value during the ciphering step in the decryption case. Signed-off-by: Romain Izard Reviewed-by: Tudor Ambarus Cc: Signed-off-by: Herbert Xu --- crypto/ccm.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crypto/ccm.c b/crypto/ccm.c index 1ce37ae0ce565a..0a083342ec8cf3 100644 --- a/crypto/ccm.c +++ b/crypto/ccm.c @@ -363,7 +363,7 @@ static int crypto_ccm_decrypt(struct aead_request *req) unsigned int cryptlen = req->cryptlen; u8 *authtag = pctx->auth_tag; u8 *odata = pctx->odata; - u8 *iv = req->iv; + u8 *iv = pctx->idata; int err; cryptlen -= authsize; @@ -379,6 +379,8 @@ static int crypto_ccm_decrypt(struct aead_request *req) if (req->src != req->dst) dst = pctx->dst; + memcpy(iv, req->iv, 16); + skcipher_request_set_tfm(skreq, ctx->ctr); skcipher_request_set_callback(skreq, pctx->flags, crypto_ccm_decrypt_done, req); From 136fc5c41f349296db1910677bb7402b0eeff376 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Mon, 6 Nov 2017 16:19:27 +1100 Subject: [PATCH 5/5] scripts: add leaking_addresses.pl Currently we are leaking addresses from the kernel to user space. This script is an attempt to find some of those leakages. Script parses `dmesg` output and /proc and /sys files for hex strings that look like kernel addresses. Only works for 64 bit kernels, the reason being that kernel addresses on 64 bit kernels have 'ffff' as the leading bit pattern making greping possible. On 32 kernels we don't have this luxury. Scripts is _slightly_ smarter than a straight grep, we check for false positives (all 0's or all 1's, and vsyscall start/finish addresses). [ I think there is a lot of room for improvement here, but it's already useful, so I'm merging it as-is. The whole "hash %p format" series is expected to go into 4.15, but will not fix %x users, and will not incentivize people to look at what they are leaking. - Linus ] Signed-off-by: Tobin C. Harding Signed-off-by: Linus Torvalds --- MAINTAINERS | 5 + scripts/leaking_addresses.pl | 305 +++++++++++++++++++++++++++++++++++ 2 files changed, 310 insertions(+) create mode 100755 scripts/leaking_addresses.pl diff --git a/MAINTAINERS b/MAINTAINERS index 2f4e462aa4a214..a7995c73772811 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -7745,6 +7745,11 @@ S: Maintained F: Documentation/scsi/53c700.txt F: drivers/scsi/53c700* +LEAKING_ADDRESSES +M: Tobin C. Harding +S: Maintained +F: scripts/leaking_addresses.pl + LED SUBSYSTEM M: Richard Purdie M: Jacek Anaszewski diff --git a/scripts/leaking_addresses.pl b/scripts/leaking_addresses.pl new file mode 100755 index 00000000000000..2977371b29563b --- /dev/null +++ b/scripts/leaking_addresses.pl @@ -0,0 +1,305 @@ +#!/usr/bin/env perl +# +# (c) 2017 Tobin C. Harding +# Licensed under the terms of the GNU GPL License version 2 +# +# leaking_addresses.pl: Scan 64 bit kernel for potential leaking addresses. +# - Scans dmesg output. +# - Walks directory tree and parses each file (for each directory in @DIRS). +# +# You can configure the behaviour of the script; +# +# - By adding paths, for directories you do not want to walk; +# absolute paths: @skip_walk_dirs_abs +# directory names: @skip_walk_dirs_any +# +# - By adding paths, for files you do not want to parse; +# absolute paths: @skip_parse_files_abs +# file names: @skip_parse_files_any +# +# The use of @skip_xxx_xxx_any causes files to be skipped where ever they occur. +# For example adding 'fd' to @skip_walk_dirs_any causes the fd/ directory to be +# skipped for all PID sub-directories of /proc +# +# The same thing can be achieved by passing command line options to --dont-walk +# and --dont-parse. If absolute paths are supplied to these options they are +# appended to the @skip_xxx_xxx_abs arrays. If file names are supplied to these +# options, they are appended to the @skip_xxx_xxx_any arrays. +# +# Use --debug to output path before parsing, this is useful to find files that +# cause the script to choke. +# +# You may like to set kptr_restrict=2 before running script +# (see Documentation/sysctl/kernel.txt). + +use warnings; +use strict; +use POSIX; +use File::Basename; +use File::Spec; +use Cwd 'abs_path'; +use Term::ANSIColor qw(:constants); +use Getopt::Long qw(:config no_auto_abbrev); + +my $P = $0; +my $V = '0.01'; + +# Directories to scan. +my @DIRS = ('/proc', '/sys'); + +# Command line options. +my $help = 0; +my $debug = 0; +my @dont_walk = (); +my @dont_parse = (); + +# Do not parse these files (absolute path). +my @skip_parse_files_abs = ('/proc/kmsg', + '/proc/kcore', + '/proc/fs/ext4/sdb1/mb_groups', + '/proc/1/fd/3', + '/sys/kernel/debug/tracing/trace_pipe', + '/sys/kernel/security/apparmor/revision'); + +# Do not parse thes files under any subdirectory. +my @skip_parse_files_any = ('0', + '1', + '2', + 'pagemap', + 'events', + 'access', + 'registers', + 'snapshot_raw', + 'trace_pipe_raw', + 'ptmx', + 'trace_pipe'); + +# Do not walk these directories (absolute path). +my @skip_walk_dirs_abs = (); + +# Do not walk these directories under any subdirectory. +my @skip_walk_dirs_any = ('self', + 'thread-self', + 'cwd', + 'fd', + 'stderr', + 'stdin', + 'stdout'); + +sub help +{ + my ($exitcode) = @_; + + print << "EOM"; +Usage: $P [OPTIONS] +Version: $V + +Options: + + --dont-walk= Don't walk tree starting at . + --dont-parse= Don't parse . + -d, --debug Display debugging output. + -h, --help, --version Display this help and exit. + +If an absolute path is passed to --dont_XXX then this path is skipped. If a +single filename is passed then this file/directory will be skipped when +appearing under any subdirectory. + +Example: + + # Just scan dmesg output. + scripts/leaking_addresses.pl --dont_walk_abs /proc --dont_walk_abs /sys + +Scans the running (64 bit) kernel for potential leaking addresses. + +EOM + exit($exitcode); +} + +GetOptions( + 'dont-walk=s' => \@dont_walk, + 'dont-parse=s' => \@dont_parse, + 'd|debug' => \$debug, + 'h|help' => \$help, + 'version' => \$help +) or help(1); + +help(0) if ($help); + +push_to_global(); + +parse_dmesg(); +walk(@DIRS); + +exit 0; + +sub debug_arrays +{ + print 'dirs_any: ' . join(", ", @skip_walk_dirs_any) . "\n"; + print 'dirs_abs: ' . join(", ", @skip_walk_dirs_abs) . "\n"; + print 'parse_any: ' . join(", ", @skip_parse_files_any) . "\n"; + print 'parse_abs: ' . join(", ", @skip_parse_files_abs) . "\n"; +} + +sub dprint +{ + printf(STDERR @_) if $debug; +} + +sub push_in_abs_any +{ + my ($in, $abs, $any) = @_; + + foreach my $path (@$in) { + if (File::Spec->file_name_is_absolute($path)) { + push @$abs, $path; + } elsif (index($path,'/') == -1) { + push @$any, $path; + } else { + print 'path error: ' . $path; + } + } +} + +# Push command line options to global arrays. +sub push_to_global +{ + push_in_abs_any(\@dont_walk, \@skip_walk_dirs_abs, \@skip_walk_dirs_any); + push_in_abs_any(\@dont_parse, \@skip_parse_files_abs, \@skip_parse_files_any); +} + +sub is_false_positive +{ + my ($match) = @_; + + if ($match =~ '\b(0x)?(f|F){16}\b' or + $match =~ '\b(0x)?0{16}\b') { + return 1; + } + + # vsyscall memory region, we should probably check against a range here. + if ($match =~ '\bf{10}600000\b' or + $match =~ '\bf{10}601000\b') { + return 1; + } + + return 0; +} + +# True if argument potentially contains a kernel address. +sub may_leak_address +{ + my ($line) = @_; + my $address = '\b(0x)?ffff[[:xdigit:]]{12}\b'; + + # Signal masks. + if ($line =~ '^SigBlk:' or + $line =~ '^SigCgt:') { + return 0; + } + + if ($line =~ '\bKEY=[[:xdigit:]]{14} [[:xdigit:]]{16} [[:xdigit:]]{16}\b' or + $line =~ '\b[[:xdigit:]]{14} [[:xdigit:]]{16} [[:xdigit:]]{16}\b') { + return 0; + } + + while (/($address)/g) { + if (!is_false_positive($1)) { + return 1; + } + } + + return 0; +} + +sub parse_dmesg +{ + open my $cmd, '-|', 'dmesg'; + while (<$cmd>) { + if (may_leak_address($_)) { + print 'dmesg: ' . $_; + } + } + close $cmd; +} + +# True if we should skip this path. +sub skip +{ + my ($path, $paths_abs, $paths_any) = @_; + + foreach (@$paths_abs) { + return 1 if (/^$path$/); + } + + my($filename, $dirs, $suffix) = fileparse($path); + foreach (@$paths_any) { + return 1 if (/^$filename$/); + } + + return 0; +} + +sub skip_parse +{ + my ($path) = @_; + return skip($path, \@skip_parse_files_abs, \@skip_parse_files_any); +} + +sub parse_file +{ + my ($file) = @_; + + if (! -R $file) { + return; + } + + if (skip_parse($file)) { + dprint "skipping file: $file\n"; + return; + } + dprint "parsing: $file\n"; + + open my $fh, "<", $file or return; + while ( <$fh> ) { + if (may_leak_address($_)) { + print $file . ': ' . $_; + } + } + close $fh; +} + + +# True if we should skip walking this directory. +sub skip_walk +{ + my ($path) = @_; + return skip($path, \@skip_walk_dirs_abs, \@skip_walk_dirs_any) +} + +# Recursively walk directory tree. +sub walk +{ + my @dirs = @_; + my %seen; + + while (my $pwd = shift @dirs) { + next if (skip_walk($pwd)); + next if (!opendir(DIR, $pwd)); + my @files = readdir(DIR); + closedir(DIR); + + foreach my $file (@files) { + next if ($file eq '.' or $file eq '..'); + + my $path = "$pwd/$file"; + next if (-l $path); + + if (-d $path) { + push @dirs, $path; + } else { + parse_file($path); + } + } + } +}