| 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: <20160304215614.GB8104@treble.redhat.com>
Date: Fri, 4 Mar 2016 15:56:14 -0600
From: Josh Poimboeuf <jpoimboe@...hat.com>
To: Petr Mladek <pmladek@...e.com>
Cc: linuxppc-dev@...abs.org, Balbir Singh <bsingharora@...il.com>,
duwe@....de, linux-kernel@...r.kernel.org, rostedt@...dmis.org,
kamalesh@...ux.vnet.ibm.com, jeyu@...hat.com, jkosina@...e.cz,
live-patching@...r.kernel.org, mbenes@...e.cz,
Torsten Duwe <duwe@...e.de>
Subject: Re: [PATCH][v4] livepatch/ppc: Enable livepatching on powerpc
Hi Petr,
On Thu, Mar 03, 2016 at 05:52:01PM +0100, Petr Mladek wrote:
> From: Balbir Singh <bsingharora@...il.com>
>
> Changelog v4:
> 1. Renamed klp_matchaddr() to klp_get_ftrace_location()
> and used it just to convert the function address.
> 2. Synced klp_write_module_reloc() with s390(); made it
> inline, no error message, return -ENOSYS
> 3. Added an error message when including
> powerpc/include/asm/livepatch.h without HAVE_LIVEPATCH
> 4. Update some comments.
> Changelog v3:
> 1. Moved -ENOSYS to -EINVAL in klp_write_module_reloc
> 2. Moved klp_matchaddr to use ftrace_location_range
> Changelog v2:
> 1. Implement review comments by Michael
> 2. The previous version compared _NIP from the
> wrong location to check for whether we
> are going to a patched location
>
> This applies on top of the patches posted by Michael
> https://patchwork.ozlabs.org/patch/589791/
>
> It enables livepatching. This takes patch 6/8 and 7/8 of v8 as the base.
> (See the reference [1] below) and adds logic for checking offset ranges
> in livepatch with ftrace_location_range.
>
> I tested the sample in the livepatch
This commit message needs a rewrite. Most of the above information can
be moved to the diffstat section so it doesn't end up in the git log.
The log itself should be a straightforward description of the changes in
the patch itself.
> Signed-off-by: Torsten Duwe <duwe@...e.de>
> Signed-off-by: Balbir Singh <bsingharora@...il.com>
> Signed-off-by: Petr Mladek <pmladek@...e.com>
> ---
> arch/powerpc/Kconfig | 3 +++
> arch/powerpc/include/asm/livepatch.h | 47 +++++++++++++++++++++++++++++++++
> arch/powerpc/kernel/Makefile | 1 +
> arch/powerpc/kernel/entry_64.S | 50 ++++++++++++++++++++++++++++++++++++
> arch/powerpc/kernel/livepatch.c | 29 +++++++++++++++++++++
> include/linux/ftrace.h | 1 +
> include/linux/livepatch.h | 2 ++
> kernel/livepatch/core.c | 28 +++++++++++++++++---
> kernel/trace/ftrace.c | 14 +++++++++-
> 9 files changed, 171 insertions(+), 4 deletions(-)
> create mode 100644 arch/powerpc/include/asm/livepatch.h
> create mode 100644 arch/powerpc/kernel/livepatch.c
[...]
> diff --git a/arch/powerpc/kernel/livepatch.c b/arch/powerpc/kernel/livepatch.c
> new file mode 100644
> index 000000000000..267ce0fdd89b
> --- /dev/null
> +++ b/arch/powerpc/kernel/livepatch.c
> @@ -0,0 +1,29 @@
> +/*
> + * livepatch.c - powerpc-specific Kernel Live Patching Core
> + *
> + * Copyright (C) 2015 SUSE
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version 2
> + * of the License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, see <http://www.gnu.org/licenses/>.
> + */
> +#include <linux/ftrace.h>
> +#include <asm/livepatch.h>
> +
> +/*
> + * LivePatch works only with -mprofile-kernel on PPC. In this case,
> + * the ftrace location is always within the first 16 bytes.
> + */
No need for use CamelCase for LivePatch :-)
> +unsigned long klp_get_ftrace_location(unsigned long faddr)
> +{
> + return ftrace_location_range(faddr, faddr + 16);
> +}
> diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
> index c2b340e23f62..fb13cd3e68f9 100644
> --- a/include/linux/ftrace.h
> +++ b/include/linux/ftrace.h
> @@ -455,6 +455,7 @@ int ftrace_update_record(struct dyn_ftrace *rec, int enable);
> int ftrace_test_record(struct dyn_ftrace *rec, int enable);
> void ftrace_run_stop_machine(int command);
> unsigned long ftrace_location(unsigned long ip);
> +unsigned long ftrace_location_range(unsigned long start, unsigned long end);
> unsigned long ftrace_get_addr_new(struct dyn_ftrace *rec);
> unsigned long ftrace_get_addr_curr(struct dyn_ftrace *rec);
>
> diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h
> index a8828652f794..25a267bcb01f 100644
> --- a/include/linux/livepatch.h
> +++ b/include/linux/livepatch.h
> @@ -134,6 +134,8 @@ int klp_unregister_patch(struct klp_patch *);
> int klp_enable_patch(struct klp_patch *);
> int klp_disable_patch(struct klp_patch *);
>
> +unsigned long klp_get_ftrace_location(unsigned long faddr);
> +
> #endif /* CONFIG_LIVEPATCH */
>
> #endif /* _LINUX_LIVEPATCH_H_ */
> diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
> index bc2c85c064c1..9ad597faa57f 100644
> --- a/kernel/livepatch/core.c
> +++ b/kernel/livepatch/core.c
> @@ -298,22 +298,39 @@ unlock:
> rcu_read_unlock();
> }
>
> +/*
> + * Convert a function address into the appropriate ftrace location.
> + *
> + * The given address is returned on most architectures. LivePatching
> + * usually works only when the ftrace location is the first instruction
> + * in the function.
> + */
s/LivePatching/Live patching/
Otherwise the livepatch changes look good to me.
--
Josh
Powered by blists - more mailing lists