| 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
| ||
|
Message-ID: <CAKxVwgdKjwPMkCtz+MDuCuf7L+pcRDYsUYd8xPFYTQBrTqC6Jw@mail.gmail.com>
Date: Tue, 5 Aug 2025 17:29:26 +0800
From: Miao Chen <chenmiao.ku@...il.com>
To: Stafford Horne <shorne@...il.com>
Cc: Jonas Bonn <jonas@...thpole.se>,
Stefan Kristiansson <stefan.kristiansson@...nalahti.fi>, Arnd Bergmann <arnd@...db.de>,
"Mike Rapoport (Microsoft)" <rppt@...nel.org>, Geert Uytterhoeven <geert@...ux-m68k.org>,
Luis Chamberlain <mcgrof@...nel.org>, Sahil Siddiq <sahilcdq0@...il.com>,
Johannes Berg <johannes@...solutions.net>, Masahiro Yamada <masahiroy@...nel.org>,
Dave Hansen <dave.hansen@...ux.intel.com>, Andrew Morton <akpm@...ux-foundation.org>,
"open list:OPENRISC ARCHITECTURE" <linux-openrisc@...r.kernel.org>, open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/2] openrisc: Add text patching API support
I also considered this approach. In the initial design, there was a
size_t len parameter, but since OpenRISC doesn't support it yet, it
was removed to only support single instructions. If there are plans to
support modifying multiple instructions in the future, we can simply
add size_t lento this API and replace OPENRISC_INSN_SIZE with len.
That's why I didn't change it to unsigned long and kept it as const
void*, while using OPENRISC_INSN_SIZE to fix the length of a single
instruction.
Stafford Horne <shorne@...il.com> 于2025年8月5日周二 17:15写道：
>
> On Tue, Aug 05, 2025 at 08:40:57AM +0000, ChenMiao wrote:
> > From: chenmiao <chenmiao.ku@...il.com>
> >
> > We need a text patching mechanism to ensure that in the subsequent
> > implementation of jump_label, the code can be modified to the correct
> > location. Therefore, FIX_TEXT_POKE0 has been added as a mapping area.
> >
> > And, I create a new file named insn-def.h to define the or1k insn macro
> > size and more define in the future.
> >
> > Among these changes, we implement patch_map and support the
> > patch_insn_write API for single instruction writing.
> >
> > Signed-off-by: chenmiao <chenmiao.ku@...il.com>
> > ---
> > arch/openrisc/include/asm/Kbuild | 1 -
> > arch/openrisc/include/asm/fixmap.h | 1 +
> > arch/openrisc/include/asm/insn-def.h | 12 ++++
> > arch/openrisc/include/asm/text-patching.h | 13 ++++
> > arch/openrisc/kernel/Makefile | 1 +
> > arch/openrisc/kernel/patching.c | 79 +++++++++++++++++++++++
> > arch/openrisc/mm/init.c | 2 +-
> > 7 files changed, 107 insertions(+), 2 deletions(-)
> > create mode 100644 arch/openrisc/include/asm/insn-def.h
> > create mode 100644 arch/openrisc/include/asm/text-patching.h
> > create mode 100644 arch/openrisc/kernel/patching.c
> >
> > diff --git a/arch/openrisc/include/asm/Kbuild b/arch/openrisc/include/asm/Kbuild
> > index 2b1a6b00cdac..cef49d60d74c 100644
> > --- a/arch/openrisc/include/asm/Kbuild
> > +++ b/arch/openrisc/include/asm/Kbuild
> > @@ -9,4 +9,3 @@ generic-y += spinlock.h
> > generic-y += qrwlock_types.h
> > generic-y += qrwlock.h
> > generic-y += user.h
> > -generic-y += text-patching.h
> > diff --git a/arch/openrisc/include/asm/fixmap.h b/arch/openrisc/include/asm/fixmap.h
> > index aaa6a26a3e92..74000215064d 100644
> > --- a/arch/openrisc/include/asm/fixmap.h
> > +++ b/arch/openrisc/include/asm/fixmap.h
> > @@ -28,6 +28,7 @@
> >
> > enum fixed_addresses {
> > FIX_EARLYCON_MEM_BASE,
> > + FIX_TEXT_POKE0,
> > __end_of_fixed_addresses
> > };
> >
> > diff --git a/arch/openrisc/include/asm/insn-def.h b/arch/openrisc/include/asm/insn-def.h
> > new file mode 100644
> > index 000000000000..dc8d16db1579
> > --- /dev/null
> > +++ b/arch/openrisc/include/asm/insn-def.h
> > @@ -0,0 +1,12 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +/*
> > + * Copyright (C) 2025 Chen Miao
> > + */
> > +
> > +#ifndef __ASM_INSN_DEF_H
> > +#define __ASM_INSN_DEF_H
> > +
> > +/* or1k instructions are always 32 bits. */
> > +#define OPENRISC_INSN_SIZE 4
> > +
> > +#endif /* __ASM_INSN_DEF_H */
> > diff --git a/arch/openrisc/include/asm/text-patching.h b/arch/openrisc/include/asm/text-patching.h
> > new file mode 100644
> > index 000000000000..4c3d8a6a906a
> > --- /dev/null
> > +++ b/arch/openrisc/include/asm/text-patching.h
> > @@ -0,0 +1,13 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +/*
> > + * Copyright (C) 2025 Chen Miao
> > + */
> > +
> > +#ifndef _ASM_PATCHING_H_
> > +#define _ASM_PATCHING_H_
> > +
> > +#include <linux/types.h>
> > +
> > +int patch_insn_write(void *addr, const void *insn);
> > +
> > +#endif /* _ASM_PATCHING_H_ */
> > diff --git a/arch/openrisc/kernel/Makefile b/arch/openrisc/kernel/Makefile
> > index 58e6a1b525b7..f0957ce16d6b 100644
> > --- a/arch/openrisc/kernel/Makefile
> > +++ b/arch/openrisc/kernel/Makefile
> > @@ -13,5 +13,6 @@ obj-$(CONFIG_SMP) += smp.o sync-timer.o
> > obj-$(CONFIG_STACKTRACE) += stacktrace.o
> > obj-$(CONFIG_MODULES) += module.o
> > obj-$(CONFIG_OF) += prom.o
> > +obj-y += patching.o
> >
> > clean:
> > diff --git a/arch/openrisc/kernel/patching.c b/arch/openrisc/kernel/patching.c
> > new file mode 100644
> > index 000000000000..2b979a0bc584
> > --- /dev/null
> > +++ b/arch/openrisc/kernel/patching.c
> > @@ -0,0 +1,79 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/* Copyright (C) 2020 SiFive
> > + * Copyright (C) 2025 Chen Miao
> > + */
> > +
> > +#include <linux/mm.h>
> > +#include <linux/kernel.h>
> > +#include <linux/spinlock.h>
> > +#include <linux/uaccess.h>
> > +
> > +#include <asm/insn-def.h>
> > +#include <asm/cacheflush.h>
> > +#include <asm/page.h>
> > +#include <asm/fixmap.h>
> > +#include <asm/text-patching.h>
> > +#include <asm/sections.h>
> > +
> > +static DEFINE_RAW_SPINLOCK(patch_lock);
> > +
> > +static inline bool is_exit_text(uintptr_t addr)
> > +{
> > + /* Now Have NO Mechanism to do */
> > + return true;
> > +}
> > +
> > +static __always_inline void *patch_map(void *addr, int fixmap)
> > +{
> > + uintptr_t uaddr = (uintptr_t) addr;
> > + phys_addr_t phys;
> > +
> > + if (core_kernel_text(uaddr) || is_exit_text(uaddr)) {
> > + phys = __pa_symbol(addr);
> > + } else {
> > + struct page *page = vmalloc_to_page(addr);
> > + BUG_ON(!page);
> > + phys = page_to_phys(page) + offset_in_page(addr);
> > + }
> > +
> > + return (void *)set_fixmap_offset(fixmap, phys);
> > +}
> > +
> > +static void patch_unmap(int fixmap)
> > +{
> > + clear_fixmap(fixmap);
> > +}
> > +
> > +static int __patch_insn_write(void *addr, const void *insn)
> > +{
> > + void *waddr = addr;
> > + unsigned long flags = 0;
> > + int ret;
> > +
> > + raw_spin_lock_irqsave(&patch_lock, flags);
> > +
> > + waddr = patch_map(addr, FIX_TEXT_POKE0);
> > +
> > + ret = copy_to_kernel_nofault(waddr, insn, OPENRISC_INSN_SIZE);
>
> If you change *insn to unsigned long insn, you can do:
>
> ret = copy_to_kernel_nofault(waddr, &insn, iszeof(insn));
>
> > + local_icache_range_inv((unsigned long)waddr,
> > + (unsigned long)waddr + OPENRISC_INSN_SIZE);
> > +
> > + patch_unmap(FIX_TEXT_POKE0);
> > +
> > + raw_spin_unlock_irqrestore(&patch_lock, flags);
> > +
> > + return ret;
> > +}
> > +
> > +int patch_insn_write(void *addr, const void *insn)
>
> Does insn need to be void *? It think it could be just unsigned long. See
> comment above.
>
> > +{
> > + u32 *tp = addr;
> > + int ret;
> > +
> > + if ((uintptr_t) tp & 0x3)
> > + return -EINVAL;
> > +
> > + ret = __patch_insn_write(tp, insn);
> > +
> > + return ret;
> > +}
> > diff --git a/arch/openrisc/mm/init.c b/arch/openrisc/mm/init.c
> > index e4904ca6f0a0..b5925710f954 100644
> > --- a/arch/openrisc/mm/init.c
> > +++ b/arch/openrisc/mm/init.c
> > @@ -226,7 +226,7 @@ static int __init map_page(unsigned long va, phys_addr_t pa, pgprot_t prot)
> > return 0;
> > }
> >
> > -void __init __set_fixmap(enum fixed_addresses idx,
> > +void __set_fixmap(enum fixed_addresses idx,
> > phys_addr_t phys, pgprot_t prot)
> > {
> > unsigned long address = __fix_to_virt(idx);
> > --
> > 2.45.2
> >
Powered by blists - more mailing lists