Re: [PATCH v3 08/12] treewide: Use initializer for struct vm_unmapped_area_info
On Tue, Mar 12, 2024 at 03:28:39PM -0700, Rick Edgecombe wrote: > So to be reduce the chance of bugs via uninitialized fields, perform a > tree wide change using the consensus for the best general way to do this > change. Use C99 static initializing to zero the struct and remove and > statements that simply set members to zero. > > Signed-off-by: Rick Edgecombe Thanks! This looks to do exactly what it describes. :) Reviewed-by: Kees Cook -- Kees Cook ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH v2 5/9] mm: Initialize struct vm_unmapped_area_info
On Sat, Mar 02, 2024 at 12:47:08AM +, Edgecombe, Rick P wrote:
> On Wed, 2024-02-28 at 09:21 -0800, Kees Cook wrote:
> > I totally understand. If the "uninitialized" warnings were actually
> > reliable, I would agree. I look at it this way:
> >
> > - initializations can be missed either in static initializers or via
> > run time initializers. (So the risk of mistake here is matched --
> > though I'd argue it's easier to *find* static initializers when
> > adding
> > new struct members.)
> > - uninitialized warnings are inconsistent (this becomes an unknown
> > risk)
> > - when a run time initializer is missed, the contents are whatever
> > was
> > on the stack (high risk)
> > - what a static initializer is missed, the content is 0 (low risk)
> >
> > I think unambiguous state (always 0) is significantly more important
> > for
> > the safety of the system as a whole. Yes, individual cases maybe bad
> > ("what uid should this be? root?!") but from a general memory safety
> > perspective the value doesn't become potentially influenced by order
> > of
> > operations, leftover stack memory, etc.
> >
> > I'd agree, lifting everything into a static initializer does seem
> > cleanest of all the choices.
>
> Hi Kees,
>
> Well, I just gave this a try. It is giving me flashbacks of when I last
> had to do a tree wide change that I couldn't fully test and the
> breakage was caught by Linus.
Yeah, testing isn't fun for these kinds of things. This is traditionally
why the "obviously correct" changes tend to have an easier time landing
(i.e. adding "= {}" to all of them).
> Could you let me know if you think this is additionally worthwhile
> cleanup outside of the guard gap improvements of this series? Because I
> was thinking a more cowardly approach could be a new vm_unmapped_area()
> variant that takes the new start gap member as a separate argument
> outside of struct vm_unmapped_area_info. It would be kind of strange to
> keep them separate, but it would be less likely to bump something.
I think you want a new member -- AIUI, that's what that struct is for.
Looking at this resulting set of patches, I do kinda think just adding
the "= {}" in a single patch is more sensible. Having to split things
that are know at the top of the function from the stuff known at the
existing initialization time is rather awkward.
Personally, I think a single patch that sets "= {}" for all of them and
drop the all the "= 0" or "= NULL" assignments would be the cleanest way
to go.
-Kees
--
Kees Cook
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH v2 5/9] mm: Initialize struct vm_unmapped_area_info
On Wed, Feb 28, 2024 at 01:22:09PM +, Christophe Leroy wrote:
> [...]
> My worry with initialisation at declaration is it often hides missing
> assignments. Let's take following simple exemple:
>
> char *colour(int num)
> {
> char *name;
>
> if (num == 0) {
> name = "black";
> } else if (num == 1) {
> name = "white";
> } else if (num == 2) {
> } else {
> name = "no colour";
> }
>
> return name;
> }
>
> Here, GCC warns about a missing initialisation of variable 'name'.
Sometimes. :( We build with -Wno-maybe-uninitialized because GCC gets
this wrong too often. Also, like with large structs like this, all
uninit warnings get suppressed if anything takes it by reference. So, if
before your "return name" statement above, you had something like:
do_something();
it won't warn with any option enabled.
> But if I declare it as
>
> char *name = "no colour";
>
> Then GCC won't warn anymore that we are missing a value for when num is 2.
>
> During my life I have so many times spent huge amount of time
> investigating issues and bugs due to missing assignments that were going
> undetected due to default initialisation at declaration.
I totally understand. If the "uninitialized" warnings were actually
reliable, I would agree. I look at it this way:
- initializations can be missed either in static initializers or via
run time initializers. (So the risk of mistake here is matched --
though I'd argue it's easier to *find* static initializers when adding
new struct members.)
- uninitialized warnings are inconsistent (this becomes an unknown risk)
- when a run time initializer is missed, the contents are whatever was
on the stack (high risk)
- what a static initializer is missed, the content is 0 (low risk)
I think unambiguous state (always 0) is significantly more important for
the safety of the system as a whole. Yes, individual cases maybe bad
("what uid should this be? root?!") but from a general memory safety
perspective the value doesn't become potentially influenced by order of
operations, leftover stack memory, etc.
I'd agree, lifting everything into a static initializer does seem
cleanest of all the choices.
-Kees
--
Kees Cook
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH v2 5/9] mm: Initialize struct vm_unmapped_area_info
On Tue, Feb 27, 2024 at 07:02:59AM +, Christophe Leroy wrote: > > > Le 26/02/2024 à 20:09, Rick Edgecombe a écrit : > > Future changes will need to add a field to struct vm_unmapped_area_info. > > This would cause trouble for any archs that don't initialize the > > struct. Currently every user sets each field, so if new fields are > > added, the core code parsing the struct will see garbage in the new > > field. > > > > It could be possible to initialize the new field for each arch to 0, but > > instead simply inialize the field with a C99 struct inializing syntax. > > Why doing a full init of the struct when all fields are re-written a few > lines after ? It's a nice change for robustness and makes future changes easier. It's not actually wasteful since the compiler will throw away all redundant stores. > If I take the exemple of powerpc function slice_find_area_bottomup(): > > struct vm_unmapped_area_info info; > > info.flags = 0; > info.length = len; > info.align_mask = PAGE_MASK & ((1ul << pshift) - 1); > info.align_offset = 0; But one cleanup that is possible from explicitly zero-initializing the whole structure would be dropping all the individual "= 0" assignments. :) -- Kees Cook ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
[PATCH 31/82] ARC: dw2 unwind: Refactor intentional wrap-around calculation
In an effort to separate intentional arithmetic wrap-around from
unexpected wrap-around, we need to refactor places that depend on this
kind of math. One of the most common code patterns of this is:
VAR + value < VAR
Notably, this is considered "undefined behavior" for signed and pointer
types, which the kernel works around by using the -fno-strict-overflow
option in the build[1] (which used to just be -fwrapv). Regardless, we
want to get the kernel source to the position where we can meaningfully
instrument arithmetic wrap-around conditions and catch them when they
are unexpected, regardless of whether they are signed[2], unsigned[3],
or pointer[4] types.
Refactor open-coded pointer wrap-around addition test to use
check_add_overflow(), retaining the result for later usage (which
removes the redundant open-coded addition). This paves the way to enabling
the unsigned wrap-around sanitizer[2] in the future.
Link: https://git.kernel.org/linus/68df3755e383e6fecf2354a67b08f92f18536594 [1]
Link: https://github.com/KSPP/linux/issues/26 [2]
Link: https://github.com/KSPP/linux/issues/27 [3]
Link: https://github.com/KSPP/linux/issues/344 [4]
Cc: Vineet Gupta
Cc: Luis Chamberlain
Cc: Song Liu
Cc: Yihao Han
Cc: Thomas Gleixner
Cc: "dean.yang_cp"
Cc: Jinchao Wang
Cc: linux-snps-arc@lists.infradead.org
Signed-off-by: Kees Cook
---
arch/arc/kernel/unwind.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/arch/arc/kernel/unwind.c b/arch/arc/kernel/unwind.c
index 9270d0a713c3..8924fa2a8f29 100644
--- a/arch/arc/kernel/unwind.c
+++ b/arch/arc/kernel/unwind.c
@@ -612,6 +612,7 @@ static signed fde_pointer_type(const u32 *cie)
const char *aug;
const u8 *end = (const u8 *)(cie + 1) + *cie;
uleb128_t len;
+ const u8 *sum;
/* check if augmentation size is first (and thus present) */
if (*ptr != 'z')
@@ -630,10 +631,10 @@ static signed fde_pointer_type(const u32 *cie)
version <= 1 ? (void) ++ptr : (void)get_uleb128(, end);
len = get_uleb128(, end); /* augmentation length */
- if (ptr + len < ptr || ptr + len > end)
+ if (check_add_overflow(ptr, len, ) || sum > end)
return -1;
- end = ptr + len;
+ end = sum;
while (*++aug) {
if (ptr >= end)
return -1;
--
2.34.1
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
[PATCH 74/82] ARC: dw2 unwind: Refactor intentional wrap-around test
In an effort to separate intentional arithmetic wrap-around from unexpected wrap-around, we need to refactor places that depend on this kind of math. One of the most common code patterns of this is: VAR + value < VAR Notably, this is considered "undefined behavior" for signed and pointer types, which the kernel works around by using the -fno-strict-overflow option in the build[1] (which used to just be -fwrapv). Regardless, we want to get the kernel source to the position where we can meaningfully instrument arithmetic wrap-around conditions and catch them when they are unexpected, regardless of whether they are signed[2], unsigned[3], or pointer[4] types. Refactor open-coded wrap-around addition test to use add_would_overflow(). This paves the way to enabling the wrap-around sanitizers in the future. Link: https://git.kernel.org/linus/68df3755e383e6fecf2354a67b08f92f18536594 [1] Link: https://github.com/KSPP/linux/issues/26 [2] Link: https://github.com/KSPP/linux/issues/27 [3] Link: https://github.com/KSPP/linux/issues/344 [4] Cc: Vineet Gupta Cc: Luis Chamberlain Cc: "dean.yang_cp" Cc: Song Liu Cc: Yihao Han Cc: linux-snps-arc@lists.infradead.org Signed-off-by: Kees Cook --- arch/arc/kernel/unwind.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arc/kernel/unwind.c b/arch/arc/kernel/unwind.c index 8924fa2a8f29..649b56204580 100644 --- a/arch/arc/kernel/unwind.c +++ b/arch/arc/kernel/unwind.c @@ -1278,7 +1278,7 @@ int arc_unwind(struct unwind_frame_info *frame) if ((state.regs[i].value * state.dataAlign) % sizeof(unsigned long) || addr < startLoc - || addr + sizeof(unsigned long) < addr + || add_would_overflow(addr, sizeof(unsigned long)) || addr + sizeof(unsigned long) > endLoc) return -EIO; -- 2.34.1 ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH v6 13/41] mm: Make pte_mkwrite() take a VMA
On Sat, Feb 18, 2023 at 01:14:05PM -0800, Rick Edgecombe wrote: > The x86 Control-flow Enforcement Technology (CET) feature includes a new > type of memory called shadow stack. This shadow stack memory has some > unusual properties, which requires some core mm changes to function > properly. > > One of these unusual properties is that shadow stack memory is writable, > but only in limited ways. These limits are applied via a specific PTE > bit combination. Nevertheless, the memory is writable, and core mm code > will need to apply the writable permissions in the typical paths that > call pte_mkwrite(). > > In addition to VM_WRITE, the shadow stack VMA's will have a flag denoting > that they are special shadow stack flavor of writable memory. So make > pte_mkwrite() take a VMA, so that the x86 implementation of it can know to > create regular writable memory or shadow stack memory. > > Apply the same changes for pmd_mkwrite() and huge_pte_mkwrite(). > > No functional change. > > Cc: linux-...@vger.kernel.org > Cc: linux-ker...@vger.kernel.org > Cc: linux-al...@vger.kernel.org > Cc: linux-snps-arc@lists.infradead.org > Cc: linux-arm-ker...@lists.infradead.org > Cc: linux-c...@vger.kernel.org > Cc: linux-hexa...@vger.kernel.org > Cc: linux-i...@vger.kernel.org > Cc: loonga...@lists.linux.dev > Cc: linux-m...@lists.linux-m68k.org > Cc: Michal Simek > Cc: Dinh Nguyen > Cc: linux-m...@vger.kernel.org > Cc: openr...@lists.librecores.org > Cc: linux-par...@vger.kernel.org > Cc: linuxppc-...@lists.ozlabs.org > Cc: linux-ri...@lists.infradead.org > Cc: linux-s...@vger.kernel.org > Cc: linux...@vger.kernel.org > Cc: sparcli...@vger.kernel.org > Cc: linux...@lists.infradead.org > Cc: xen-de...@lists.xenproject.org > Cc: linux-a...@vger.kernel.org > Cc: linux...@kvack.org > Tested-by: Pengfei Xu > Suggested-by: David Hildenbrand > Signed-off-by: Rick Edgecombe I'm not an arch maintainer, but it looks like a correct tree-wide refactor. Reviewed-by: Kees Cook -- Kees Cook ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH] treewide: defconfig: address renamed CONFIG_DEBUG_INFO=y
On Thu, Aug 11, 2022 at 01:44:34PM +0200, Arnd Bergmann wrote:
> From: Arnd Bergmann
>
> CONFIG_DEBUG_INFO is now implicitly selected if one picks one of the
> explicit options that could be DEBUG_INFO_DWARF_TOOLCHAIN_DEFAULT,
> DEBUG_INFO_DWARF4, DEBUG_INFO_DWARF5.
>
> This was actually not what I had in mind when I suggested making
> it a 'choice' statement, but it's too late to change again now,
> and the Kconfig logic is more sensible in the new form.
>
> Change any defconfig file that had CONFIG_DEBUG_INFO enabled
> but did not pick DWARF4 or DWARF5 explicitly to now pick the toolchain
> default.
>
> Fixes: f9b3cd245784 ("Kconfig.debug: make DEBUG_INFO selectable from a
> choice")
> Signed-off-by: Arnd Bergmann
Thanks!
Reviewed-by: Kees Cook
--
Kees Cook
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH] arc: Replace lkml.org links with lore
On Mon, Feb 15, 2021 at 09:03:54PM -0800, Vineet Gupta wrote:
> On 2/10/21 3:28 PM, Kees Cook wrote:
> > As started by commit 05a5f51ca566 ("Documentation: Replace lkml.org
> > links with lore"), replace lkml.org links with lore to better use a
> > single source that's more likely to stay available long-term.
> >
> > Signed-off-by: Kees Cook
>
> Acked-by: Vineet Gupta
>
> Let me know if you want me to pick this up.
Hi!
Oops, sorry, I never replied to this. Yes, can you pick this up please?
Thanks!
-Kees
--
Kees Cook
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH 2/2] kbuild: use more subdir- for visiting subdirectories while cleaning
On Wed, Oct 13, 2021 at 03:36:22PM +0900, Masahiro Yamada wrote:
> Documentation/kbuild/makefiles.rst suggests to use "archclean" for
> cleaning arch/$(SRCARCH)/boot/.
>
> Since commit d92cc4d51643 ("kbuild: require all architectures to have
> arch/$(SRCARCH)/Kbuild"), we can use the "subdir- += boot" trick for
> all architectures. This can take advantage of the parallel option (-j)
> for "make clean".
>
> I also cleaned up the comments. The "archdep" target does not exist.
>
> Signed-off-by: Masahiro Yamada
I like the clean-up!
Reviewed-by: Kees Cook
--
Kees Cook
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH] ARC: fix CONFIG_HARDENED_USERCOPY
On Thu, Jun 10, 2021 at 06:56:48PM +, Vineet Gupta wrote: > On 6/10/21 10:02 AM, Kees Cook wrote: > > On Wed, Jun 09, 2021 at 03:12:11PM -0700, Vineet Gupta wrote: > >> Currently enabling this triggers a warning > >> > >> | usercopy: Kernel memory overwrite attempt detected to kernel text > >> (offset 155633, size 11)! > >> | usercopy: BUG: failure at mm/usercopy.c:99/usercopy_abort()! > >> | > >> |gcc generated __builtin_trap > >> |Path: /bin/busybox > >> |CPU: 0 PID: 84 Comm: init Not tainted 5.4.22 > >> | > >> |[ECR ]: 0x00090005 => gcc generated __builtin_trap > >> |[EFA ]: 0x9024fcaa > >> |[BLINK ]: usercopy_abort+0x8a/0x8c > >> |[ERET ]: memfd_fcntl+0x0/0x470 > >> |[STAT32]: 0x80080802 : IE K > >> |BTA: 0x901ba38c SP: 0xbe161ecc FP: 0xbf9fe950 > >> |LPS: 0x90677408 LPE: 0x9067740c LPC: 0x > >> |r00: 0x003c r01: 0xbf0ed280 r02: 0x > >> |r03: 0xbe15fa30 r04: 0x00d2803e r05: 0x > >> |r06: 0x675d7000 r07: 0x r08: 0x675d9c00 > >> |r09: 0x r10: 0x035c r11: 0x61206572 > >> |r12: 0x9024fcaa r13: 0x000b r14: 0x000b > >> |r15: 0x r16: 0x90169ffc r17: 0x90168000 > >> |r18: 0x r19: 0xbf092010 r20: 0x0001 > >> |r21: 0x0011 r22: 0x5ff1 r23: 0x90169ff1 > >> |r24: 0xbe196c00 r25: 0xbf0ed280 > >> | > >> |Stack Trace: > >> | memfd_fcntl+0x0/0x470 > >> | usercopy_abort+0x8a/0x8c > >> | __check_object_size+0x10e/0x138 > >> | copy_strings+0x1f4/0x38c > >> | __do_execve_file+0x352/0x848 > >> | EV_Trap+0xcc/0xd0 > > What was the root cause here? Was it that the init section gets freed > > and reused for kmalloc? > > Right. ARC _stext was encompassing the init section (to cover the init > code) so when init gets freed and used by kmalloc, > check_kernel_text_object() trips as it thinks the allocated pointer is > in kernel .text. Actually I should have added this to changelog. Great! Yeah, if you respin it with that added, please consider it: Reviewed-by: Kees Cook Thanks! -- Kees Cook ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH] ARC: fix CONFIG_HARDENED_USERCOPY
On Wed, Jun 09, 2021 at 03:12:11PM -0700, Vineet Gupta wrote:
> Currently enabling this triggers a warning
>
> | usercopy: Kernel memory overwrite attempt detected to kernel text (offset
> 155633, size 11)!
> | usercopy: BUG: failure at mm/usercopy.c:99/usercopy_abort()!
> |
> |gcc generated __builtin_trap
> |Path: /bin/busybox
> |CPU: 0 PID: 84 Comm: init Not tainted 5.4.22
> |
> |[ECR ]: 0x00090005 => gcc generated __builtin_trap
> |[EFA ]: 0x9024fcaa
> |[BLINK ]: usercopy_abort+0x8a/0x8c
> |[ERET ]: memfd_fcntl+0x0/0x470
> |[STAT32]: 0x80080802 : IE K
> |BTA: 0x901ba38c SP: 0xbe161ecc FP: 0xbf9fe950
> |LPS: 0x90677408 LPE: 0x9067740c LPC: 0x
> |r00: 0x003c r01: 0xbf0ed280 r02: 0x
> |r03: 0xbe15fa30 r04: 0x00d2803e r05: 0x
> |r06: 0x675d7000 r07: 0x r08: 0x675d9c00
> |r09: 0x r10: 0x035c r11: 0x61206572
> |r12: 0x9024fcaa r13: 0x000b r14: 0x000b
> |r15: 0x r16: 0x90169ffc r17: 0x90168000
> |r18: 0x r19: 0xbf092010 r20: 0x0001
> |r21: 0x0011 r22: 0x5ff1 r23: 0x90169ff1
> |r24: 0xbe196c00 r25: 0xbf0ed280
> |
> |Stack Trace:
> | memfd_fcntl+0x0/0x470
> | usercopy_abort+0x8a/0x8c
> | __check_object_size+0x10e/0x138
> | copy_strings+0x1f4/0x38c
> | __do_execve_file+0x352/0x848
> | EV_Trap+0xcc/0xd0
What was the root cause here? Was it that the init section gets freed
and reused for kmalloc?
>
> Fixes: https://github.com/foss-for-synopsys-dwc-arc-processors/linux/issues/15
> Reported-by: Evgeniy Didin
> Signed-off-by: Vineet Gupta
> ---
> arch/arc/kernel/vmlinux.lds.S | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arc/kernel/vmlinux.lds.S b/arch/arc/kernel/vmlinux.lds.S
> index 33ce59d91461..e2146a8da195 100644
> --- a/arch/arc/kernel/vmlinux.lds.S
> +++ b/arch/arc/kernel/vmlinux.lds.S
> @@ -57,7 +57,6 @@ SECTIONS
> .init.ramfs : { INIT_RAM_FS }
>
> . = ALIGN(PAGE_SIZE);
> - _stext = .;
>
> HEAD_TEXT_SECTION
> INIT_TEXT_SECTION(L1_CACHE_BYTES)
> @@ -83,6 +82,7 @@ SECTIONS
>
> .text : {
> _text = .;
> + _stext = .;
> TEXT_TEXT
> SCHED_TEXT
> CPUIDLE_TEXT
> --
> 2.25.1
>
--
Kees Cook
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
[PATCH] arc: Replace lkml.org links with lore
As started by commit 05a5f51ca566 ("Documentation: Replace lkml.org
links with lore"), replace lkml.org links with lore to better use a
single source that's more likely to stay available long-term.
Signed-off-by: Kees Cook
---
arch/arc/include/asm/irqflags-compact.h | 8 ++--
arch/arc/mm/dma.c | 2 +-
arch/arc/plat-axs10x/axs10x.c | 2 +-
arch/arc/plat-hsdk/platform.c | 2 +-
4 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/arch/arc/include/asm/irqflags-compact.h
b/arch/arc/include/asm/irqflags-compact.h
index 863d63ad18d6..0d63e568d64c 100644
--- a/arch/arc/include/asm/irqflags-compact.h
+++ b/arch/arc/include/asm/irqflags-compact.h
@@ -50,8 +50,12 @@
* are redone after IRQs are re-enabled (and gcc doesn't reuse stale register)
*
* Noted at the time of Abilis Timer List corruption
- * Orig Bug + Rejected solution: https://lkml.org/lkml/2013/3/29/67
- * Reasoning : https://lkml.org/lkml/2013/4/8/15
+ *
+ * Orig Bug + Rejected solution:
+ *
https://lore.kernel.org/lkml/1364553218-31255-1-git-send-email-vgu...@synopsys.com
+ *
+ * Reasoning:
+ *
https://lore.kernel.org/lkml/ca+55afyfwjpsvqm6m266tkrg_zxjzz-nyejpmxyqxbrr42m...@mail.gmail.com
*
**/
diff --git a/arch/arc/mm/dma.c b/arch/arc/mm/dma.c
index 517988e60cfc..2a7fbbb83b70 100644
--- a/arch/arc/mm/dma.c
+++ b/arch/arc/mm/dma.c
@@ -32,7 +32,7 @@ void arch_dma_prep_coherent(struct page *page, size_t size)
/*
* Cache operations depending on function and direction argument, inspired by
- * https://lkml.org/lkml/2018/5/18/979
+ * https://lore.kernel.org/lkml/20180518175004.gf17...@n2100.armlinux.org.uk
* "dma_sync_*_for_cpu and direction=TO_DEVICE (was Re: [PATCH 02/20]
* dma-mapping: provide a generic dma-noncoherent implementation)"
*
diff --git a/arch/arc/plat-axs10x/axs10x.c b/arch/arc/plat-axs10x/axs10x.c
index 63ea5a606ecd..b821df7b0089 100644
--- a/arch/arc/plat-axs10x/axs10x.c
+++ b/arch/arc/plat-axs10x/axs10x.c
@@ -50,7 +50,7 @@ static void __init axs10x_enable_gpio_intc_wire(void)
* Current implementation of "irq-dw-apb-ictl" driver doesn't work well
* with stacked INTCs. In particular problem happens if its master INTC
* not yet instantiated. See discussion here -
-* https://lkml.org/lkml/2015/3/4/755
+* https://lore.kernel.org/lkml/54f6fe2c.7020...@synopsys.com
*
* So setup the first gpio block as a passive pass thru and hide it from
* DT hardware topology - connect MB intc directly to cpu intc
diff --git a/arch/arc/plat-hsdk/platform.c b/arch/arc/plat-hsdk/platform.c
index b3ea1fa11f87..c4a875b22352 100644
--- a/arch/arc/plat-hsdk/platform.c
+++ b/arch/arc/plat-hsdk/platform.c
@@ -52,7 +52,7 @@ static void __init hsdk_enable_gpio_intc_wire(void)
* Current implementation of "irq-dw-apb-ictl" driver doesn't work well
* with stacked INTCs. In particular problem happens if its master INTC
* not yet instantiated. See discussion here -
-* https://lkml.org/lkml/2015/3/4/755
+* https://lore.kernel.org/lkml/54f6fe2c.7020...@synopsys.com
*
* So setup the first gpio block as a passive pass thru and hide it from
* DT hardware topology - connect intc directly to cpu intc
--
2.25.1
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH 17/17] arch: rename copy_thread_tls() back to copy_thread()
On Tue, Jun 23, 2020 at 01:43:26AM +0200, Christian Brauner wrote: > Now that HAVE_COPY_THREAD_TLS has been removed, rename copy_thread_tls() > back simply copy_thread(). It's a simpler name, and doesn't imply that only > tls is copied here. This finishes an outstanding chunk of internal process > creation work since we've added clone3(). > [...] > -copy_thread_tls(unsigned long clone_flags, unsigned long user_stack_base, > +copy_thread(unsigned long clone_flags, unsigned long user_stack_base, > unsigned long user_stack_size, struct task_struct *p, > unsigned long tls) Maybe clean up the arg indentation too? I'm not sure how strongly people feel about that, but I think it'd be nice. Either way: Reviewed-by: Kees Cook -- Kees Cook ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH 16/17] arch: remove HAVE_COPY_THREAD_TLS
On Tue, Jun 23, 2020 at 01:43:25AM +0200, Christian Brauner wrote: > All architectures support copy_thread_tls() now, so remove the legacy > copy_thread() function and the HAVE_COPY_THREAD_TLS config option. Everyone > uses the same process creation calling convention based on > copy_thread_tls() and struct kernel_clone_args. This will make it easier to > maintain the core process creation code under kernel/, simplifies the > callpaths and makes the identical for all architectures. > [...] > Signed-off-by: Christian Brauner Massive CC list. ;) linux-arch@ may be sufficient. Reviewed-by: Kees Cook -- Kees Cook ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH] ARC: unwind: Mark expected switch fall-throughs
On Mon, Aug 05, 2019 at 02:32:32PM -0500, Gustavo A. R. Silva wrote:
> Mark switch cases where we are expecting to fall through.
>
> This patch fixes the following warnings (Building: haps_hs_defconfig arc):
>
> arch/arc/kernel/unwind.c:827:20: warning: this statement may fall through
> [-Wimplicit-fallthrough=]
> arch/arc/kernel/unwind.c:836:20: warning: this statement may fall through
> [-Wimplicit-fallthrough=]
>
> Signed-off-by: Gustavo A. R. Silva
Reviewed-by: Kees Cook
-Kees
> ---
> arch/arc/kernel/unwind.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arc/kernel/unwind.c b/arch/arc/kernel/unwind.c
> index c2663fce7f6c..445e4d702f43 100644
> --- a/arch/arc/kernel/unwind.c
> +++ b/arch/arc/kernel/unwind.c
> @@ -826,7 +826,7 @@ static int processCFI(const u8 *start, const u8 *end,
> unsigned long targetLoc,
> case DW_CFA_def_cfa:
> state->cfa.reg = get_uleb128(, end);
> unw_debug("cfa_def_cfa: r%lu ", state->cfa.reg);
> - /*nobreak*/
> + /* fall through */
> case DW_CFA_def_cfa_offset:
> state->cfa.offs = get_uleb128(, end);
> unw_debug("cfa_def_cfa_offset: 0x%lx ",
> @@ -834,7 +834,7 @@ static int processCFI(const u8 *start, const u8 *end,
> unsigned long targetLoc,
> break;
> case DW_CFA_def_cfa_sf:
> state->cfa.reg = get_uleb128(, end);
> - /*nobreak */
> + /* fall through */
> case DW_CFA_def_cfa_offset_sf:
> state->cfa.offs = get_sleb128(, end)
> * state->dataAlign;
> --
> 2.22.0
>
--
Kees Cook
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [RFC PATCH v2 0/2] Randomization of address chosen by mmap.
On Tue, Mar 27, 2018 at 6:51 AM, Ilya Smith <blackz...@gmail.com> wrote: > >> On 27 Mar 2018, at 10:24, Michal Hocko <mho...@kernel.org> wrote: >> >> On Mon 26-03-18 22:45:31, Ilya Smith wrote: >>> >>>> On 26 Mar 2018, at 11:46, Michal Hocko <mho...@kernel.org> wrote: >>>> >>>> On Fri 23-03-18 20:55:49, Ilya Smith wrote: >>>>> >>>>>> On 23 Mar 2018, at 15:48, Matthew Wilcox <wi...@infradead.org> wrote: >>>>>> >>>>>> On Thu, Mar 22, 2018 at 07:36:36PM +0300, Ilya Smith wrote: >>>>>>> Current implementation doesn't randomize address returned by mmap. >>>>>>> All the entropy ends with choosing mmap_base_addr at the process >>>>>>> creation. After that mmap build very predictable layout of address >>>>>>> space. It allows to bypass ASLR in many cases. This patch make >>>>>>> randomization of address on any mmap call. >>>>>> >>>>>> Why should this be done in the kernel rather than libc? libc is >>>>>> perfectly >>>>>> capable of specifying random numbers in the first argument of mmap. >>>>> Well, there is following reasons: >>>>> 1. It should be done in any libc implementation, what is not possible IMO; >>>> >>>> Is this really so helpful? >>> >>> Yes, ASLR is one of very important mitigation techniques which are really >>> used >>> to protect applications. If there is no ASLR, it is very easy to exploit >>> vulnerable application and compromise the system. We can’t just fix all the >>> vulnerabilities right now, thats why we have mitigations - techniques which >>> are >>> makes exploitation more hard or impossible in some cases. >>> >>> Thats why it is helpful. >> >> I am not questioning ASLR in general. I am asking whether we really need >> per mmap ASLR in general. I can imagine that some environments want to >> pay the additional price and other side effects, but considering this >> can be achieved by libc, why to add more code to the kernel? > > I believe this is the only one right place for it. Adding these 200+ lines of > code we give this feature for any user - on desktop, on server, on IoT device, > on SCADA, etc. But if only glibc will implement ‘user-mode-aslr’ IoT and SCADA > devices will never get it. I agree: pushing this off to libc leaves a lot of things unprotected. I think this should live in the kernel. The question I have is about making it maintainable/readable/etc. The state-of-the-art for ASLR is moving to finer granularity (over just base-address offset), so I'd really like to see this supported in the kernel. We'll be getting there for other things in the future, and I'd like to have a working production example for researchers to study, etc. -Kees -- Kees Cook Pixel Security ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
