- 16 Oct, 2020 40 commits
-
-
Jann Horn authored
In both binfmt_elf and binfmt_elf_fdpic, use a new helper dump_vma_snapshot() to take a snapshot of the VMA list (including the gate VMA, if we have one) while protected by the mmap_lock, and then use that snapshot instead of walking the VMA list without locking. An alternative approach would be to keep the mmap_lock held across the entire core dumping operation; however, keeping the mmap_lock locked while we may be blocked for an unbounded amount of time (e.g. because we're dumping to a FUSE filesystem or so) isn't really optimal; the mmap_lock blocks things like the ->release handler of userfaultfd, and we don't really want critical system daemons to grind to a halt just because someone "gifted" them SCM_RIGHTS to an eternally-locked userfaultfd, or something like that. Since both the normal ELF code and the FDPIC ELF code need this functionality (and if any other binfmt wants to add coredump support in the future, they'd probably need it, too), implement this with a common helper in fs/coredump.c. A downside of this approach is that we now need a bigger amount of kernel memory per userspace VMA in the normal ELF case, and that we need O(n) kernel memory in the FDPIC ELF case at all; but 40 bytes per VMA shouldn't be terribly bad. There currently is a data race between stack expansion and anything that reads ->vm_start or ->vm_end under the mmap_lock held in read mode; to mitigate that for core dumping, take the mmap_lock in write mode when taking a snapshot of the VMA hierarchy. (If we only took the mmap_lock in read mode, we could end up with a corrupted core dump if someone does get_user_pages_remote() concurrently. Not really a major problem, but taking the mmap_lock either way works here, so we might as well avoid the issue.) (This doesn't do anything about the existing data races with stack expansion in other mm code.) Signed-off-by:
Jann Horn <jannh@google.com> Signed-off-by:
Andrew Morton <akpm@linux-foundation.org> Acked-by:
Linus Torvalds <torvalds@linux-foundation.org> Cc: Christoph Hellwig <hch@lst.de> Cc: Alexander Viro <viro@zeniv.linux.org.uk> Cc: "Eric W . Biederman" <ebiederm@xmission.com> Cc: Oleg Nesterov <oleg@redhat.com> Cc: Hugh Dickins <hughd@google.com> Link: http://lkml.kernel.org/r/20200827114932.3572699-6-jannh@google.comSigned-off-by:
-
Jann Horn authored
At the moment, the binfmt_elf and binfmt_elf_fdpic code have slightly different code to figure out which VMAs should be dumped, and if so, whether the dump should contain the entire VMA or just its first page. Eliminate duplicate code by reworking the binfmt_elf version into a generic core dumping helper in coredump.c. As part of that, change the heuristic for detecting executable/library header pages to check whether the inode is executable instead of looking at the file mode. This is less problematic in terms of locking because it lets us avoid get_user() under the mmap_sem. (And arguably it looks nicer and makes more sense in generic code.) Adjust a little bit based on the binfmt_elf_fdpic version: ->anon_vma is only meaningful under CONFIG_MMU, otherwise we have to assume that the VMA has been written to. Suggested-by:
-
Jann Horn authored
Both fs/binfmt_elf.c and fs/binfmt_elf_fdpic.c need to dump ranges of pages into the coredump file. Extract that logic into a common helper. Signed-off-by:
-
Jann Horn authored
dump_emit() has a retry loop, but there seems to be no way for that retry logic to actually be used; and it was also buggy, writing the same data repeatedly after a short write. Let's just bail out on a short write. Suggested-by:
-
Jann Horn authored
Patch series "Fix ELF / FDPIC ELF core dumping, and use mmap_lock properly in there", v5. At the moment, we have that rather ugly mmget_still_valid() helper to work around <https://crbug.com/project-zero/1790>: ELF core dumping doesn't take the mmap_sem while traversing the task's VMAs, and if anything (like userfaultfd) then remotely messes with the VMA tree, fireworks ensue. So at the moment we use mmget_still_valid() to bail out in any writers that might be operating on a remote mm's VMAs. With this series, I'm trying to get rid of the need for that as cleanly as possible. ("cleanly" meaning "avoid holding the mmap_lock across unbounded sleeps".) Patches 1, 2, 3 and 4 are relatively unrelated cleanups in the core dumping code. Patches 5 and 6 implement the main change: Instead of repeatedly accessing the VMA list with sleeps in between, we snapshot it at the start with proper locking, and then later we just use our copy of the VMA list. This ensures that the kernel won't crash, that VMA metadata in the coredump is consistent even in the presence of concurrent modifications, and that any virtual addresses that aren't being concurrently modified have their contents show up in the core dump properly. The disadvantage of this approach is that we need a bit more memory during core dumping for storing metadata about all VMAs. At the end of the series, patch 7 removes the old workaround for this issue (mmget_still_valid()). I have tested: - Creating a simple core dump on X86-64 still works. - The created coredump on X86-64 opens in GDB and looks plausible. - X86-64 core dumps contain the first page for executable mappings at offset 0, and don't contain the first page for non-executable file mappings or executable mappings at offset !=0. - NOMMU 32-bit ARM can still generate plausible-looking core dumps through the FDPIC implementation. (I can't test this with GDB because GDB is missing some structure definition for nommu ARM, but I've poked around in the hexdump and it looked decent.) This patch (of 7): dump_emit() is for kernel pointers, and VMAs describe userspace memory. Let's be tidy here and avoid accessing userspace pointers under KERNEL_DS, even if it probably doesn't matter much on !MMU systems - especially given that it looks like we can just use the same get_dump_page() as on MMU if we move it out of the CONFIG_MMU block. One small change we have to make in get_dump_page() is to use __get_user_pages_locked() instead of __get_user_pages(), since the latter doesn't exist on nommu. On mmu builds, __get_user_pages_locked() will just call __get_user_pages() for us. Signed-off-by:
-
Chris Kennelly authored
This produces a PIE binary with a variety of p_align requirements, suitable for verifying that the load address meets that alignment requirement. Signed-off-by:
Chris Kennelly <ckennelly@google.com> Signed-off-by:
-
Chris Kennelly authored
Patch series "Selecting Load Addresses According to p_align", v3. The current ELF loading mechancism provides page-aligned mappings. This can lead to the program being loaded in a way unsuitable for file-backed, transparent huge pages when handling PIE executables. While specifying -z,max-page-size=0x200000 to the linker will generate suitably aligned segments for huge pages on x86_64, the executable needs to be loaded at a suitably aligned address as well. This alignment requires the binary's cooperation, as distinct segments need to be appropriately paddded to be eligible for THP. For binaries built with increased alignment, this limits the number of bits usable for ASLR, but provides some randomization over using fixed load addresses/non-PIE binaries. This patch (of 2): The current ELF loading mechancism provides page-aligned mappings. This can lead to the program being loaded in a way unsuitable for file-backed, transparent huge pages when handling PIE executables. For binaries built with increased alignment, this limits the number of bits usable for ASLR, but provides some randomization over using fixed load addresses/non-PIE binaries. Tested by verifying program with -Wl,-z,max-page-size=0x200000 loading. [akpm@linux-foundation.org: fix max() warning] [ckennelly@google.com: augment comment] Link: https://lkml.kernel.org/r/20200821233848.3904680-2-ckennelly@google.comSigned-off-by:
-
Dwaipayan Ray authored
The author signed-off-by checks are currently very vague. Cases like same name or same address are not handled separately. For example, running checkpatch on commit be6577af ("parisc: Add atomic64_set_release() define to avoid CPU soft lockups"), gives: WARNING: Missing Signed-off-by: line by nominal patch author 'John David Anglin <dave.anglin@bell.net>' The signoff line was: "Signed-off-by: Dave Anglin <dave.anglin@bell.net>" Clearly the author has signed off but with a slightly different version of his name. A more appropriate warning would have been to point out at the name mismatch instead. Previously, the values assumed by $authorsignoff were either 0 or 1 to indicate whether a proper sign off by author is present. Extended the checks to handle four new cases. $authorsignoff values now denote the following: 0: Missing sign off by patch author. 1: Sign off present and identical. 2: Addresses and names match, but comments differ. "James Watson(JW) <james@gmail.com>", "James Watson <james@gmail.com>" 3: Addresses match, but names are different. "James Watson <james@gmail.com>", "James <james@gmail.com>" 4: Names match, but addresses are different. "James Watson <james@watson.com>", "James Watson <james@gmail.com>" 5: Names match, addresses excluding subaddress details (RFC 5233) match. "James Watson <james@gmail.com>", "James Watson <james+a@gmail.com>" Also introduced a new message type FROM_SIGN_OFF_MISMATCH for cases 2, 3, 4 and 5. Suggested-by:
Joe Perches <joe@perches.com> Signed-off-by:
Dwaipayan Ray <dwaipayanray1@gmail.com> Signed-off-by:
-
Łukasz Stelmach authored
To avoid false positives in presence of SPDX-License-Identifier in networking files it is required to increase the leeway for empty block comment lines by one line. For example, checking drivers/net/loopback.c which starts with // SPDX-License-Identifier: GPL-2.0-or-later /* * INET An implementation of the TCP/IP protocol suite for the LINUX rsults in an unnecessary warning WARNING: networking block comments don't use an empty /* line, use /* Comment... +/* + * INET An implementation of the TCP/IP protocol suite for the LINUX Signed-off-by:Łukasz Stelmach <l.stelmach@samsung.com> Signed-off-by:
-
Dwaipayan Ray authored
Checkpatch.pl doesn't have a check for excluding while (...) {...} blocks from MULTISTATEMENT_MACRO_USE_DO_WHILE error. For example, running checkpatch.pl on the file mm/maccess.c in the kernel generates the following error: ERROR: Macros with complex values should be enclosed in parentheses +#define copy_from_kernel_nofault_loop(dst, src, len, type, err_label) \ + while (len >= sizeof(type)) { \ + __get_kernel_nofault(dst, src, type, err_label); \ + dst += sizeof(type); \ + src += sizeof(type); \ + len -= sizeof(type); \ + } The error is misleading for this case. Enclosing it in parentheses doesn't make any sense. Checkpatch already has an exception list for such common macro types. Added a new exception for while (...) {...} style blocks to the same. In addition, the brace flatten logic was modified by changing the substitution characters from "1" to "1u". This was done to ensure that macros in the form "#define foo(bar) while(bar){bar--;}" were also correctly procecssed. Link: https://lore.kernel.org/linux-kernel-mentees/dc985938aa3986702815a0bd68dfca8a03c85447.camel@perches.com/Suggested-by: -
Joe Perches authored
Embedding the complete filename path inside the file isn't particularly useful as often the path is moved around and becomes incorrect. Emit a warning when the source contains the filename. [akpm@linux-foundation.org: remove stray " di"] Signed-off-by:
-
Dwaipayan Ray authored
Checkpatch did not handle cases where the author From: header was split into multiple lines. The author identity could not be resolved and checkpatch generated a false NO_AUTHOR_SIGN_OFF warning. A typical example is commit e33bcbab ("tee: add support for session's client UUID generation"). When checkpatch was run on this commit, it displayed: "WARNING:NO_AUTHOR_SIGN_OFF: Missing Signed-off-by: line by nominal patch author ''" This was due to split header lines not being handled properly and the author himself wrote in commit cd261496 ("checkpatch: warn if missing author Signed-off-by"): "Split From: headers are not fully handled: only the first part is compared." Support split From: headers by correctly parsing the header extension lines. RFC 5322, Section-2.2.3 stated that each extended line must start with a WSP character (a space or htab). The solution was therefore to concatenate the lines which start with a WSP to get the correct long header. Suggested-by:
Lukas Bulwahn <lukas.bulwahn@gmail.com> Reviewed-by:
-
Joe Perches authored
If a file exists in git and checkpatch is used without the -f flag for scanning a file, then checkpatch will scan the file assuming it's a patch and emit: ERROR: Does not appear to be a unified-diff format patch Change the behavior to assume the -f flag if the file exists in git. [joe@perches.com: fix git "fatal" warning if file argument outside kernel tree] Link: https://lkml.kernel.org/r/b6afa04112d450c2fc120a308d706acd60cee294.camel@perches.comSigned-off-by:
Julia Lawall <julia.lawall@inria.fr> Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk> Link: https://lkml.kernel.org/r/45b81a48e1568bd0126a96f5046eb7aaae9b83c9.camel@perches.comSigned-off-by:
-
Joe Perches authored
The uninitialized_var() macro was removed recently via commit 63a0895d ("compiler: Remove uninitialized_var() macro") as it's not a particularly useful warning and its use can "paper over real bugs". Add a checkpatch test to warn on self-assignments as a means to avoid compiler warnings and as a back-door mechanism to reproduce the old uninitialized_var macro behavior. [akpm@linux-foundation.org: coding style fixes] Signed-off-by:
-
Rikard Falkeborn authored
All usages of include/linux of these are const pointers, and all instances in the kernel except one, that are not const can be made const (patches have been posted for those separately). Signed-off-by:
Rikard Falkeborn <rikard.falkeborn@gmail.com> Signed-off-by:
Linus Walleij <linus.walleij@linaro.org> Acked-by:
Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> Cc: Joe Perches <joe@perches.com> Cc: Andreas Färber <afaerber@suse.de> Cc: Rikard Falkeborn <rikard.falkeborn@gmail.com> Link: https://lkml.kernel.org/r/20200830224352.37114-1-rikard.falkeborn@gmail.comSigned-off-by:
-
Nicolas Boichat authored
trace_printk is meant as a debugging tool, and should not be compiled into production code without specific debug Kconfig options enabled, or source code changes, as indicated by the warning that shows up on boot if any trace_printk is called: ** NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE ** ** ** ** trace_printk() being used. Allocating extra memory. ** ** ** ** This means that this is a DEBUG kernel and it is ** ** unsafe for production use. ** Let's warn developers when they try to submit such a change. Signed-off-by:
Nicolas Boichat <drinkcat@chromium.org> Signed-off-by:
-
Rikard Falkeborn authored
All usages of phy_ops in include/linux uses const phy_ops * and all instances of phy_ops in the kernel that are not const already can be made const (patches have been posted for those separately). Suggested-by:
-
Joe Perches authored
There are commas used as statement terminations that should typically have used semicolons instead. Only direct assignments or use of a single function or value on a single line are detected by this test. e.g.: foo = bar(), /* typical use is semicolon not comma */ bar = baz(); Add an imperfect test to detect these comma uses. No false positives were found in testing, but many types of false negatives are possible. e.g.: foo = bar() + 1, /* comma use, but not direct assignment */ bar = baz(); Signed-off-by:
-
Joe Perches authored
Currently this test only works on .[ch] files. Move the test to check more file types and the commit log. Signed-off-by:
-
Jerome Forissier authored
Kconfig allows to customize the CONFIG_ prefix via the $CONFIG_ environment variable. Out-of-tree projects may therefore use Kconfig with a different prefix, or they may use a custom configuration tool which does not use the CONFIG_ prefix at all. Such projects may still want to adhere to the Linux kernel coding style and run checkpatch.pl. One example is OP-TEE [1] which does not use Kconfig but does have configuration options prefixed with CFG_. It also mostly follows the kernel coding style and therefore being able to use checkpatch is quite valuable. To make this possible, add the --kconfig-prefix command line option. [1] https://github.com/OP-TEE/optee_osSigned-off-by:
Jerome Forissier <jerome@forissier.org> Signed-off-by:
-
Wei Yang authored
These two functions share the same logic. Signed-off-by:
Wei Yang <richard.weiyang@linux.alibaba.com> Signed-off-by:
-
Wei Yang authored
These two cases could be unified into one. Signed-off-by:
-
Tobias Jordan authored
Whether crc32_be needs a lookup table is chosen based on CRC_LE_BITS. Obviously, the _be function should be governed by the _BE_ define. This probably never pops up as it's hard to come up with a configuration where CRC_BE_BITS isn't the same as CRC_LE_BITS and as nobody is using bitwise CRC anyway. Fixes: 46c5801e ("crc32: bolt on crc32c") Signed-off-by:
Tobias Jordan <kernel@cdqe.de> Signed-off-by:
-
Dan Carpenter authored
This is supposed to return false on failure, not a negative error code. Fixes: 170e38548b81 ("mm/hmm/test: use after free in dmirror_allocate_chunk()") Signed-off-by:Dan Carpenter <dan.carpenter@oracle.com> Signed-off-by:
Ralph Campbell <rcampbell@nvidia.com> Cc: Jerome Glisse <jglisse@redhat.com> Cc: Stephen Rothwell <sfr@canb.auug.org.au> Cc: Jason Gunthorpe <jgg@ziepe.ca> Cc: Dan Williams <dan.j.williams@intel.com> Link: https://lkml.kernel.org/r/20201010200812.GA1886610@mwandaSigned-off-by:
-
Andy Shevchenko authored
Add a macro to test if entry is pointing to the head of the list which is useful in cases like: list_for_each_entry(pos, &head, member) { if (cond) break; } if (list_entry_is_head(pos, &head, member)) return -ERRNO; that allows to avoid additional variable to be added to track if loop has not been stopped in the middle. While here, convert list_for_each_entry*() family of macros to use a new one. Signed-off-by:Andy Shevchenko <andriy.shevchenko@linux.intel.com> Signed-off-by:
Cezary Rojewski <cezary.rojewski@intel.com> Link: https://lkml.kernel.org/r/20200929134342.51489-1-andriy.shevchenko@linux.intel.comSigned-off-by:
-
Miaohe Lin authored
Use helper macro abs() to simplify the "x >= t || x <= -t" cmp. Signed-off-by:
Miaohe Lin <linmiaohe@huawei.com> Signed-off-by:
-
Christophe JAILLET authored
'sgl' is zeroed a few lines below in 'sg_init_table()'. There is no need to clear it twice. Remove the redundant initialization. Signed-off-by:
Christophe JAILLET <christophe.jaillet@wanadoo.fr> Signed-off-by:
-
Stephen Boyd authored
These two functions are deprecated. Users should call ida_alloc() or ida_free() respectively instead. Add documentation to this effect until the macro can be removed. Signed-off-by:
Stephen Boyd <swboyd@chromium.org> Signed-off-by:
Tri Vo <trong@android.com> Cc: Greg KH <gregkh@linuxfoundation.org> Cc: Jonathan Corbet <corbet@lwn.net> Cc: Matthew Wilcox <willy@infradead.org> Link: https://lkml.kernel.org/r/20200910055246.2297797-2-swboyd@chromium.orgSigned-off-by:
-
Stephen Boyd authored
The documentation for these functions indicates that callers don't need to hold a lock while calling them, but that documentation is only in one place under "IDA Usage". Let's state the same information on each IDA function so that it's clear what the calling context requires. Furthermore, let's document ida_simple_get() with the same information so that callers know how this API works. Signed-off-by:
Greg Kroah-Hartman <gregkh@linuxfoundation.org> Cc: Tri Vo <trong@android.com> Cc: Jonathan Corbet <corbet@lwn.net> Cc: Matthew Wilcox <willy@infradead.org> Link: https://lkml.kernel.org/r/20200910055246.2297797-1-swboyd@chromium.orgSigned-off-by:
-
Randy Dunlap authored
Fix typo/spello of "functions". Signed-off-by:
Randy Dunlap <rdunlap@infradead.org> Signed-off-by:
-
Randy Dunlap authored
Drop the repeated word "the". Signed-off-by:
-
Randy Dunlap authored
Drop the repeated word "the". Signed-off-by:
-
Randy Dunlap authored
Drop the repeated word "be". Signed-off-by:
-
Randy Dunlap authored
Drop the repeated word "the". Signed-off-by:
-
Randy Dunlap authored
Drop the repeated word "the". Fix spelling of "excess". Signed-off-by:
-
Randy Dunlap authored
Drop the repeated word "how". Signed-off-by:
-
Randy Dunlap authored
Drop the repeated word "the". Signed-off-by:
-
Randy Dunlap authored
Drop the repeated word "an". Signed-off-by:
-
Jarkko Sakkinen authored
Use @kernel.org address as the main communications end point. Update the corresponding M-entries and .mailmap (for git shortlog translation). Signed-off-by:
Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> Signed-off-by:
-
Joe Perches authored
MAINTAINERS files generally have no specific maintainer but are updated by individuals for subsystems all over the source tree. Exclude MAINTAINERS file(s) from --git-fallback searches so the unlucky individuals that update the files the most are not shown by default. Signed-off-by:
-
