LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH v2 8/8] powerpc/bpf: Reallocate BPF registers to volatile registers when possible on PPC32
From: Christophe Leroy @ 2022-01-10 12:13 UTC (permalink / raw)
  To: Naveen N. Rao, andrii@kernel.org, ast@kernel.org,
	Benjamin Herrenschmidt, daniel@iogearbox.net,
	john.fastabend@gmail.com, kafai@fb.com, kpsingh@chromium.org,
	Michael Ellerman, Paul Mackerras, sandipan@linux.ibm.com,
	songliubraving@fb.com, yhs@fb.com
  Cc: netdev@vger.kernel.org, bpf@vger.kernel.org,
	linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org
In-Reply-To: <1641556157.ms6rd82ggh.naveen@linux.ibm.com>



Le 07/01/2022 à 12:51, Naveen N. Rao a écrit :
> Christophe Leroy wrote:
>> When the BPF routine doesn't call any function, the non volatile
>> registers can be reallocated to volatile registers in order to
>> avoid having to save them/restore on the stack.
>>
>> Before this patch, the test #359 ADD default X is:
>>
>>    0:    7c 64 1b 78     mr      r4,r3
>>    4:    38 60 00 00     li      r3,0
>>    8:    94 21 ff b0     stwu    r1,-80(r1)
>>    c:    60 00 00 00     nop
>>   10:    92 e1 00 2c     stw     r23,44(r1)
>>   14:    93 01 00 30     stw     r24,48(r1)
>>   18:    93 21 00 34     stw     r25,52(r1)
>>   1c:    93 41 00 38     stw     r26,56(r1)
>>   20:    39 80 00 00     li      r12,0
>>   24:    39 60 00 00     li      r11,0
>>   28:    3b 40 00 00     li      r26,0
>>   2c:    3b 20 00 00     li      r25,0
>>   30:    7c 98 23 78     mr      r24,r4
>>   34:    7c 77 1b 78     mr      r23,r3
>>   38:    39 80 00 42     li      r12,66
>>   3c:    39 60 00 00     li      r11,0
>>   40:    7d 8c d2 14     add     r12,r12,r26
>>   44:    39 60 00 00     li      r11,0
>>   48:    7d 83 63 78     mr      r3,r12
>>   4c:    82 e1 00 2c     lwz     r23,44(r1)
>>   50:    83 01 00 30     lwz     r24,48(r1)
>>   54:    83 21 00 34     lwz     r25,52(r1)
>>   58:    83 41 00 38     lwz     r26,56(r1)
>>   5c:    38 21 00 50     addi    r1,r1,80
>>   60:    4e 80 00 20     blr
>>
>> After this patch, the same test has become:
>>
>>    0:    7c 64 1b 78     mr      r4,r3
>>    4:    38 60 00 00     li      r3,0
>>    8:    94 21 ff b0     stwu    r1,-80(r1)
>>    c:    60 00 00 00     nop
>>   10:    39 80 00 00     li      r12,0
>>   14:    39 60 00 00     li      r11,0
>>   18:    39 00 00 00     li      r8,0
>>   1c:    38 e0 00 00     li      r7,0
>>   20:    7c 86 23 78     mr      r6,r4
>>   24:    7c 65 1b 78     mr      r5,r3
>>   28:    39 80 00 42     li      r12,66
>>   2c:    39 60 00 00     li      r11,0
>>   30:    7d 8c 42 14     add     r12,r12,r8
>>   34:    39 60 00 00     li      r11,0
>>   38:    7d 83 63 78     mr      r3,r12
>>   3c:    38 21 00 50     addi    r1,r1,80
>>   40:    4e 80 00 20     blr
>>
>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
>> ---
>>  arch/powerpc/net/bpf_jit.h        | 16 ++++++++++++++++
>>  arch/powerpc/net/bpf_jit64.h      |  2 +-
>>  arch/powerpc/net/bpf_jit_comp.c   |  2 ++
>>  arch/powerpc/net/bpf_jit_comp32.c | 30 ++++++++++++++++++++++++++++--
>>  arch/powerpc/net/bpf_jit_comp64.c |  4 ++++
>>  5 files changed, 51 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/powerpc/net/bpf_jit.h b/arch/powerpc/net/bpf_jit.h
>> index a45b8266355d..776abef4d2a0 100644
>> --- a/arch/powerpc/net/bpf_jit.h
>> +++ b/arch/powerpc/net/bpf_jit.h
>> @@ -116,6 +116,15 @@ static inline bool is_nearbranch(int offset)
>>  #define SEEN_STACK    0x40000000 /* uses BPF stack */
>>  #define SEEN_TAILCALL    0x80000000 /* uses tail calls */
>>
>> +#define SEEN_VREG_MASK    0x1ff80000 /* Volatile registers r3-r12 */
>> +#define SEEN_NVREG_MASK    0x0003ffff /* Non volatile registers 
>> r14-r31 */
>> +
>> +#ifdef CONFIG_PPC64
>> +extern const int b2p[MAX_BPF_JIT_REG + 2];
>> +#else
>> +extern const int b2p[MAX_BPF_JIT_REG + 1];
>> +#endif
>> +
>>  struct codegen_context {
>>      /*
>>       * This is used to track register usage as well
>> @@ -129,6 +138,7 @@ struct codegen_context {
>>      unsigned int seen;
>>      unsigned int idx;
>>      unsigned int stack_size;
>> +    int b2p[ARRAY_SIZE(b2p)];
>>  };
>>
>>  static inline void bpf_flush_icache(void *start, void *end)
>> @@ -147,11 +157,17 @@ static inline void bpf_set_seen_register(struct 
>> codegen_context *ctx, int i)
>>      ctx->seen |= 1 << (31 - i);
>>  }
>>
>> +static inline void bpf_clear_seen_register(struct codegen_context 
>> *ctx, int i)
>> +{
>> +    ctx->seen &= ~(1 << (31 - i));
>> +}
>> +
>>  void bpf_jit_emit_func_call_rel(u32 *image, struct codegen_context 
>> *ctx, u64 func);
>>  int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, struct 
>> codegen_context *ctx,
>>                 u32 *addrs, bool extra_pass);
>>  void bpf_jit_build_prologue(u32 *image, struct codegen_context *ctx);
>>  void bpf_jit_build_epilogue(u32 *image, struct codegen_context *ctx);
>> +void bpf_jit_realloc_regs(struct codegen_context *ctx);
>>
>>  #endif
>>
>> diff --git a/arch/powerpc/net/bpf_jit64.h b/arch/powerpc/net/bpf_jit64.h
>> index b05f2e67bba1..7b713edfa7e2 100644
>> --- a/arch/powerpc/net/bpf_jit64.h
>> +++ b/arch/powerpc/net/bpf_jit64.h
>> @@ -39,7 +39,7 @@
>>  #define TMP_REG_2    (MAX_BPF_JIT_REG + 1)
>>
>>  /* BPF to ppc register mappings */
>> -static const int b2p[] = {
>> +const int b2p[MAX_BPF_JIT_REG + 2] = {
>>      /* function return value */
>>      [BPF_REG_0] = 8,
>>      /* function arguments */
>> diff --git a/arch/powerpc/net/bpf_jit_comp.c 
>> b/arch/powerpc/net/bpf_jit_comp.c
>> index efac89964873..798ac4350a82 100644
>> --- a/arch/powerpc/net/bpf_jit_comp.c
>> +++ b/arch/powerpc/net/bpf_jit_comp.c
>> @@ -143,6 +143,7 @@ struct bpf_prog *bpf_int_jit_compile(struct 
>> bpf_prog *fp)
>>      }
>>
>>      memset(&cgctx, 0, sizeof(struct codegen_context));
>> +    memcpy(cgctx.b2p, b2p, sizeof(cgctx.b2p));
>>
>>      /* Make sure that the stack is quadword aligned. */
>>      cgctx.stack_size = round_up(fp->aux->stack_depth, 16);
>> @@ -167,6 +168,7 @@ struct bpf_prog *bpf_int_jit_compile(struct 
>> bpf_prog *fp)
>>          }
>>      }
>>
>> +    bpf_jit_realloc_regs(&cgctx);
>>      /*
>>       * Pretend to build prologue, given the features we've seen.  
>> This will
>>       * update ctgtx.idx as it pretends to output instructions, then 
>> we can
>> diff --git a/arch/powerpc/net/bpf_jit_comp32.c 
>> b/arch/powerpc/net/bpf_jit_comp32.c
>> index 29ce802d7534..003843273b43 100644
>> --- a/arch/powerpc/net/bpf_jit_comp32.c
>> +++ b/arch/powerpc/net/bpf_jit_comp32.c
>> @@ -37,7 +37,7 @@
>>  #define TMP_REG    (MAX_BPF_JIT_REG + 0)
>>
>>  /* BPF to ppc register mappings */
>> -static const int b2p[] = {
>> +const int b2p[MAX_BPF_JIT_REG + 1] = {
>>      /* function return value */
>>      [BPF_REG_0] = 12,
>>      /* function arguments */
>> @@ -60,7 +60,7 @@ static const int b2p[] = {
>>
>>  static int bpf_to_ppc(struct codegen_context *ctx, int reg)
>>  {
>> -    return b2p[reg];
>> +    return ctx->b2p[reg];
>>  }
>>
>>  /* PPC NVR range -- update this if we ever use NVRs below r17 */
>> @@ -77,6 +77,32 @@ static int bpf_jit_stack_offsetof(struct 
>> codegen_context *ctx, int reg)
>>      return BPF_PPC_STACKFRAME(ctx) - 4;
>>  }
>>
>> +void bpf_jit_realloc_regs(struct codegen_context *ctx)
>> +{
>> +    if (ctx->seen & SEEN_FUNC)
>> +        return;
> 
> Can't you remap BPF_REG_5, BPF_REG_AX and TMP_REG regardless of SEEN_FUNC?
> 

Oh yes, we can do that.

BPF_REG_5 is unlikely to be used unless BPF_REG_0 to 4 are used, so I 
guess we won't have any volatile register available.

BPF_REG_AX, I wasn't sure but it is a volatile register on PPC64 so I 
guess it is OK.

TMP_REG for sure can be reallocated to a volatile reg when one is available.

I'll send a patch for that.

Thanks
Christophe

^ permalink raw reply

* [PATCH] powerpc/bpf: Always reallocate BPF_REG_5, BPF_REG_AX and TMP_REG when possible
From: Christophe Leroy @ 2022-01-10 12:29 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: Naveen N . Rao, linuxppc-dev@lists.ozlabs.org,
	linux-kernel@vger.kernel.org

BPF_REG_5, BPF_REG_AX and TMP_REG are mapped on non volatile registers
because there are not enough volatile registers, but they don't need
to be preserved on function calls.

So when some volatile registers become available, those registers can
always be reallocated regardless of whether SEEN_FUNC is set or not.

Suggested-by: Naveen N. Rao <naveen.n.rao@linux.ibm.com>
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 arch/powerpc/net/bpf_jit.h        |  3 ---
 arch/powerpc/net/bpf_jit_comp32.c | 14 +++++++++++---
 2 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/arch/powerpc/net/bpf_jit.h b/arch/powerpc/net/bpf_jit.h
index b20a2a83a6e7..b75507fc8f6b 100644
--- a/arch/powerpc/net/bpf_jit.h
+++ b/arch/powerpc/net/bpf_jit.h
@@ -127,9 +127,6 @@
 #define SEEN_FUNC	0x20000000 /* might call external helpers */
 #define SEEN_TAILCALL	0x40000000 /* uses tail calls */
 
-#define SEEN_VREG_MASK	0x1ff80000 /* Volatile registers r3-r12 */
-#define SEEN_NVREG_MASK	0x0003ffff /* Non volatile registers r14-r31 */
-
 #ifdef CONFIG_PPC64
 extern const int b2p[MAX_BPF_JIT_REG + 2];
 #else
diff --git a/arch/powerpc/net/bpf_jit_comp32.c b/arch/powerpc/net/bpf_jit_comp32.c
index d3a52cd42f53..cfec42c8a511 100644
--- a/arch/powerpc/net/bpf_jit_comp32.c
+++ b/arch/powerpc/net/bpf_jit_comp32.c
@@ -77,14 +77,22 @@ static int bpf_jit_stack_offsetof(struct codegen_context *ctx, int reg)
 	return BPF_PPC_STACKFRAME(ctx) - 4;
 }
 
+#define SEEN_VREG_MASK		0x1ff80000 /* Volatile registers r3-r12 */
+#define SEEN_NVREG_FULL_MASK	0x0003ffff /* Non volatile registers r14-r31 */
+#define SEEN_NVREG_TEMP_MASK	0x00001e01 /* BPF_REG_5, BPF_REG_AX, TMP_REG */
+
 void bpf_jit_realloc_regs(struct codegen_context *ctx)
 {
+	unsigned int nvreg_mask;
+
 	if (ctx->seen & SEEN_FUNC)
-		return;
+		nvreg_mask = SEEN_NVREG_TEMP_MASK;
+	else
+		nvreg_mask = SEEN_NVREG_FULL_MASK;
 
-	while (ctx->seen & SEEN_NVREG_MASK &&
+	while (ctx->seen & nvreg_mask &&
 	      (ctx->seen & SEEN_VREG_MASK) != SEEN_VREG_MASK) {
-		int old = 32 - fls(ctx->seen & (SEEN_NVREG_MASK & 0xaaaaaaab));
+		int old = 32 - fls(ctx->seen & (nvreg_mask & 0xaaaaaaab));
 		int new = 32 - fls(~ctx->seen & (SEEN_VREG_MASK & 0xaaaaaaaa));
 		int i;
 
-- 
2.33.1

^ permalink raw reply related

* Re: [PATCH v5] powerpc/pseries: read the lpar name from the firmware
From: kernel test robot @ 2022-01-10 13:33 UTC (permalink / raw)
  To: Laurent Dufour, linuxppc-dev, linux-kernel
  Cc: Nathan Lynch, tyreld, kbuild-all
In-Reply-To: <20220106161339.74656-1-ldufour@linux.ibm.com>

Hi Laurent,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on powerpc/next]
[also build test WARNING on linux/master linus/master v5.16 next-20220110]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Laurent-Dufour/powerpc-pseries-read-the-lpar-name-from-the-firmware/20220107-001503
base:   https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc64-randconfig-r026-20220106 (https://download.01.org/0day-ci/archive/20220110/202201102154.a95OQEPr-lkp@intel.com/config)
compiler: powerpc64-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/5cf0dea6e919e93ff3088f87acd40e84608a6861
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Laurent-Dufour/powerpc-pseries-read-the-lpar-name-from-the-firmware/20220107-001503
        git checkout 5cf0dea6e919e93ff3088f87acd40e84608a6861
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=powerpc SHELL=/bin/bash arch/powerpc/platforms/pseries/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   arch/powerpc/platforms/pseries/lparcfg.c:257: warning: Function parameter or member 'm' not described in 'parse_mpp_data'
   arch/powerpc/platforms/pseries/lparcfg.c:295: warning: Function parameter or member 'm' not described in 'parse_mpp_x_data'
   arch/powerpc/platforms/pseries/lparcfg.c:334: warning: Function parameter or member 'm' not described in 'read_RTAS_lpar_name'
>> arch/powerpc/platforms/pseries/lparcfg.c:334: warning: expecting prototype for Read the lpar name using the RTAS ibm,get-system(). Prototype was for read_RTAS_lpar_name() instead
>> arch/powerpc/platforms/pseries/lparcfg.c:378: warning: This comment starts with '/**', but isn't a kernel-doc comment. Refer Documentation/doc-guide/kernel-doc.rst
    * Read the LPAR name from the Device Tree.
   arch/powerpc/platforms/pseries/lparcfg.c:678: warning: Function parameter or member 'entitlement' not described in 'update_mpp'
   arch/powerpc/platforms/pseries/lparcfg.c:678: warning: Function parameter or member 'weight' not described in 'update_mpp'


vim +334 arch/powerpc/platforms/pseries/lparcfg.c

   323	
   324	/**
   325	 * Read the lpar name using the RTAS ibm,get-system-parameter call.
   326	 *
   327	 * The name read through this call is updated if changes are made by the end
   328	 * user on the hypervisor side.
   329	 *
   330	 * Some hypervisor (like Qemu) may not provide this value. In that case, a non
   331	 * null value is returned.
   332	 */
   333	static int read_RTAS_lpar_name(struct seq_file *m)
 > 334	{
   335		int rc, len, token;
   336		union {
   337			char raw_buffer[GET_SYS_PARM_BUF_SIZE];
   338			struct {
   339				__be16 len;
   340				char name[GET_SYS_PARM_BUF_SIZE-2];
   341			};
   342		} *local_buffer;
   343	
   344		token = rtas_token("ibm,get-system-parameter");
   345		if (token == RTAS_UNKNOWN_SERVICE)
   346			return -EINVAL;
   347	
   348		local_buffer = kmalloc(sizeof(*local_buffer), GFP_KERNEL);
   349		if (!local_buffer)
   350			return -ENOMEM;
   351	
   352		do {
   353			spin_lock(&rtas_data_buf_lock);
   354			memset(rtas_data_buf, 0, sizeof(*local_buffer));
   355			rc = rtas_call(token, 3, 1, NULL, SPLPAR_LPAR_NAME_TOKEN,
   356				       __pa(rtas_data_buf), sizeof(*local_buffer));
   357			if (!rc)
   358				memcpy(local_buffer->raw_buffer, rtas_data_buf,
   359				       sizeof(local_buffer->raw_buffer));
   360			spin_unlock(&rtas_data_buf_lock);
   361		} while (rtas_busy_delay(rc));
   362	
   363		if (!rc) {
   364			/* Force end of string */
   365			len = min((int) be16_to_cpu(local_buffer->len),
   366				  (int) sizeof(local_buffer->name)-1);
   367			local_buffer->name[len] = '\0';
   368	
   369			seq_printf(m, "partition_name=%s\n", local_buffer->name);
   370		} else
   371			rc = -ENODATA;
   372	
   373		kfree(local_buffer);
   374		return rc;
   375	}
   376	
   377	/**
 > 378	 * Read the LPAR name from the Device Tree.
   379	 *
   380	 * The value read in the DT is not updated if the end-user is touching the LPAR
   381	 * name on the hypervisor side.
   382	 */
   383	static int read_DT_lpar_name(struct seq_file *m)
   384	{
   385		struct device_node *rootdn;
   386		const char *name;
   387	
   388		rootdn = of_find_node_by_path("/");
   389		if (!rootdn)
   390			return -ENOENT;
   391	
   392		name = of_get_property(rootdn, "ibm,partition-name", NULL);
   393		if (!name)
   394			return -ENOENT;
   395	
   396		seq_printf(m, "partition_name=%s\n", name);
   397		return 0;
   398	}
   399	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

^ permalink raw reply

* Re: [PATCH V2 03/17] asm-generic: fcntl: compat: Remove duplicate definitions
From: Arnd Bergmann @ 2022-01-10 13:35 UTC (permalink / raw)
  To: Guo Ren
  Cc: Guo Ren, open list:BROADCOM NVRAM DRIVER, J. Bruce Fields,
	sparclinux, linux-riscv, inux-parisc, linux-s390,
	the arch/x86 maintainers, linux-csky, Christoph Hellwig,
	Arnd Bergmann, Linux ARM, Drew Fustini, gregkh, Wang Junqiang,
	Anup Patel, Jeff Layton, Linux Kernel Mailing List,
	Palmer Dabbelt, liush, Linux FS-devel Mailing List, linuxppc-dev,
	Wei Fu
In-Reply-To: <20211228143958.3409187-4-guoren@kernel.org>

On Tue, Dec 28, 2021 at 3:39 PM <guoren@kernel.org> wrote:
>
> From: Guo Ren <guoren@linux.alibaba.com>
>
> Remove duplicate F_GETLK64,F_SETLK64,F_SETLKW64 definitions in
> arch/*/include/asm/compat.h.
>
> Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> Signed-off-by: Guo Ren <guoren@kernel.org>
> Cc: Arnd Bergmann <arnd@arndb.de>

Unfortunately, this one does not look correct to me:

> @@ -116,7 +116,7 @@
>  #define F_GETSIG       11      /* for sockets. */
>  #endif
>
> -#ifndef CONFIG_64BIT
> +#if !defined(CONFIG_64BIT) || defined(CONFIG_COMPAT)
>  #ifndef F_GETLK64
>  #define F_GETLK64      12      /*  using 'struct flock64' */
>  #define F_SETLK64      13

The problem here is that include/uapi/ headers cannot contain checks for
CONFIG_* symbols because those may have different meanings in user space
compared to kernel.

This is a preexisting problem in the header, but I think the change
makes it worse.

With the current behavior, user space will always see the definitions,
unless it happens to have its own definition for CONFIG_64BIT already.
On 64-bit parisc, this has the effect of defining the macros to the
same values as F_SETOWN/F_SETSIG/F_GETSIG, which is potentially
harmful. On MIPS, it uses values that are different from the 32-bit numbers
but are otherwise unused. Everywhere else, we get the definition from
the 32-bit architecture in user space, which will do nothing in the kernel.

The correct check for a uapi header would be to test for
__BITS_PER_LONG==32. We should probably do that here, but
this won't help you move the definitions, and it is a user-visible change
as the incorrect definition will no longer be visible. [Adding Jeff and Bruce
(the flock mainainers) to Cc for additional feedback on this]

For your series, I would suggest just moving the macro definitions to
include/linux/compat.h along with the 'struct compat_flock64'
definition, and leaving the duplicate one in the uapi header unchanged
until we have decided on a solution.

        Arnd

^ permalink raw reply

* [PATCH v4 0/6] KEXEC_SIG with appended signature
From: Michal Suchanek @ 2022-01-10 13:49 UTC (permalink / raw)
  To: keyrings, linux-crypto, linux-integrity
  Cc: Nayna, Mimi Zohar, David Howells, Paul Mackerras,
	Alexander Gordeev, Rob Herring, Herbert Xu, Baoquan He,
	Christian Borntraeger, James Morris, Lakshmi Ramasubramanian,
	Michal Suchanek, Serge E. Hallyn, Vasily Gorbik, linux-s390,
	Heiko Carstens, Dmitry Kasatkin, Hari Bathini, Daniel Axtens,
	Christian Borntraeger, Philipp Rudo, Frank van der Linden, kexec,
	linux-kernel, Luis Chamberlain, Sven Schnelle,
	linux-security-module, Jessica Yu, linuxppc-dev, David S. Miller,
	Thiago Jung Bauermann, buendgen

Hello,

This is a refresh of the KEXEC_SIG series.

This adds KEXEC_SIG support on powerpc and deduplicates the code dealing
with appended signatures in the kernel.

powerpc supports IMA_KEXEC but that's an exception rather than the norm.
On the other hand, KEXEC_SIG is portable across platforms.

For distributions to have uniform security features across platforms one
option should be used on all platforms.

Thanks

Michal

Previous revision: https://lore.kernel.org/linuxppc-dev/cover.1637862358.git.msuchanek@suse.de/
Patched kernel tree: https://github.com/hramrach/kernel/tree/kexec_sig

Michal Suchanek (6):
  s390/kexec_file: Don't opencode appended signature check.
  powerpc/kexec_file: Add KEXEC_SIG support.
  kexec_file: Don't opencode appended signature verification.
  module: strip the signature marker in the verification function.
  module: Use key_being_used_for for log messages in
    verify_appended_signature
  module: Move duplicate mod_check_sig users code to mod_parse_sig

 arch/powerpc/Kconfig                     | 16 +++++++
 arch/powerpc/kexec/elf_64.c              | 12 +++++
 arch/s390/Kconfig                        |  2 +-
 arch/s390/kernel/machine_kexec_file.c    | 41 +----------------
 crypto/asymmetric_keys/asymmetric_type.c |  1 +
 include/linux/module_signature.h         |  1 +
 include/linux/verification.h             |  5 +++
 kernel/module-internal.h                 |  2 -
 kernel/module.c                          | 12 +++--
 kernel/module_signature.c                | 56 +++++++++++++++++++++++-
 kernel/module_signing.c                  | 34 +++++++-------
 security/integrity/ima/ima_modsig.c      | 22 ++--------
 12 files changed, 116 insertions(+), 88 deletions(-)

-- 
2.31.1


^ permalink raw reply

* [PATCH v4 1/6] s390/kexec_file: Don't opencode appended signature check.
From: Michal Suchanek @ 2022-01-10 13:49 UTC (permalink / raw)
  To: keyrings, linux-crypto, linux-integrity
  Cc: Nayna, Mimi Zohar, David Howells, Paul Mackerras,
	Alexander Gordeev, Rob Herring, Herbert Xu, Baoquan He,
	Christian Borntraeger, James Morris, Lakshmi Ramasubramanian,
	Michal Suchanek, Serge E. Hallyn, Vasily Gorbik, linux-s390,
	Heiko Carstens, Dmitry Kasatkin, Hari Bathini, Daniel Axtens,
	Christian Borntraeger, Philipp Rudo, Frank van der Linden, kexec,
	linux-kernel, Luis Chamberlain, Sven Schnelle,
	linux-security-module, Jessica Yu, linuxppc-dev, David S. Miller,
	Thiago Jung Bauermann, buendgen
In-Reply-To: <cover.1641822505.git.msuchanek@suse.de>

Module verification already implements appeded signature check.

Reuse it for kexec_file.

The kexec_file implementation uses EKEYREJECTED error in some cases when
there is no key and the common implementation uses ENOPKG or EBADMSG
instead.

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
Acked-by: Heiko Carstens <hca@linux.ibm.com>
---
v3: Philipp Rudo <prudo@redhat.com>: Update the commit with note about
change of return value
---
 arch/s390/kernel/machine_kexec_file.c | 22 +++++-----------------
 1 file changed, 5 insertions(+), 17 deletions(-)

diff --git a/arch/s390/kernel/machine_kexec_file.c b/arch/s390/kernel/machine_kexec_file.c
index 8f43575a4dd3..c944d71316c7 100644
--- a/arch/s390/kernel/machine_kexec_file.c
+++ b/arch/s390/kernel/machine_kexec_file.c
@@ -31,6 +31,7 @@ int s390_verify_sig(const char *kernel, unsigned long kernel_len)
 	const unsigned long marker_len = sizeof(MODULE_SIG_STRING) - 1;
 	struct module_signature *ms;
 	unsigned long sig_len;
+	int ret;
 
 	/* Skip signature verification when not secure IPLed. */
 	if (!ipl_secure_flag)
@@ -45,25 +46,12 @@ int s390_verify_sig(const char *kernel, unsigned long kernel_len)
 	kernel_len -= marker_len;
 
 	ms = (void *)kernel + kernel_len - sizeof(*ms);
-	kernel_len -= sizeof(*ms);
+	ret = mod_check_sig(ms, kernel_len, "kexec");
+	if (ret)
+		return ret;
 
 	sig_len = be32_to_cpu(ms->sig_len);
-	if (sig_len >= kernel_len)
-		return -EKEYREJECTED;
-	kernel_len -= sig_len;
-
-	if (ms->id_type != PKEY_ID_PKCS7)
-		return -EKEYREJECTED;
-
-	if (ms->algo != 0 ||
-	    ms->hash != 0 ||
-	    ms->signer_len != 0 ||
-	    ms->key_id_len != 0 ||
-	    ms->__pad[0] != 0 ||
-	    ms->__pad[1] != 0 ||
-	    ms->__pad[2] != 0) {
-		return -EBADMSG;
-	}
+	kernel_len -= sizeof(*ms) + sig_len;
 
 	return verify_pkcs7_signature(kernel, kernel_len,
 				      kernel + kernel_len, sig_len,
-- 
2.31.1


^ permalink raw reply related

* [PATCH v4 3/6] kexec_file: Don't opencode appended signature verification.
From: Michal Suchanek @ 2022-01-10 13:49 UTC (permalink / raw)
  To: keyrings, linux-crypto, linux-integrity
  Cc: Nayna, Mimi Zohar, David Howells, Paul Mackerras,
	Alexander Gordeev, Rob Herring, Herbert Xu, Baoquan He,
	Christian Borntraeger, James Morris, Lakshmi Ramasubramanian,
	Michal Suchanek, Serge E. Hallyn, Vasily Gorbik, linux-s390,
	Heiko Carstens, Dmitry Kasatkin, Hari Bathini, Daniel Axtens,
	Christian Borntraeger, Philipp Rudo, Frank van der Linden, kexec,
	linux-kernel, Luis Chamberlain, Sven Schnelle,
	linux-security-module, Jessica Yu, linuxppc-dev, David S. Miller,
	Thiago Jung Bauermann, buendgen
In-Reply-To: <cover.1641822505.git.msuchanek@suse.de>

Module verification already implements appeded signature verification.

Reuse it for kexec_file.

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
v3: - Philipp Rudo <prudo@redhat.com>: Update the dependency on
      MODULE_SIG_FORMAT to MODULE_SIG
    - Include linux/verification.h - previously added in earlier patch
v4: - kernel test robot <lkp@intel.com>: Use unsigned long rather than size_t for data length
    - Update the code in kernel/module_signing.c to use pointer rather
      than memcpy as the kexec and IMA code does
---
 arch/powerpc/Kconfig                  |  2 +-
 arch/powerpc/kexec/elf_64.c           | 19 +++------------
 arch/s390/Kconfig                     |  2 +-
 arch/s390/kernel/machine_kexec_file.c | 18 ++------------
 include/linux/verification.h          |  3 +++
 kernel/module-internal.h              |  2 --
 kernel/module.c                       |  4 +++-
 kernel/module_signing.c               | 34 ++++++++++++++++-----------
 8 files changed, 33 insertions(+), 51 deletions(-)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 1cde9b6c5987..4092187474ff 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -562,7 +562,7 @@ config ARCH_HAS_KEXEC_PURGATORY
 
 config KEXEC_SIG
 	bool "Verify kernel signature during kexec_file_load() syscall"
-	depends on KEXEC_FILE && MODULE_SIG_FORMAT
+	depends on KEXEC_FILE && MODULE_SIG
 	help
 	  This option makes kernel signature verification mandatory for
 	  the kexec_file_load() syscall.
diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c
index 98d1cb5135b4..64cd314cce0d 100644
--- a/arch/powerpc/kexec/elf_64.c
+++ b/arch/powerpc/kexec/elf_64.c
@@ -23,6 +23,7 @@
 #include <linux/of_fdt.h>
 #include <linux/slab.h>
 #include <linux/types.h>
+#include <linux/verification.h>
 #include <linux/module_signature.h>
 
 static void *elf64_load(struct kimage *image, char *kernel_buf,
@@ -156,9 +157,6 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,
 int elf64_verify_sig(const char *kernel, unsigned long kernel_len)
 {
 	const unsigned long marker_len = sizeof(MODULE_SIG_STRING) - 1;
-	struct module_signature *ms;
-	unsigned long sig_len;
-	int ret;
 
 	if (marker_len > kernel_len)
 		return -EKEYREJECTED;
@@ -168,19 +166,8 @@ int elf64_verify_sig(const char *kernel, unsigned long kernel_len)
 		return -EKEYREJECTED;
 	kernel_len -= marker_len;
 
-	ms = (void *)kernel + kernel_len - sizeof(*ms);
-	ret = mod_check_sig(ms, kernel_len, "kexec");
-	if (ret)
-		return ret;
-
-	sig_len = be32_to_cpu(ms->sig_len);
-	kernel_len -= sizeof(*ms) + sig_len;
-
-	return verify_pkcs7_signature(kernel, kernel_len,
-				      kernel + kernel_len, sig_len,
-				      VERIFY_USE_PLATFORM_KEYRING,
-				      VERIFYING_MODULE_SIGNATURE,
-				      NULL, NULL);
+	return verify_appended_signature(kernel, &kernel_len, VERIFY_USE_PLATFORM_KEYRING,
+					 "kexec_file");
 }
 #endif /* CONFIG_KEXEC_SIG */
 
diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index 2a5bb4f29cfe..cece7152ea35 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -544,7 +544,7 @@ config ARCH_HAS_KEXEC_PURGATORY
 
 config KEXEC_SIG
 	bool "Verify kernel signature during kexec_file_load() syscall"
-	depends on KEXEC_FILE && MODULE_SIG_FORMAT
+	depends on KEXEC_FILE && MODULE_SIG
 	help
 	  This option makes kernel signature verification mandatory for
 	  the kexec_file_load() syscall.
diff --git a/arch/s390/kernel/machine_kexec_file.c b/arch/s390/kernel/machine_kexec_file.c
index c944d71316c7..345f2eab6e04 100644
--- a/arch/s390/kernel/machine_kexec_file.c
+++ b/arch/s390/kernel/machine_kexec_file.c
@@ -29,9 +29,6 @@ const struct kexec_file_ops * const kexec_file_loaders[] = {
 int s390_verify_sig(const char *kernel, unsigned long kernel_len)
 {
 	const unsigned long marker_len = sizeof(MODULE_SIG_STRING) - 1;
-	struct module_signature *ms;
-	unsigned long sig_len;
-	int ret;
 
 	/* Skip signature verification when not secure IPLed. */
 	if (!ipl_secure_flag)
@@ -45,19 +42,8 @@ int s390_verify_sig(const char *kernel, unsigned long kernel_len)
 		return -EKEYREJECTED;
 	kernel_len -= marker_len;
 
-	ms = (void *)kernel + kernel_len - sizeof(*ms);
-	ret = mod_check_sig(ms, kernel_len, "kexec");
-	if (ret)
-		return ret;
-
-	sig_len = be32_to_cpu(ms->sig_len);
-	kernel_len -= sizeof(*ms) + sig_len;
-
-	return verify_pkcs7_signature(kernel, kernel_len,
-				      kernel + kernel_len, sig_len,
-				      VERIFY_USE_PLATFORM_KEYRING,
-				      VERIFYING_MODULE_SIGNATURE,
-				      NULL, NULL);
+	return verify_appended_signature(kernel, &kernel_len, VERIFY_USE_PLATFORM_KEYRING,
+					"kexec_file");
 }
 #endif /* CONFIG_KEXEC_SIG */
 
diff --git a/include/linux/verification.h b/include/linux/verification.h
index a655923335ae..32db9287a7b0 100644
--- a/include/linux/verification.h
+++ b/include/linux/verification.h
@@ -60,5 +60,8 @@ extern int verify_pefile_signature(const void *pebuf, unsigned pelen,
 				   enum key_being_used_for usage);
 #endif
 
+int verify_appended_signature(const void *data, unsigned long *len,
+			      struct key *trusted_keys, const char *what);
+
 #endif /* CONFIG_SYSTEM_DATA_VERIFICATION */
 #endif /* _LINUX_VERIFY_PEFILE_H */
diff --git a/kernel/module-internal.h b/kernel/module-internal.h
index 33783abc377b..80461e14bf29 100644
--- a/kernel/module-internal.h
+++ b/kernel/module-internal.h
@@ -27,5 +27,3 @@ struct load_info {
 		unsigned int sym, str, mod, vers, info, pcpu;
 	} index;
 };
-
-extern int mod_verify_sig(const void *mod, struct load_info *info);
diff --git a/kernel/module.c b/kernel/module.c
index 84a9141a5e15..8481933dfa92 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -57,6 +57,7 @@
 #include <linux/bsearch.h>
 #include <linux/dynamic_debug.h>
 #include <linux/audit.h>
+#include <linux/verification.h>
 #include <uapi/linux/module.h>
 #include "module-internal.h"
 
@@ -2894,7 +2895,8 @@ static int module_sig_check(struct load_info *info, int flags)
 	    memcmp(mod + info->len - markerlen, MODULE_SIG_STRING, markerlen) == 0) {
 		/* We truncate the module to discard the signature */
 		info->len -= markerlen;
-		err = mod_verify_sig(mod, info);
+		err = verify_appended_signature(mod, &info->len,
+						VERIFY_USE_SECONDARY_KEYRING, "module");
 		if (!err) {
 			info->sig_ok = true;
 			return 0;
diff --git a/kernel/module_signing.c b/kernel/module_signing.c
index 8723ae70ea1f..30149969f21f 100644
--- a/kernel/module_signing.c
+++ b/kernel/module_signing.c
@@ -14,32 +14,38 @@
 #include <crypto/public_key.h>
 #include "module-internal.h"
 
-/*
- * Verify the signature on a module.
+/**
+ * verify_appended_signature - Verify the signature on a module with the
+ * signature marker stripped.
+ * @data: The data to be verified
+ * @len: Size of @data.
+ * @trusted_keys: Keyring to use for verification
+ * @what: Informational string for log messages
  */
-int mod_verify_sig(const void *mod, struct load_info *info)
+int verify_appended_signature(const void *data, unsigned long *len,
+			      struct key *trusted_keys, const char *what)
 {
-	struct module_signature ms;
-	size_t sig_len, modlen = info->len;
+	struct module_signature *ms;
+	unsigned long sig_len, modlen = *len;
 	int ret;
 
-	pr_devel("==>%s(,%zu)\n", __func__, modlen);
+	pr_devel("==>%s(,%lu)\n", __func__, modlen);
 
-	if (modlen <= sizeof(ms))
+	if (modlen <= sizeof(*ms))
 		return -EBADMSG;
 
-	memcpy(&ms, mod + (modlen - sizeof(ms)), sizeof(ms));
+	ms = data + modlen - sizeof(*ms);
 
-	ret = mod_check_sig(&ms, modlen, "module");
+	ret = mod_check_sig(ms, modlen, what);
 	if (ret)
 		return ret;
 
-	sig_len = be32_to_cpu(ms.sig_len);
-	modlen -= sig_len + sizeof(ms);
-	info->len = modlen;
+	sig_len = be32_to_cpu(ms->sig_len);
+	modlen -= sig_len + sizeof(*ms);
+	*len = modlen;
 
-	return verify_pkcs7_signature(mod, modlen, mod + modlen, sig_len,
-				      VERIFY_USE_SECONDARY_KEYRING,
+	return verify_pkcs7_signature(data, modlen, data + modlen, sig_len,
+				      trusted_keys,
 				      VERIFYING_MODULE_SIGNATURE,
 				      NULL, NULL);
 }
-- 
2.31.1


^ permalink raw reply related

* [PATCH v4 2/6] powerpc/kexec_file: Add KEXEC_SIG support.
From: Michal Suchanek @ 2022-01-10 13:49 UTC (permalink / raw)
  To: keyrings, linux-crypto, linux-integrity
  Cc: Nayna, Mimi Zohar, David Howells, Paul Mackerras,
	Alexander Gordeev, Rob Herring, Herbert Xu, Baoquan He,
	Christian Borntraeger, James Morris, Lakshmi Ramasubramanian,
	Michal Suchanek, Serge E. Hallyn, Vasily Gorbik, linux-s390,
	Heiko Carstens, Dmitry Kasatkin, Hari Bathini, Daniel Axtens,
	Christian Borntraeger, Philipp Rudo, Frank van der Linden, kexec,
	linux-kernel, Luis Chamberlain, Sven Schnelle,
	linux-security-module, Jessica Yu, linuxppc-dev, David S. Miller,
	Thiago Jung Bauermann, buendgen
In-Reply-To: <cover.1641822505.git.msuchanek@suse.de>

Copy the code from s390x

Both powerpc and s390x use appended signature format (as opposed to EFI
based patforms using PE format).

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
v3: - Philipp Rudo <prudo@redhat.com>: Update the comit message with
      explanation why the s390 code is usable on powerpc.
    - Include correct header for mod_check_sig
    - Nayna <nayna@linux.vnet.ibm.com>: Mention additional IMA features
      in kconfig text
---
 arch/powerpc/Kconfig        | 16 ++++++++++++++++
 arch/powerpc/kexec/elf_64.c | 36 ++++++++++++++++++++++++++++++++++++
 2 files changed, 52 insertions(+)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index dea74d7717c0..1cde9b6c5987 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -560,6 +560,22 @@ config KEXEC_FILE
 config ARCH_HAS_KEXEC_PURGATORY
 	def_bool KEXEC_FILE
 
+config KEXEC_SIG
+	bool "Verify kernel signature during kexec_file_load() syscall"
+	depends on KEXEC_FILE && MODULE_SIG_FORMAT
+	help
+	  This option makes kernel signature verification mandatory for
+	  the kexec_file_load() syscall.
+
+	  In addition to that option, you need to enable signature
+	  verification for the corresponding kernel image type being
+	  loaded in order for this to work.
+
+	  Note: on powerpc IMA_ARCH_POLICY also implements kexec'ed kernel
+	  verification. In addition IMA adds kernel hashes to the measurement
+	  list, extends IMA PCR in the TPM, and implements kernel image
+	  blacklist by hash.
+
 config RELOCATABLE
 	bool "Build a relocatable kernel"
 	depends on PPC64 || (FLATMEM && (44x || FSL_BOOKE))
diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c
index eeb258002d1e..98d1cb5135b4 100644
--- a/arch/powerpc/kexec/elf_64.c
+++ b/arch/powerpc/kexec/elf_64.c
@@ -23,6 +23,7 @@
 #include <linux/of_fdt.h>
 #include <linux/slab.h>
 #include <linux/types.h>
+#include <linux/module_signature.h>
 
 static void *elf64_load(struct kimage *image, char *kernel_buf,
 			unsigned long kernel_len, char *initrd,
@@ -151,7 +152,42 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,
 	return ret ? ERR_PTR(ret) : NULL;
 }
 
+#ifdef CONFIG_KEXEC_SIG
+int elf64_verify_sig(const char *kernel, unsigned long kernel_len)
+{
+	const unsigned long marker_len = sizeof(MODULE_SIG_STRING) - 1;
+	struct module_signature *ms;
+	unsigned long sig_len;
+	int ret;
+
+	if (marker_len > kernel_len)
+		return -EKEYREJECTED;
+
+	if (memcmp(kernel + kernel_len - marker_len, MODULE_SIG_STRING,
+		   marker_len))
+		return -EKEYREJECTED;
+	kernel_len -= marker_len;
+
+	ms = (void *)kernel + kernel_len - sizeof(*ms);
+	ret = mod_check_sig(ms, kernel_len, "kexec");
+	if (ret)
+		return ret;
+
+	sig_len = be32_to_cpu(ms->sig_len);
+	kernel_len -= sizeof(*ms) + sig_len;
+
+	return verify_pkcs7_signature(kernel, kernel_len,
+				      kernel + kernel_len, sig_len,
+				      VERIFY_USE_PLATFORM_KEYRING,
+				      VERIFYING_MODULE_SIGNATURE,
+				      NULL, NULL);
+}
+#endif /* CONFIG_KEXEC_SIG */
+
 const struct kexec_file_ops kexec_elf64_ops = {
 	.probe = kexec_elf_probe,
 	.load = elf64_load,
+#ifdef CONFIG_KEXEC_SIG
+	.verify_sig = elf64_verify_sig,
+#endif
 };
-- 
2.31.1


^ permalink raw reply related

* [PATCH v4 4/6] module: strip the signature marker in the verification function.
From: Michal Suchanek @ 2022-01-10 13:49 UTC (permalink / raw)
  To: keyrings, linux-crypto, linux-integrity
  Cc: Nayna, Mimi Zohar, David Howells, Paul Mackerras,
	Alexander Gordeev, Rob Herring, Herbert Xu, Baoquan He,
	Christian Borntraeger, James Morris, Lakshmi Ramasubramanian,
	Michal Suchanek, Serge E. Hallyn, Vasily Gorbik, linux-s390,
	Heiko Carstens, Dmitry Kasatkin, Hari Bathini, Daniel Axtens,
	Christian Borntraeger, Philipp Rudo, Frank van der Linden, kexec,
	linux-kernel, Luis Chamberlain, Sven Schnelle,
	linux-security-module, Jessica Yu, linuxppc-dev, David S. Miller,
	Thiago Jung Bauermann, buendgen
In-Reply-To: <cover.1641822505.git.msuchanek@suse.de>

It is stripped by each caller separately.

Note: this changes the error for kexec_file from EKEYREJECTED to ENODATA
when the signature marker is missing.

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
v3: - Philipp Rudo <prudo@redhat.com>: Update the commit with note about
      change of raturn value
    - the module_signature.h is now no longer needed for kexec_file
---
 arch/powerpc/kexec/elf_64.c           | 11 -----------
 arch/s390/kernel/machine_kexec_file.c | 11 -----------
 kernel/module.c                       |  7 +------
 kernel/module_signing.c               | 12 ++++++++++--
 4 files changed, 11 insertions(+), 30 deletions(-)

diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c
index 64cd314cce0d..6dec8151ef73 100644
--- a/arch/powerpc/kexec/elf_64.c
+++ b/arch/powerpc/kexec/elf_64.c
@@ -24,7 +24,6 @@
 #include <linux/slab.h>
 #include <linux/types.h>
 #include <linux/verification.h>
-#include <linux/module_signature.h>
 
 static void *elf64_load(struct kimage *image, char *kernel_buf,
 			unsigned long kernel_len, char *initrd,
@@ -156,16 +155,6 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,
 #ifdef CONFIG_KEXEC_SIG
 int elf64_verify_sig(const char *kernel, unsigned long kernel_len)
 {
-	const unsigned long marker_len = sizeof(MODULE_SIG_STRING) - 1;
-
-	if (marker_len > kernel_len)
-		return -EKEYREJECTED;
-
-	if (memcmp(kernel + kernel_len - marker_len, MODULE_SIG_STRING,
-		   marker_len))
-		return -EKEYREJECTED;
-	kernel_len -= marker_len;
-
 	return verify_appended_signature(kernel, &kernel_len, VERIFY_USE_PLATFORM_KEYRING,
 					 "kexec_file");
 }
diff --git a/arch/s390/kernel/machine_kexec_file.c b/arch/s390/kernel/machine_kexec_file.c
index 345f2eab6e04..c3deccf1da83 100644
--- a/arch/s390/kernel/machine_kexec_file.c
+++ b/arch/s390/kernel/machine_kexec_file.c
@@ -12,7 +12,6 @@
 #include <linux/elf.h>
 #include <linux/errno.h>
 #include <linux/kexec.h>
-#include <linux/module_signature.h>
 #include <linux/verification.h>
 #include <linux/vmalloc.h>
 #include <asm/boot_data.h>
@@ -28,20 +27,10 @@ const struct kexec_file_ops * const kexec_file_loaders[] = {
 #ifdef CONFIG_KEXEC_SIG
 int s390_verify_sig(const char *kernel, unsigned long kernel_len)
 {
-	const unsigned long marker_len = sizeof(MODULE_SIG_STRING) - 1;
-
 	/* Skip signature verification when not secure IPLed. */
 	if (!ipl_secure_flag)
 		return 0;
 
-	if (marker_len > kernel_len)
-		return -EKEYREJECTED;
-
-	if (memcmp(kernel + kernel_len - marker_len, MODULE_SIG_STRING,
-		   marker_len))
-		return -EKEYREJECTED;
-	kernel_len -= marker_len;
-
 	return verify_appended_signature(kernel, &kernel_len, VERIFY_USE_PLATFORM_KEYRING,
 					"kexec_file");
 }
diff --git a/kernel/module.c b/kernel/module.c
index 8481933dfa92..d91ca0f93a40 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2882,7 +2882,6 @@ static inline void kmemleak_load_module(const struct module *mod,
 static int module_sig_check(struct load_info *info, int flags)
 {
 	int err = -ENODATA;
-	const unsigned long markerlen = sizeof(MODULE_SIG_STRING) - 1;
 	const char *reason;
 	const void *mod = info->hdr;
 
@@ -2890,11 +2889,7 @@ static int module_sig_check(struct load_info *info, int flags)
 	 * Require flags == 0, as a module with version information
 	 * removed is no longer the module that was signed
 	 */
-	if (flags == 0 &&
-	    info->len > markerlen &&
-	    memcmp(mod + info->len - markerlen, MODULE_SIG_STRING, markerlen) == 0) {
-		/* We truncate the module to discard the signature */
-		info->len -= markerlen;
+	if (flags == 0) {
 		err = verify_appended_signature(mod, &info->len,
 						VERIFY_USE_SECONDARY_KEYRING, "module");
 		if (!err) {
diff --git a/kernel/module_signing.c b/kernel/module_signing.c
index 30149969f21f..39a6dd7c6dd2 100644
--- a/kernel/module_signing.c
+++ b/kernel/module_signing.c
@@ -15,8 +15,7 @@
 #include "module-internal.h"
 
 /**
- * verify_appended_signature - Verify the signature on a module with the
- * signature marker stripped.
+ * verify_appended_signature - Verify the signature on a module
  * @data: The data to be verified
  * @len: Size of @data.
  * @trusted_keys: Keyring to use for verification
@@ -25,12 +24,21 @@
 int verify_appended_signature(const void *data, unsigned long *len,
 			      struct key *trusted_keys, const char *what)
 {
+	const unsigned long markerlen = sizeof(MODULE_SIG_STRING) - 1;
 	struct module_signature *ms;
 	unsigned long sig_len, modlen = *len;
 	int ret;
 
 	pr_devel("==>%s(,%lu)\n", __func__, modlen);
 
+	if (markerlen > modlen)
+		return -ENODATA;
+
+	if (memcmp(data + modlen - markerlen, MODULE_SIG_STRING,
+		   markerlen))
+		return -ENODATA;
+	modlen -= markerlen;
+
 	if (modlen <= sizeof(*ms))
 		return -EBADMSG;
 
-- 
2.31.1


^ permalink raw reply related

* [PATCH v4 5/6] module: Use key_being_used_for for log messages in verify_appended_signature
From: Michal Suchanek @ 2022-01-10 13:49 UTC (permalink / raw)
  To: keyrings, linux-crypto, linux-integrity
  Cc: Nayna, Mimi Zohar, David Howells, Paul Mackerras,
	Alexander Gordeev, Rob Herring, Herbert Xu, Baoquan He,
	Christian Borntraeger, James Morris, Lakshmi Ramasubramanian,
	Michal Suchanek, Serge E. Hallyn, Vasily Gorbik, linux-s390,
	Heiko Carstens, Dmitry Kasatkin, Hari Bathini, Daniel Axtens,
	Christian Borntraeger, Philipp Rudo, Frank van der Linden, kexec,
	linux-kernel, Luis Chamberlain, Sven Schnelle,
	linux-security-module, Jessica Yu, linuxppc-dev, David S. Miller,
	Thiago Jung Bauermann, buendgen
In-Reply-To: <cover.1641822505.git.msuchanek@suse.de>

Add value for kexec appended signature and pass in key_being_used_for
enum rather than a string to verify_appended_signature to produce log
messages about the signature.

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
 arch/powerpc/kexec/elf_64.c              |  2 +-
 arch/s390/kernel/machine_kexec_file.c    |  2 +-
 crypto/asymmetric_keys/asymmetric_type.c |  1 +
 include/linux/verification.h             |  4 +++-
 kernel/module.c                          |  3 ++-
 kernel/module_signing.c                  | 11 ++++++-----
 6 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c
index 6dec8151ef73..c50869195d51 100644
--- a/arch/powerpc/kexec/elf_64.c
+++ b/arch/powerpc/kexec/elf_64.c
@@ -156,7 +156,7 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,
 int elf64_verify_sig(const char *kernel, unsigned long kernel_len)
 {
 	return verify_appended_signature(kernel, &kernel_len, VERIFY_USE_PLATFORM_KEYRING,
-					 "kexec_file");
+					 VERIFYING_KEXEC_APPENDED_SIGNATURE);
 }
 #endif /* CONFIG_KEXEC_SIG */
 
diff --git a/arch/s390/kernel/machine_kexec_file.c b/arch/s390/kernel/machine_kexec_file.c
index c3deccf1da83..63eec38e3137 100644
--- a/arch/s390/kernel/machine_kexec_file.c
+++ b/arch/s390/kernel/machine_kexec_file.c
@@ -32,7 +32,7 @@ int s390_verify_sig(const char *kernel, unsigned long kernel_len)
 		return 0;
 
 	return verify_appended_signature(kernel, &kernel_len, VERIFY_USE_PLATFORM_KEYRING,
-					"kexec_file");
+					VERIFYING_KEXEC_APPENDED_SIGNATURE);
 }
 #endif /* CONFIG_KEXEC_SIG */
 
diff --git a/crypto/asymmetric_keys/asymmetric_type.c b/crypto/asymmetric_keys/asymmetric_type.c
index ad8af3d70ac0..6fd20eec3882 100644
--- a/crypto/asymmetric_keys/asymmetric_type.c
+++ b/crypto/asymmetric_keys/asymmetric_type.c
@@ -25,6 +25,7 @@ const char *const key_being_used_for[NR__KEY_BEING_USED_FOR] = {
 	[VERIFYING_KEXEC_PE_SIGNATURE]		= "kexec PE sig",
 	[VERIFYING_KEY_SIGNATURE]		= "key sig",
 	[VERIFYING_KEY_SELF_SIGNATURE]		= "key self sig",
+	[VERIFYING_KEXEC_APPENDED_SIGNATURE]	= "kexec appended sig",
 	[VERIFYING_UNSPECIFIED_SIGNATURE]	= "unspec sig",
 };
 EXPORT_SYMBOL_GPL(key_being_used_for);
diff --git a/include/linux/verification.h b/include/linux/verification.h
index 32db9287a7b0..f92c49443b4f 100644
--- a/include/linux/verification.h
+++ b/include/linux/verification.h
@@ -26,6 +26,7 @@ enum key_being_used_for {
 	VERIFYING_KEXEC_PE_SIGNATURE,
 	VERIFYING_KEY_SIGNATURE,
 	VERIFYING_KEY_SELF_SIGNATURE,
+	VERIFYING_KEXEC_APPENDED_SIGNATURE,
 	VERIFYING_UNSPECIFIED_SIGNATURE,
 	NR__KEY_BEING_USED_FOR
 };
@@ -61,7 +62,8 @@ extern int verify_pefile_signature(const void *pebuf, unsigned pelen,
 #endif
 
 int verify_appended_signature(const void *data, unsigned long *len,
-			      struct key *trusted_keys, const char *what);
+			      struct key *trusted_keys,
+			      enum key_being_used_for purpose);
 
 #endif /* CONFIG_SYSTEM_DATA_VERIFICATION */
 #endif /* _LINUX_VERIFY_PEFILE_H */
diff --git a/kernel/module.c b/kernel/module.c
index d91ca0f93a40..0a359dc6b690 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2891,7 +2891,8 @@ static int module_sig_check(struct load_info *info, int flags)
 	 */
 	if (flags == 0) {
 		err = verify_appended_signature(mod, &info->len,
-						VERIFY_USE_SECONDARY_KEYRING, "module");
+						VERIFY_USE_SECONDARY_KEYRING,
+						VERIFYING_MODULE_SIGNATURE);
 		if (!err) {
 			info->sig_ok = true;
 			return 0;
diff --git a/kernel/module_signing.c b/kernel/module_signing.c
index 39a6dd7c6dd2..20857d2a15ca 100644
--- a/kernel/module_signing.c
+++ b/kernel/module_signing.c
@@ -19,17 +19,18 @@
  * @data: The data to be verified
  * @len: Size of @data.
  * @trusted_keys: Keyring to use for verification
- * @what: Informational string for log messages
+ * @purpose: The use to which the key is being put
  */
 int verify_appended_signature(const void *data, unsigned long *len,
-			      struct key *trusted_keys, const char *what)
+			      struct key *trusted_keys,
+			      enum key_being_used_for purpose)
 {
 	const unsigned long markerlen = sizeof(MODULE_SIG_STRING) - 1;
 	struct module_signature *ms;
 	unsigned long sig_len, modlen = *len;
 	int ret;
 
-	pr_devel("==>%s(,%lu)\n", __func__, modlen);
+	pr_devel("==>%s %s(,%lu)\n", __func__, key_being_used_for[purpose], modlen);
 
 	if (markerlen > modlen)
 		return -ENODATA;
@@ -44,7 +45,7 @@ int verify_appended_signature(const void *data, unsigned long *len,
 
 	ms = data + modlen - sizeof(*ms);
 
-	ret = mod_check_sig(ms, modlen, what);
+	ret = mod_check_sig(ms, modlen, key_being_used_for[purpose]);
 	if (ret)
 		return ret;
 
@@ -54,6 +55,6 @@ int verify_appended_signature(const void *data, unsigned long *len,
 
 	return verify_pkcs7_signature(data, modlen, data + modlen, sig_len,
 				      trusted_keys,
-				      VERIFYING_MODULE_SIGNATURE,
+				      purpose,
 				      NULL, NULL);
 }
-- 
2.31.1


^ permalink raw reply related

* [PATCH v4 6/6] module: Move duplicate mod_check_sig users code to mod_parse_sig
From: Michal Suchanek @ 2022-01-10 13:49 UTC (permalink / raw)
  To: keyrings, linux-crypto, linux-integrity
  Cc: Nayna, Mimi Zohar, David Howells, Paul Mackerras,
	Alexander Gordeev, Rob Herring, Herbert Xu, Baoquan He,
	Christian Borntraeger, James Morris, Lakshmi Ramasubramanian,
	Michal Suchanek, Serge E. Hallyn, Vasily Gorbik, linux-s390,
	Heiko Carstens, Dmitry Kasatkin, Hari Bathini, Daniel Axtens,
	Christian Borntraeger, Philipp Rudo, Frank van der Linden, kexec,
	linux-kernel, Luis Chamberlain, Sven Schnelle,
	linux-security-module, Jessica Yu, linuxppc-dev, David S. Miller,
	Thiago Jung Bauermann, buendgen
In-Reply-To: <cover.1641822505.git.msuchanek@suse.de>

Multiple users of mod_check_sig check for the marker, then call
mod_check_sig, extract signature length, and remove the signature.

Put this code in one place together with mod_check_sig.

This changes the error from ENOENT to ENODATA for ima_read_modsig in the
case the signature marker is missing.

This also changes the buffer length in ima_read_modsig from size_t to
unsigned long. This reduces the possible value range on 32bit but the
length refers to kernel in-memory buffer which cannot be longer than
ULONG_MAX.

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
v3: - Philipp Rudo <prudo@redhat.com>: Update the commit with note about
      change of raturn value
    - Preserve the EBADMSG error code while moving code araound
v4: - remove unused variable ms in module_signing
    - note about buffer length
---
 include/linux/module_signature.h    |  1 +
 kernel/module_signature.c           | 56 ++++++++++++++++++++++++++++-
 kernel/module_signing.c             | 27 +++-----------
 security/integrity/ima/ima_modsig.c | 22 ++----------
 4 files changed, 63 insertions(+), 43 deletions(-)

diff --git a/include/linux/module_signature.h b/include/linux/module_signature.h
index 7eb4b00381ac..1343879b72b3 100644
--- a/include/linux/module_signature.h
+++ b/include/linux/module_signature.h
@@ -42,5 +42,6 @@ struct module_signature {
 
 int mod_check_sig(const struct module_signature *ms, size_t file_len,
 		  const char *name);
+int mod_parse_sig(const void *data, size_t *len, size_t *sig_len, const char *name);
 
 #endif /* _LINUX_MODULE_SIGNATURE_H */
diff --git a/kernel/module_signature.c b/kernel/module_signature.c
index 00132d12487c..b8eb30182183 100644
--- a/kernel/module_signature.c
+++ b/kernel/module_signature.c
@@ -8,14 +8,36 @@
 
 #include <linux/errno.h>
 #include <linux/printk.h>
+#include <linux/string.h>
 #include <linux/module_signature.h>
 #include <asm/byteorder.h>
 
+/**
+ * mod_check_sig_marker - check that the given data has signature marker at the end
+ *
+ * @data:	Data with appended signature
+ * @len:	Length of data. Signature marker length is subtracted on success.
+ */
+static inline int mod_check_sig_marker(const void *data, size_t *len)
+{
+	const unsigned long markerlen = sizeof(MODULE_SIG_STRING) - 1;
+
+	if (markerlen > *len)
+		return -ENODATA;
+
+	if (memcmp(data + *len - markerlen, MODULE_SIG_STRING,
+		   markerlen))
+		return -ENODATA;
+
+	*len -= markerlen;
+	return 0;
+}
+
 /**
  * mod_check_sig - check that the given signature is sane
  *
  * @ms:		Signature to check.
- * @file_len:	Size of the file to which @ms is appended.
+ * @file_len:	Size of the file to which @ms is appended (without the marker).
  * @name:	What is being checked. Used for error messages.
  */
 int mod_check_sig(const struct module_signature *ms, size_t file_len,
@@ -44,3 +66,35 @@ int mod_check_sig(const struct module_signature *ms, size_t file_len,
 
 	return 0;
 }
+
+/**
+ * mod_parse_sig - check that the given signature is sane and determine signature length
+ *
+ * @data:	Data with appended signature.
+ * @len:	Length of data. Signature and marker length is subtracted on success.
+ * @sig_len:	Length of signature. Filled on success.
+ * @name:	What is being checked. Used for error messages.
+ */
+int mod_parse_sig(const void *data, size_t *len, size_t *sig_len, const char *name)
+{
+	const struct module_signature *sig;
+	int rc;
+
+	rc = mod_check_sig_marker(data, len);
+	if (rc)
+		return rc;
+
+	if (*len < sizeof(*sig))
+		return -EBADMSG;
+
+	sig = (const struct module_signature *)(data + (*len - sizeof(*sig)));
+
+	rc = mod_check_sig(sig, *len, name);
+	if (rc)
+		return rc;
+
+	*sig_len = be32_to_cpu(sig->sig_len);
+	*len -= *sig_len + sizeof(*sig);
+
+	return 0;
+}
diff --git a/kernel/module_signing.c b/kernel/module_signing.c
index 20857d2a15ca..1d4cb03cce21 100644
--- a/kernel/module_signing.c
+++ b/kernel/module_signing.c
@@ -25,35 +25,16 @@ int verify_appended_signature(const void *data, unsigned long *len,
 			      struct key *trusted_keys,
 			      enum key_being_used_for purpose)
 {
-	const unsigned long markerlen = sizeof(MODULE_SIG_STRING) - 1;
-	struct module_signature *ms;
-	unsigned long sig_len, modlen = *len;
+	unsigned long sig_len;
 	int ret;
 
-	pr_devel("==>%s %s(,%lu)\n", __func__, key_being_used_for[purpose], modlen);
+	pr_devel("==>%s %s(,%lu)\n", __func__, key_being_used_for[purpose], *len);
 
-	if (markerlen > modlen)
-		return -ENODATA;
-
-	if (memcmp(data + modlen - markerlen, MODULE_SIG_STRING,
-		   markerlen))
-		return -ENODATA;
-	modlen -= markerlen;
-
-	if (modlen <= sizeof(*ms))
-		return -EBADMSG;
-
-	ms = data + modlen - sizeof(*ms);
-
-	ret = mod_check_sig(ms, modlen, key_being_used_for[purpose]);
+	ret = mod_parse_sig(data, len, &sig_len, key_being_used_for[purpose]);
 	if (ret)
 		return ret;
 
-	sig_len = be32_to_cpu(ms->sig_len);
-	modlen -= sig_len + sizeof(*ms);
-	*len = modlen;
-
-	return verify_pkcs7_signature(data, modlen, data + modlen, sig_len,
+	return verify_pkcs7_signature(data, *len, data + *len, sig_len,
 				      trusted_keys,
 				      purpose,
 				      NULL, NULL);
diff --git a/security/integrity/ima/ima_modsig.c b/security/integrity/ima/ima_modsig.c
index fb25723c65bc..b40c8fdf6139 100644
--- a/security/integrity/ima/ima_modsig.c
+++ b/security/integrity/ima/ima_modsig.c
@@ -37,33 +37,17 @@ struct modsig {
  *
  * Return: 0 on success, error code otherwise.
  */
-int ima_read_modsig(enum ima_hooks func, const void *buf, loff_t buf_len,
+int ima_read_modsig(enum ima_hooks func, const void *buf, loff_t len,
 		    struct modsig **modsig)
 {
-	const size_t marker_len = strlen(MODULE_SIG_STRING);
-	const struct module_signature *sig;
 	struct modsig *hdr;
-	size_t sig_len;
-	const void *p;
+	unsigned long sig_len, buf_len = len;
 	int rc;
 
-	if (buf_len <= marker_len + sizeof(*sig))
-		return -ENOENT;
-
-	p = buf + buf_len - marker_len;
-	if (memcmp(p, MODULE_SIG_STRING, marker_len))
-		return -ENOENT;
-
-	buf_len -= marker_len;
-	sig = (const struct module_signature *)(p - sizeof(*sig));
-
-	rc = mod_check_sig(sig, buf_len, func_tokens[func]);
+	rc = mod_parse_sig(buf, &buf_len, &sig_len, func_tokens[func]);
 	if (rc)
 		return rc;
 
-	sig_len = be32_to_cpu(sig->sig_len);
-	buf_len -= sig_len + sizeof(*sig);
-
 	/* Allocate sig_len additional bytes to hold the raw PKCS#7 data. */
 	hdr = kzalloc(sizeof(*hdr) + sig_len, GFP_KERNEL);
 	if (!hdr)
-- 
2.31.1


^ permalink raw reply related

* Re: [PATCH V2 11/17] riscv: compat: Add elf.h implementation
From: Arnd Bergmann @ 2022-01-10 14:28 UTC (permalink / raw)
  To: Guo Ren
  Cc: linux-s390, the arch/x86 maintainers, Arnd Bergmann, Drew Fustini,
	gregkh, Wang Junqiang, Anup Patel, Linux Kernel Mailing List,
	linux-csky, open list:BROADCOM NVRAM DRIVER, Christoph Hellwig,
	Guo Ren, Palmer Dabbelt, liush, sparclinux, linux-riscv,
	linuxppc-dev, inux-parisc, Linux ARM, Wei Fu
In-Reply-To: <20211228143958.3409187-12-guoren@kernel.org>

On Tue, Dec 28, 2021 at 3:39 PM <guoren@kernel.org> wrote:
>
> From: Guo Ren <guoren@linux.alibaba.com>
>
> Implement necessary type and macro for compat elf. See the code
> comment for detail.
>
> Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> Signed-off-by: Guo Ren <guoren@kernel.org>
> Cc: Arnd Bergmann <arnd@arndb.de>

This looks mostly correct,

> +/*
> + * FIXME: not sure SET_PERSONALITY for compat process is right!
> + */
> +#define SET_PERSONALITY(ex)                                               \
> +do {    if ((ex).e_ident[EI_CLASS] == ELFCLASS32)                         \
> +               set_thread_flag(TIF_32BIT);                                \
> +       else                                                               \
> +               clear_thread_flag(TIF_32BIT);                              \
> +       set_personality(PER_LINUX | (current->personality & (~PER_MASK))); \
> +} while (0)

This means the personality after exec is always set to PER_LINUX, not
PER_LINUX32, which I think is wrong: you want the PER_LINUX32
setting to stick, just like the upper bits do in the default implementation.

What the other ones do is:

| arch/parisc/include/asm/elf.h-
set_personality((current->personality & ~PER_MASK) | PER_LINUX); \

This looks like the same problem you introduce here: always forcing PER_LINUX
instead of PER_LINUX32 makes it impossible to use PER_LINUX32.

| arch/alpha/include/asm/elf.h:#define SET_PERSONALITY(EX)
                           \
| arch/alpha/include/asm/elf.h-   set_personality(((EX).e_flags &
EF_ALPHA_32BIT)         \
| arch/alpha/include/asm/elf.h-      ? PER_LINUX_32BIT : PER_LINUX)
| arch/csky/include/asm/elf.h:#define SET_PERSONALITY(ex)
set_personality(PER_LINUX)
| arch/nds32/include/asm/elf.h:#define SET_PERSONALITY(ex)
set_personality(PER_LINUX)

These look even worse: instead of forcing the lower bits to
PER_LINUX/PER_LINUX32 and
leaving the upper bits untouched, these also clear the upper bits
unconditionally.

| arch/arm64/include/asm/elf.h:#define SET_PERSONALITY(ex)
                                   \
| arch/arm64/include/asm/elf.h-   current->personality &=
~READ_IMPLIES_EXEC;                     \
| arch/x86/um/asm/elf.h:#define SET_PERSONALITY(ex) do {} while(0)
| arch/x86/include/asm/elf.h:#define set_personality_64bit()      do {
} while (0)
| arch/x86/kernel/process_64.c:static void __set_personality_ia32(void)
|         current->personality |= force_personality32;

Inconsistent: does not enforce PER_LINUX/PER_LINUX32 as the default
implementation
does, but just leaves the value untouched (other than (re)setting
READ_IMPLIES_EXEC).
I think this is harmless otherwise, as we generally ignore the lower
bits, except for the
bit of code that checks for PER_LINUX32 in override_architecture() to mangle the
output of sys_newuname() or in /proc/cpuinfo.

| arch/s390/include/asm/elf.h-    if
(personality(current->personality) != PER_LINUX32)   \
| arch/s390/include/asm/elf.h-            set_personality(PER_LINUX |
                   \
| arch/s390/include/asm/elf.h-
(current->personality & ~PER_MASK));    \
| arch/powerpc/include/asm/elf.h- if
(personality(current->personality) != PER_LINUX32)   \
| arch/powerpc/include/asm/elf.h-         set_personality(PER_LINUX |
                   \
| arch/powerpc/include/asm/elf.h-
(current->personality & (~PER_MASK)));  \
| arch/sparc/include/asm/elf_64.h-        if
(personality(current->personality) != PER_LINUX32)   \
| arch/sparc/include/asm/elf_64.h-
set_personality(PER_LINUX |             \
| arch/sparc/include/asm/elf_64.h-
(current->personality & (~PER_MASK)));  \

This is probably the behavior you want to copy.

      Arnd

^ permalink raw reply

* [PATCH v3] powerpc/32s: Fix kasan_init_region() for KASAN
From: Christophe Leroy @ 2022-01-10 15:29 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: Maxime Bizon, stable@vger.kernel.org,
	linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org

It has been reported some configuration where the kernel doesn't
boot with KASAN enabled.

This is due to wrong BAT allocation for the KASAN area:

	---[ Data Block Address Translation ]---
	0: 0xc0000000-0xcfffffff 0x00000000       256M Kernel rw      m
	1: 0xd0000000-0xdfffffff 0x10000000       256M Kernel rw      m
	2: 0xe0000000-0xefffffff 0x20000000       256M Kernel rw      m
	3: 0xf8000000-0xf9ffffff 0x2a000000        32M Kernel rw      m
	4: 0xfa000000-0xfdffffff 0x2c000000        64M Kernel rw      m

A BAT must have both virtual and physical addresses alignment matching
the size of the BAT. This is not the case for BAT 4 above.

Fix kasan_init_region() by using block_size() function that is in
book3s32/mmu.c. To be able to reuse it here, make it non static and
change its name to bat_block_size() in order to avoid name conflict
with block_size() defined in <linux/blkdev.h>

Also reuse find_free_bat() to avoid an error message from setbat()
when no BAT is available.

And allocate memory outside of linear memory mapping to avoid
wasting that precious space.

With this change we get correct alignment for BATs and KASAN shadow
memory is allocated outside the linear memory space.

	---[ Data Block Address Translation ]---
	0: 0xc0000000-0xcfffffff 0x00000000       256M Kernel rw
	1: 0xd0000000-0xdfffffff 0x10000000       256M Kernel rw
	2: 0xe0000000-0xefffffff 0x20000000       256M Kernel rw
	3: 0xf8000000-0xfbffffff 0x7c000000        64M Kernel rw
	4: 0xfc000000-0xfdffffff 0x7a000000        32M Kernel rw

Reported-by: Maxime Bizon <mbizon@freebox.fr>
Fixes: 7974c4732642 ("powerpc/32s: Implement dedicated kasan_init_region()")
Cc: stable@vger.kernel.org
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Tested-by: Maxime Bizon <mbizon@freebox.fr>
---
v3: Rebased

v2:
- Allocate kasan shadow memory outside precious kernel linear memory
- Properly zeroise kasan shadow memory
---
 arch/powerpc/include/asm/book3s/32/mmu-hash.h |  2 +
 arch/powerpc/mm/book3s32/mmu.c                | 10 ++--
 arch/powerpc/mm/kasan/book3s_32.c             | 59 ++++++++++---------
 3 files changed, 38 insertions(+), 33 deletions(-)

diff --git a/arch/powerpc/include/asm/book3s/32/mmu-hash.h b/arch/powerpc/include/asm/book3s/32/mmu-hash.h
index 7be27862329f..78c6a5fde1d6 100644
--- a/arch/powerpc/include/asm/book3s/32/mmu-hash.h
+++ b/arch/powerpc/include/asm/book3s/32/mmu-hash.h
@@ -223,6 +223,8 @@ static __always_inline void update_user_segments(u32 val)
 	update_user_segment(15, val);
 }
 
+int __init find_free_bat(void);
+unsigned int bat_block_size(unsigned long base, unsigned long top);
 #endif /* !__ASSEMBLY__ */
 
 /* We happily ignore the smaller BATs on 601, we don't actually use
diff --git a/arch/powerpc/mm/book3s32/mmu.c b/arch/powerpc/mm/book3s32/mmu.c
index 94045b265b6b..203735caf691 100644
--- a/arch/powerpc/mm/book3s32/mmu.c
+++ b/arch/powerpc/mm/book3s32/mmu.c
@@ -76,7 +76,7 @@ unsigned long p_block_mapped(phys_addr_t pa)
 	return 0;
 }
 
-static int __init find_free_bat(void)
+int __init find_free_bat(void)
 {
 	int b;
 	int n = mmu_has_feature(MMU_FTR_USE_HIGH_BATS) ? 8 : 4;
@@ -100,7 +100,7 @@ static int __init find_free_bat(void)
  * - block size has to be a power of two. This is calculated by finding the
  *   highest bit set to 1.
  */
-static unsigned int block_size(unsigned long base, unsigned long top)
+unsigned int bat_block_size(unsigned long base, unsigned long top)
 {
 	unsigned int max_size = SZ_256M;
 	unsigned int base_shift = (ffs(base) - 1) & 31;
@@ -145,7 +145,7 @@ static unsigned long __init __mmu_mapin_ram(unsigned long base, unsigned long to
 	int idx;
 
 	while ((idx = find_free_bat()) != -1 && base != top) {
-		unsigned int size = block_size(base, top);
+		unsigned int size = bat_block_size(base, top);
 
 		if (size < 128 << 10)
 			break;
@@ -201,12 +201,12 @@ void mmu_mark_initmem_nx(void)
 	unsigned long size;
 
 	for (i = 0; i < nb - 1 && base < top;) {
-		size = block_size(base, top);
+		size = bat_block_size(base, top);
 		setibat(i++, PAGE_OFFSET + base, base, size, PAGE_KERNEL_TEXT);
 		base += size;
 	}
 	if (base < top) {
-		size = block_size(base, top);
+		size = bat_block_size(base, top);
 		if ((top - base) > size) {
 			size <<= 1;
 			if (strict_kernel_rwx_enabled() && base + size > border)
diff --git a/arch/powerpc/mm/kasan/book3s_32.c b/arch/powerpc/mm/kasan/book3s_32.c
index 35b287b0a8da..450a67ef0bbe 100644
--- a/arch/powerpc/mm/kasan/book3s_32.c
+++ b/arch/powerpc/mm/kasan/book3s_32.c
@@ -10,48 +10,51 @@ int __init kasan_init_region(void *start, size_t size)
 {
 	unsigned long k_start = (unsigned long)kasan_mem_to_shadow(start);
 	unsigned long k_end = (unsigned long)kasan_mem_to_shadow(start + size);
-	unsigned long k_cur = k_start;
-	int k_size = k_end - k_start;
-	int k_size_base = 1 << (ffs(k_size) - 1);
+	unsigned long k_nobat = k_start;
+	unsigned long k_cur;
+	phys_addr_t phys;
 	int ret;
-	void *block;
 
-	block = memblock_alloc(k_size, k_size_base);
-
-	if (block && k_size_base >= SZ_128K && k_start == ALIGN(k_start, k_size_base)) {
-		int shift = ffs(k_size - k_size_base);
-		int k_size_more = shift ? 1 << (shift - 1) : 0;
-
-		setbat(-1, k_start, __pa(block), k_size_base, PAGE_KERNEL);
-		if (k_size_more >= SZ_128K)
-			setbat(-1, k_start + k_size_base, __pa(block) + k_size_base,
-			       k_size_more, PAGE_KERNEL);
-		if (v_block_mapped(k_start))
-			k_cur = k_start + k_size_base;
-		if (v_block_mapped(k_start + k_size_base))
-			k_cur = k_start + k_size_base + k_size_more;
-
-		update_bats();
+	while (k_nobat < k_end) {
+		unsigned int k_size = bat_block_size(k_nobat, k_end);
+		int idx = find_free_bat();
+
+		if (idx == -1)
+			break;
+		if (k_size < SZ_128K)
+			break;
+		phys = memblock_phys_alloc_range(k_size, k_size, 0,
+						 MEMBLOCK_ALLOC_ANYWHERE);
+		if (!phys)
+			break;
+
+		setbat(idx, k_nobat, phys, k_size, PAGE_KERNEL);
+		k_nobat += k_size;
 	}
+	if (k_nobat != k_start)
+		update_bats();
 
-	if (!block)
-		block = memblock_alloc(k_size, PAGE_SIZE);
-	if (!block)
-		return -ENOMEM;
+	if (k_nobat < k_end) {
+		phys = memblock_phys_alloc_range(k_end - k_nobat, PAGE_SIZE, 0,
+						 MEMBLOCK_ALLOC_ANYWHERE);
+		if (!phys)
+			return -ENOMEM;
+	}
 
 	ret = kasan_init_shadow_page_tables(k_start, k_end);
 	if (ret)
 		return ret;
 
-	kasan_update_early_region(k_start, k_cur, __pte(0));
+	kasan_update_early_region(k_start, k_nobat, __pte(0));
 
-	for (; k_cur < k_end; k_cur += PAGE_SIZE) {
+	for (k_cur = k_nobat; k_cur < k_end; k_cur += PAGE_SIZE) {
 		pmd_t *pmd = pmd_off_k(k_cur);
-		void *va = block + k_cur - k_start;
-		pte_t pte = pfn_pte(PHYS_PFN(__pa(va)), PAGE_KERNEL);
+		pte_t pte = pfn_pte(PHYS_PFN(phys + k_cur - k_nobat), PAGE_KERNEL);
 
 		__set_pte_at(&init_mm, k_cur, pte_offset_kernel(pmd, k_cur), pte, 0);
 	}
 	flush_tlb_kernel_range(k_start, k_end);
+	memset(kasan_mem_to_shadow(start), 0, k_end - k_start);
+
 	return 0;
 }
-- 
2.33.1

^ permalink raw reply related

* [PATCH] powerpc/time: Fix build failure due to do_hard_irq_enable() on PPC32
From: Christophe Leroy @ 2022-01-10 15:29 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org,
	Nicholas Piggin

	  CC      arch/powerpc/kernel/time.o
	In file included from <command-line>:
	./arch/powerpc/include/asm/hw_irq.h: In function 'do_hard_irq_enable':
	././include/linux/compiler_types.h:335:45: error: call to '__compiletime_assert_35' declared with attribute error: BUILD_BUG failed
	  335 |         _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
	      |                                             ^
	././include/linux/compiler_types.h:316:25: note: in definition of macro '__compiletime_assert'
	  316 |                         prefix ## suffix();                             \
	      |                         ^~~~~~
	././include/linux/compiler_types.h:335:9: note: in expansion of macro '_compiletime_assert'
	  335 |         _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
	      |         ^~~~~~~~~~~~~~~~~~~
	./include/linux/build_bug.h:39:37: note: in expansion of macro 'compiletime_assert'
	   39 | #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
	      |                                     ^~~~~~~~~~~~~~~~~~
	./include/linux/build_bug.h:59:21: note: in expansion of macro 'BUILD_BUG_ON_MSG'
	   59 | #define BUILD_BUG() BUILD_BUG_ON_MSG(1, "BUILD_BUG failed")
	      |                     ^~~~~~~~~~~~~~~~
	./arch/powerpc/include/asm/hw_irq.h:483:9: note: in expansion of macro 'BUILD_BUG'
	  483 |         BUILD_BUG();
	      |         ^~~~~~~~~

should_hard_irq_enable() returns false on PPC32 so this BUILD_BUG() shouldn't trigger.

Force inlining of should_hard_irq_enable()

Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Fixes: 0faf20a1ad16 ("powerpc/64s/interrupt: Don't enable MSR[EE] in irq handlers unless perf is in use")
Cc: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/include/asm/hw_irq.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/include/asm/hw_irq.h b/arch/powerpc/include/asm/hw_irq.h
index a58fb4aa6c81..674e5aaafcbd 100644
--- a/arch/powerpc/include/asm/hw_irq.h
+++ b/arch/powerpc/include/asm/hw_irq.h
@@ -473,7 +473,7 @@ static inline bool arch_irq_disabled_regs(struct pt_regs *regs)
 	return !(regs->msr & MSR_EE);
 }
 
-static inline bool should_hard_irq_enable(void)
+static __always_inline bool should_hard_irq_enable(void)
 {
 	return false;
 }
-- 
2.33.1

^ permalink raw reply related

* Re: [PATCH V2 03/17] asm-generic: fcntl: compat: Remove duplicate definitions
From: Christophe Leroy @ 2022-01-10 15:56 UTC (permalink / raw)
  To: guoren@kernel.org, palmer@dabbelt.com, arnd@arndb.de,
	anup.patel@wdc.com, gregkh@linuxfoundation.org,
	liush@allwinnertech.com, wefu@redhat.com, drew@beagleboard.org,
	wangjunqiang@iscas.ac.cn, hch@infradead.org
  Cc: linux-s390@vger.kernel.org, Guo Ren, x86@kernel.org,
	linux-mips@vger.kernel.org, linux-csky@vger.kernel.org,
	linux-kernel@vger.kernel.org, sparclinux@vger.kernel.org,
	linux-riscv@lists.infradead.org, linuxppc-dev@lists.ozlabs.org,
	inux-parisc@vger.kernel.org, linux-arm-kernel@lists.infradead.org
In-Reply-To: <20211228143958.3409187-4-guoren@kernel.org>



Le 28/12/2021 à 15:39, guoren@kernel.org a écrit :
> From: Guo Ren <guoren@linux.alibaba.com>
> 
> Remove duplicate F_GETLK64,F_SETLK64,F_SETLKW64 definitions in
> arch/*/include/asm/compat.h.
> 
> Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> Signed-off-by: Guo Ren <guoren@kernel.org>
> Cc: Arnd Bergmann <arnd@arndb.de>
> ---
>   arch/arm64/include/asm/compat.h   | 4 ----
>   arch/powerpc/include/asm/compat.h | 4 ----
>   arch/s390/include/asm/compat.h    | 4 ----
>   arch/sparc/include/asm/compat.h   | 4 ----
>   arch/x86/include/asm/compat.h     | 4 ----
>   include/uapi/asm-generic/fcntl.h  | 2 +-
>   6 files changed, 1 insertion(+), 21 deletions(-)
> 

...

> diff --git a/include/uapi/asm-generic/fcntl.h b/include/uapi/asm-generic/fcntl.h
> index ecd0f5bdfc1d..5bc1e51d73b1 100644
> --- a/include/uapi/asm-generic/fcntl.h
> +++ b/include/uapi/asm-generic/fcntl.h
> @@ -116,7 +116,7 @@
>   #define F_GETSIG	11	/* for sockets. */
>   #endif
>   
> -#ifndef CONFIG_64BIT
> +#if !defined(CONFIG_64BIT) || defined(CONFIG_COMPAT)
>   #ifndef F_GETLK64
>   #define F_GETLK64	12	/*  using 'struct flock64' */
>   #define F_SETLK64	13

There seems to be a problem with this change:

error: /linux/include/uapi/asm-generic/fcntl.h: leak CONFIG_COMPAT to 
user-space
make[3]: *** [/linux/scripts/Makefile.headersinst:63: 
usr/include/asm-generic/fcntl.h] Error 1
make[3]: *** Waiting for unfinished jobs....
make[2]: *** [/linux/Makefile:1283: headers] Error 2
make[1]: *** [Makefile:219: __sub-make] Error 2
make[2]: Leaving directory '/output'
make[1]: Leaving directory '/linux'
make: *** [Makefile:157: khdr] Error 2
make: Leaving directory '/linux/tools/testing/selftests'
## Selftest build completed rc = 2
## Found 2 binaries
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
!! Error build failed rc 2
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
Error: Process completed with exit code 2.

^ permalink raw reply

* Re: [PATCH V2 03/17] asm-generic: fcntl: compat: Remove duplicate definitions
From: Christoph Hellwig @ 2022-01-10 16:30 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Guo Ren, open list:BROADCOM NVRAM DRIVER, J. Bruce Fields,
	Guo Ren, sparclinux, linux-riscv, inux-parisc, linux-s390,
	the arch/x86 maintainers, linux-csky, Christoph Hellwig,
	Linux ARM, Drew Fustini, liush, Wang Junqiang, Anup Patel,
	Jeff Layton, Linux Kernel Mailing List, Palmer Dabbelt, gregkh,
	Linux FS-devel Mailing List, linuxppc-dev, Wei Fu
In-Reply-To: <CAK8P3a2zn9M6X09WsjJ9HYiS9WnO_YPCvJLSBk+HaH+yZHQqfA@mail.gmail.com>

On Mon, Jan 10, 2022 at 02:35:19PM +0100, Arnd Bergmann wrote:
> > +#if !defined(CONFIG_64BIT) || defined(CONFIG_COMPAT)
> >  #ifndef F_GETLK64
> >  #define F_GETLK64      12      /*  using 'struct flock64' */
> >  #define F_SETLK64      13
> 
> The problem here is that include/uapi/ headers cannot contain checks for
> CONFIG_* symbols because those may have different meanings in user space
> compared to kernel.
> 
> This is a preexisting problem in the header, but I think the change
> makes it worse.

FYI, this is what I did in my old branch, which also sidesteps the
duplicate value problem on parisc. The rebase is untested so far,
but I can spend some cycles on finishing it:

http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/fcntl-asm-generic-cleanup

^ permalink raw reply

* Re: [PATCH 00/16] Remove usage of the deprecated "pci-dma-compat.h" API
From: Martin K. Petersen @ 2022-01-10 15:35 UTC (permalink / raw)
  To: Christophe JAILLET
  Cc: airlied, trix, linux-fpga, linux-pci, paulus, sparclinux,
	kernel-janitors, linux-scsi, sathya.prakash, hch,
	MPT-FusionLinux.pdl, hao.wu, arnd, suganath-prabu.subramani,
	sreekanth.reddy, ink, bhelgaas, mchehab, mattst88, rth, awalls,
	linux-kernel, davem, alex.bou9, vkoul, linux-alpha, dmaengine,
	mdf, akpm, linux-media, linuxppc-dev, yilun.xu
In-Reply-To: <cover.1641500561.git.christophe.jaillet@wanadoo.fr>


Christophe,

> This serie axes all the remaining usages of the deprecated
> "pci-dma-compat.h" API.

Applied patches 10-15 to 5.17/scsi-staging, thanks!

-- 
Martin K. Petersen	Oracle Linux Engineering

^ permalink raw reply

* Re: [PATCH] powerpc: dts: t1040rdb: fix ports names for Seville Ethernet switch
From: Maxim Kiselev @ 2022-01-10  7:40 UTC (permalink / raw)
  To: Vladimir Oltean
  Cc: Andrew Lunn, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, fido_max@inbox.ru, Rob Herring,
	Paul Mackerras, linuxppc-dev@lists.ozlabs.org, David S. Miller
In-Reply-To: <20211231125247.ugne3h44pmpzliin@skbuf>


[-- Attachment #1.1: Type: text/plain, Size: 30 bytes --]

Here are photos of my boards.

[-- Attachment #1.2: Type: text/html, Size: 55 bytes --]

[-- Attachment #2: t1040__hw.jpg --]
[-- Type: image/jpeg, Size: 135377 bytes --]

[-- Attachment #3: t1040__front.jpg --]
[-- Type: image/jpeg, Size: 159648 bytes --]

^ permalink raw reply

* [5.16.0] build error on unrecognized opcode: ptesync
From: Mike @ 2022-01-10 12:32 UTC (permalink / raw)
  To: linuxppc-dev

[-- Attachment #1: Type: text/plain, Size: 725 bytes --]

Hey, so I originally sat down to compile the fast headers V2 patch, but
quickly discovered other things at play, and grabbed 5.16.0 a few hours
after it lifted off,  arch/powerpc/mm/mmu_context.c I had to specifically
say had to include -maltivec or it barfed on a 'dssall', I'm fine with
that, I've spent years in kernel land, I can deal with that, then came
arch/powerpc/lib/step.c with the ptesync. This seems like a totally normal
instruction that shouldn't need any extra flags or anything, yet the
assembler throws up, and no flag I can think of fixes it. This is a G4
7447. I reverted back to the Debian 5.15. defconfig before dropping this
mail as I had tweaked my config to be more G4.

Best regards.
Michael Heltne

[-- Attachment #2: Type: text/html, Size: 836 bytes --]

^ permalink raw reply

* Re: [PATCH 3/6] KVM: Remove opaque from kvm_arch_check_processor_compat
From: Sean Christopherson @ 2022-01-10 23:06 UTC (permalink / raw)
  To: Chao Gao
  Cc: x86, Wanpeng Li, kvm, David Hildenbrand, Dave Hansen, linux-mips,
	Atish Patra, Paul Mackerras, H. Peter Anvin, Alexander Gordeev,
	Claudio Imbrenda, Will Deacon, Maciej S. Szmigiero, linux-s390,
	Janosch Frank, Marc Zyngier, Joerg Roedel, Huacai Chen,
	linux-riscv, kvmarm, Aleksandar Markovic, Ingo Molnar,
	Catalin Marinas, Palmer Dabbelt, Christian Borntraeger,
	Ravi Bangoria, kevin.tian, Albert Ou, Vasily Gorbik,
	Suzuki K Poulose, Heiko Carstens, Nicholas Piggin,
	Borislav Petkov, Cédric Le Goater, Paul Walmsley, tglx,
	Alexandru Elisei, linux-arm-kernel, Jim Mattson,
	Thomas Bogendoerfer, Fabiano Rosas, Anup Patel, linux-kernel,
	Bharata B Rao, James Morse, kvm-riscv, pbonzini, Vitaly Kuznetsov,
	linuxppc-dev
In-Reply-To: <20211227081515.2088920-4-chao.gao@intel.com>

On Mon, Dec 27, 2021, Chao Gao wrote:
> No arch implementation uses this opaque now.

Except for the RISC-V part, this can be a pure revert of commit b99040853738 ("KVM:
Pass kvm_init()'s opaque param to additional arch funcs").  I think it makes sense
to process it as a revert, with a short blurb in the changelog to note that RISC-V
is manually modified as RISC-V support came along in the interim.

^ permalink raw reply

* Re: [PATCH v3 5/6] KVM: PPC: mmio: Return to guest after emulation failure
From: Alexey Kardashevskiy @ 2022-01-10 23:51 UTC (permalink / raw)
  To: Nicholas Piggin, Fabiano Rosas, kvm-ppc; +Cc: linuxppc-dev
In-Reply-To: <1641799578.6dxlxsaaos.astroid@bobo.none>



On 1/10/22 18:36, Nicholas Piggin wrote:
> Excerpts from Fabiano Rosas's message of January 8, 2022 7:00 am:
>> If MMIO emulation fails we don't want to crash the whole guest by
>> returning to userspace.
>>
>> The original commit bbf45ba57eae ("KVM: ppc: PowerPC 440 KVM
>> implementation") added a todo:
>>
>>    /* XXX Deliver Program interrupt to guest. */
>>
>> and later the commit d69614a295ae ("KVM: PPC: Separate loadstore
>> emulation from priv emulation") added the Program interrupt injection
>> but in another file, so I'm assuming it was missed that this block
>> needed to be altered.
>>
>> Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
>> Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>> ---
>>   arch/powerpc/kvm/powerpc.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
>> index 6daeea4a7de1..56b0faab7a5f 100644
>> --- a/arch/powerpc/kvm/powerpc.c
>> +++ b/arch/powerpc/kvm/powerpc.c
>> @@ -309,7 +309,7 @@ int kvmppc_emulate_mmio(struct kvm_vcpu *vcpu)
>>   		kvmppc_get_last_inst(vcpu, INST_GENERIC, &last_inst);
>>   		kvmppc_core_queue_program(vcpu, 0);
>>   		pr_info("%s: emulation failed (%08x)\n", __func__, last_inst);
>> -		r = RESUME_HOST;
>> +		r = RESUME_GUEST;
> 
> So at this point can the pr_info just go away?
> 
> I wonder if this shouldn't be a DSI rather than a program check.
> DSI with DSISR[37] looks a bit more expected. Not that Linux
> probably does much with it but at least it would give a SIGBUS
> rather than SIGILL.

It does not like it is more expected to me, it is not about wrong memory 
attributes, it is the instruction itself which cannot execute.

DSISR[37]:
Set to 1 if the access is due to a lq, stq, lwat, ldat, lbarx, lharx, 
lwarx, ldarx, lqarx, stwat,
stdat, stbcx., sthcx., stwcx., stdcx., or stqcx. instruction that 
addresses storage that is Write
Through Required or Caching Inhibited; or if the access is due to a copy 
or paste. instruction
that addresses storage that is Caching Inhibited; or if the access is 
due to a lwat, ldat, stwat, or
stdat instruction that addresses storage that is Guarded; otherwise set 
to 0.

^ permalink raw reply

* [PATCH kernel v5] KVM: PPC: Merge powerpc's debugfs entry content into generic entry
From: Alexey Kardashevskiy @ 2022-01-11  0:54 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: Alexey Kardashevskiy, kvm, Cédric Le Goater, kvm-ppc,
	Fabiano Rosas

At the moment KVM on PPC creates 4 types of entries under the kvm debugfs:
1) "%pid-%fd" per a KVM instance (for all platforms);
2) "vm%pid" (for PPC Book3s HV KVM);
3) "vm%u_vcpu%u_timing" (for PPC Book3e KVM);
4) "kvm-xive-%p" (for XIVE PPC Book3s KVM, the same for XICS);

The problem with this is that multiple VMs per process is not allowed for
2) and 3) which makes it possible for userspace to trigger errors when
creating duplicated debugfs entries.

This merges all these into 1).

This defines kvm_arch_create_kvm_debugfs() similar to
kvm_arch_create_vcpu_debugfs().

This defines 2 hooks in kvmppc_ops that allow specific KVM implementations
add necessary entries, this adds the _e500 suffix to
kvmppc_create_vcpu_debugfs_e500() to make it clear what platform it is for.

This makes use of already existing kvm_arch_create_vcpu_debugfs() on PPC.

This removes no more used debugfs_dir pointers from PPC kvm_arch structs.

This stops removing vcpu entries as once created vcpus stay around
for the entire life of a VM and removed when the KVM instance is closed,
see commit d56f5136b010 ("KVM: let kvm_destroy_vm_debugfs clean up vCPU
debugfs directories").

Suggested-by: Fabiano Rosas <farosas@linux.ibm.com>
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
Changes:
v5:
* fixed e500mc2

v4:
* added "kvm-xive-%p"

v3:
* reworked commit log, especially, the bit about removing vcpus

v2:
* handled powerpc-booke
* s/kvm/vm/ in arch hooks
---
 arch/powerpc/include/asm/kvm_host.h    |  6 ++---
 arch/powerpc/include/asm/kvm_ppc.h     |  2 ++
 arch/powerpc/kvm/timing.h              | 12 +++++-----
 arch/powerpc/kvm/book3s_64_mmu_hv.c    |  2 +-
 arch/powerpc/kvm/book3s_64_mmu_radix.c |  2 +-
 arch/powerpc/kvm/book3s_hv.c           | 31 ++++++++++----------------
 arch/powerpc/kvm/book3s_xics.c         | 13 ++---------
 arch/powerpc/kvm/book3s_xive.c         | 13 ++---------
 arch/powerpc/kvm/book3s_xive_native.c  | 13 ++---------
 arch/powerpc/kvm/e500.c                |  1 +
 arch/powerpc/kvm/e500mc.c              |  1 +
 arch/powerpc/kvm/powerpc.c             | 16 ++++++++++---
 arch/powerpc/kvm/timing.c              | 21 +++++------------
 13 files changed, 51 insertions(+), 82 deletions(-)

diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h
index 17263276189e..f5e14fa683f4 100644
--- a/arch/powerpc/include/asm/kvm_host.h
+++ b/arch/powerpc/include/asm/kvm_host.h
@@ -26,6 +26,8 @@
 #include <asm/hvcall.h>
 #include <asm/mce.h>
 
+#define __KVM_HAVE_ARCH_VCPU_DEBUGFS
+
 #define KVM_MAX_VCPUS		NR_CPUS
 #define KVM_MAX_VCORES		NR_CPUS
 
@@ -295,7 +297,6 @@ struct kvm_arch {
 	bool dawr1_enabled;
 	pgd_t *pgtable;
 	u64 process_table;
-	struct dentry *debugfs_dir;
 	struct kvm_resize_hpt *resize_hpt; /* protected by kvm->lock */
 #endif /* CONFIG_KVM_BOOK3S_HV_POSSIBLE */
 #ifdef CONFIG_KVM_BOOK3S_PR_POSSIBLE
@@ -673,7 +674,6 @@ struct kvm_vcpu_arch {
 	u64 timing_min_duration[__NUMBER_OF_KVM_EXIT_TYPES];
 	u64 timing_max_duration[__NUMBER_OF_KVM_EXIT_TYPES];
 	u64 timing_last_exit;
-	struct dentry *debugfs_exit_timing;
 #endif
 
 #ifdef CONFIG_PPC_BOOK3S
@@ -829,8 +829,6 @@ struct kvm_vcpu_arch {
 	struct kvmhv_tb_accumulator rm_exit;	/* real-mode exit code */
 	struct kvmhv_tb_accumulator guest_time;	/* guest execution */
 	struct kvmhv_tb_accumulator cede_time;	/* time napping inside guest */
-
-	struct dentry *debugfs_dir;
 #endif /* CONFIG_KVM_BOOK3S_HV_EXIT_TIMING */
 };
 
diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/asm/kvm_ppc.h
index 33db83b82fbd..d2b192dea0d2 100644
--- a/arch/powerpc/include/asm/kvm_ppc.h
+++ b/arch/powerpc/include/asm/kvm_ppc.h
@@ -316,6 +316,8 @@ struct kvmppc_ops {
 	int (*svm_off)(struct kvm *kvm);
 	int (*enable_dawr1)(struct kvm *kvm);
 	bool (*hash_v3_possible)(void);
+	int (*create_vm_debugfs)(struct kvm *kvm);
+	int (*create_vcpu_debugfs)(struct kvm_vcpu *vcpu, struct dentry *debugfs_dentry);
 };
 
 extern struct kvmppc_ops *kvmppc_hv_ops;
diff --git a/arch/powerpc/kvm/timing.h b/arch/powerpc/kvm/timing.h
index feef7885ba82..45817ab82bb4 100644
--- a/arch/powerpc/kvm/timing.h
+++ b/arch/powerpc/kvm/timing.h
@@ -14,8 +14,8 @@
 #ifdef CONFIG_KVM_EXIT_TIMING
 void kvmppc_init_timing_stats(struct kvm_vcpu *vcpu);
 void kvmppc_update_timing_stats(struct kvm_vcpu *vcpu);
-void kvmppc_create_vcpu_debugfs(struct kvm_vcpu *vcpu, unsigned int id);
-void kvmppc_remove_vcpu_debugfs(struct kvm_vcpu *vcpu);
+int kvmppc_create_vcpu_debugfs_e500(struct kvm_vcpu *vcpu,
+				    struct dentry *debugfs_dentry);
 
 static inline void kvmppc_set_exit_type(struct kvm_vcpu *vcpu, int type)
 {
@@ -26,9 +26,11 @@ static inline void kvmppc_set_exit_type(struct kvm_vcpu *vcpu, int type)
 /* if exit timing is not configured there is no need to build the c file */
 static inline void kvmppc_init_timing_stats(struct kvm_vcpu *vcpu) {}
 static inline void kvmppc_update_timing_stats(struct kvm_vcpu *vcpu) {}
-static inline void kvmppc_create_vcpu_debugfs(struct kvm_vcpu *vcpu,
-						unsigned int id) {}
-static inline void kvmppc_remove_vcpu_debugfs(struct kvm_vcpu *vcpu) {}
+static inline int kvmppc_create_vcpu_debugfs_e500(struct kvm_vcpu *vcpu,
+						  struct dentry *debugfs_dentry)
+{
+	return 0;
+}
 static inline void kvmppc_set_exit_type(struct kvm_vcpu *vcpu, int type) {}
 #endif /* CONFIG_KVM_EXIT_TIMING */
 
diff --git a/arch/powerpc/kvm/book3s_64_mmu_hv.c b/arch/powerpc/kvm/book3s_64_mmu_hv.c
index c63e263312a4..33dae253a0ac 100644
--- a/arch/powerpc/kvm/book3s_64_mmu_hv.c
+++ b/arch/powerpc/kvm/book3s_64_mmu_hv.c
@@ -2112,7 +2112,7 @@ static const struct file_operations debugfs_htab_fops = {
 
 void kvmppc_mmu_debugfs_init(struct kvm *kvm)
 {
-	debugfs_create_file("htab", 0400, kvm->arch.debugfs_dir, kvm,
+	debugfs_create_file("htab", 0400, kvm->debugfs_dentry, kvm,
 			    &debugfs_htab_fops);
 }
 
diff --git a/arch/powerpc/kvm/book3s_64_mmu_radix.c b/arch/powerpc/kvm/book3s_64_mmu_radix.c
index 8cebe5542256..e4ce2a35483f 100644
--- a/arch/powerpc/kvm/book3s_64_mmu_radix.c
+++ b/arch/powerpc/kvm/book3s_64_mmu_radix.c
@@ -1454,7 +1454,7 @@ static const struct file_operations debugfs_radix_fops = {
 
 void kvmhv_radix_debugfs_init(struct kvm *kvm)
 {
-	debugfs_create_file("radix", 0400, kvm->arch.debugfs_dir, kvm,
+	debugfs_create_file("radix", 0400, kvm->debugfs_dentry, kvm,
 			    &debugfs_radix_fops);
 }
 
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index f64e45d6c0f4..1e282045f4a7 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -2768,20 +2768,17 @@ static const struct file_operations debugfs_timings_ops = {
 };
 
 /* Create a debugfs directory for the vcpu */
-static void debugfs_vcpu_init(struct kvm_vcpu *vcpu, unsigned int id)
+static int kvmppc_arch_create_vcpu_debugfs_hv(struct kvm_vcpu *vcpu, struct dentry *debugfs_dentry)
 {
-	char buf[16];
-	struct kvm *kvm = vcpu->kvm;
-
-	snprintf(buf, sizeof(buf), "vcpu%u", id);
-	vcpu->arch.debugfs_dir = debugfs_create_dir(buf, kvm->arch.debugfs_dir);
-	debugfs_create_file("timings", 0444, vcpu->arch.debugfs_dir, vcpu,
+	debugfs_create_file("timings", 0444, debugfs_dentry, vcpu,
 			    &debugfs_timings_ops);
+	return 0;
 }
 
 #else /* CONFIG_KVM_BOOK3S_HV_EXIT_TIMING */
-static void debugfs_vcpu_init(struct kvm_vcpu *vcpu, unsigned int id)
+static int kvmppc_arch_create_vcpu_debugfs_hv(struct kvm_vcpu *vcpu, struct dentry *debugfs_dentry)
 {
+	return 0;
 }
 #endif /* CONFIG_KVM_BOOK3S_HV_EXIT_TIMING */
 
@@ -2904,8 +2901,6 @@ static int kvmppc_core_vcpu_create_hv(struct kvm_vcpu *vcpu)
 	vcpu->arch.cpu_type = KVM_CPU_3S_64;
 	kvmppc_sanity_check(vcpu);
 
-	debugfs_vcpu_init(vcpu, id);
-
 	return 0;
 }
 
@@ -5227,7 +5222,6 @@ void kvmppc_free_host_rm_ops(void)
 static int kvmppc_core_init_vm_hv(struct kvm *kvm)
 {
 	unsigned long lpcr, lpid;
-	char buf[32];
 	int ret;
 
 	mutex_init(&kvm->arch.uvmem_lock);
@@ -5360,15 +5354,14 @@ static int kvmppc_core_init_vm_hv(struct kvm *kvm)
 		kvm->arch.smt_mode = 1;
 	kvm->arch.emul_smt_mode = 1;
 
-	/*
-	 * Create a debugfs directory for the VM
-	 */
-	snprintf(buf, sizeof(buf), "vm%d", current->pid);
-	kvm->arch.debugfs_dir = debugfs_create_dir(buf, kvm_debugfs_dir);
+	return 0;
+}
+
+static int kvmppc_arch_create_vm_debugfs_hv(struct kvm *kvm)
+{
 	kvmppc_mmu_debugfs_init(kvm);
 	if (radix_enabled())
 		kvmhv_radix_debugfs_init(kvm);
-
 	return 0;
 }
 
@@ -5383,8 +5376,6 @@ static void kvmppc_free_vcores(struct kvm *kvm)
 
 static void kvmppc_core_destroy_vm_hv(struct kvm *kvm)
 {
-	debugfs_remove_recursive(kvm->arch.debugfs_dir);
-
 	if (!cpu_has_feature(CPU_FTR_ARCH_300))
 		kvm_hv_vm_deactivated();
 
@@ -6045,6 +6036,8 @@ static struct kvmppc_ops kvm_ops_hv = {
 	.svm_off = kvmhv_svm_off,
 	.enable_dawr1 = kvmhv_enable_dawr1,
 	.hash_v3_possible = kvmppc_hash_v3_possible,
+	.create_vcpu_debugfs = kvmppc_arch_create_vcpu_debugfs_hv,
+	.create_vm_debugfs = kvmppc_arch_create_vm_debugfs_hv,
 };
 
 static int kvm_init_subcore_bitmap(void)
diff --git a/arch/powerpc/kvm/book3s_xics.c b/arch/powerpc/kvm/book3s_xics.c
index ebd5d920de8c..3dfa9285e4a4 100644
--- a/arch/powerpc/kvm/book3s_xics.c
+++ b/arch/powerpc/kvm/book3s_xics.c
@@ -1016,19 +1016,10 @@ DEFINE_SHOW_ATTRIBUTE(xics_debug);
 
 static void xics_debugfs_init(struct kvmppc_xics *xics)
 {
-	char *name;
-
-	name = kasprintf(GFP_KERNEL, "kvm-xics-%p", xics);
-	if (!name) {
-		pr_err("%s: no memory for name\n", __func__);
-		return;
-	}
-
-	xics->dentry = debugfs_create_file(name, 0444, arch_debugfs_dir,
+	xics->dentry = debugfs_create_file("xics", 0444, xics->kvm->debugfs_dentry,
 					   xics, &xics_debug_fops);
 
-	pr_debug("%s: created %s\n", __func__, name);
-	kfree(name);
+	pr_debug("%s: created\n", __func__);
 }
 
 static struct kvmppc_ics *kvmppc_xics_create_ics(struct kvm *kvm,
diff --git a/arch/powerpc/kvm/book3s_xive.c b/arch/powerpc/kvm/book3s_xive.c
index 225008882958..bb41afbb68fc 100644
--- a/arch/powerpc/kvm/book3s_xive.c
+++ b/arch/powerpc/kvm/book3s_xive.c
@@ -2351,19 +2351,10 @@ DEFINE_SHOW_ATTRIBUTE(xive_debug);
 
 static void xive_debugfs_init(struct kvmppc_xive *xive)
 {
-	char *name;
-
-	name = kasprintf(GFP_KERNEL, "kvm-xive-%p", xive);
-	if (!name) {
-		pr_err("%s: no memory for name\n", __func__);
-		return;
-	}
-
-	xive->dentry = debugfs_create_file(name, S_IRUGO, arch_debugfs_dir,
+	xive->dentry = debugfs_create_file("xive", S_IRUGO, xive->kvm->debugfs_dentry,
 					   xive, &xive_debug_fops);
 
-	pr_debug("%s: created %s\n", __func__, name);
-	kfree(name);
+	pr_debug("%s: created\n", __func__);
 }
 
 static void kvmppc_xive_init(struct kvm_device *dev)
diff --git a/arch/powerpc/kvm/book3s_xive_native.c b/arch/powerpc/kvm/book3s_xive_native.c
index 99db9ac49901..e86f5b6c2ae1 100644
--- a/arch/powerpc/kvm/book3s_xive_native.c
+++ b/arch/powerpc/kvm/book3s_xive_native.c
@@ -1259,19 +1259,10 @@ DEFINE_SHOW_ATTRIBUTE(xive_native_debug);
 
 static void xive_native_debugfs_init(struct kvmppc_xive *xive)
 {
-	char *name;
-
-	name = kasprintf(GFP_KERNEL, "kvm-xive-%p", xive);
-	if (!name) {
-		pr_err("%s: no memory for name\n", __func__);
-		return;
-	}
-
-	xive->dentry = debugfs_create_file(name, 0444, arch_debugfs_dir,
+	xive->dentry = debugfs_create_file("xive", 0444, xive->kvm->debugfs_dentry,
 					   xive, &xive_native_debug_fops);
 
-	pr_debug("%s: created %s\n", __func__, name);
-	kfree(name);
+	pr_debug("%s: created\n", __func__);
 }
 
 static void kvmppc_xive_native_init(struct kvm_device *dev)
diff --git a/arch/powerpc/kvm/e500.c b/arch/powerpc/kvm/e500.c
index 7e8b69015d20..c8b2b4478545 100644
--- a/arch/powerpc/kvm/e500.c
+++ b/arch/powerpc/kvm/e500.c
@@ -495,6 +495,7 @@ static struct kvmppc_ops kvm_ops_e500 = {
 	.emulate_op = kvmppc_core_emulate_op_e500,
 	.emulate_mtspr = kvmppc_core_emulate_mtspr_e500,
 	.emulate_mfspr = kvmppc_core_emulate_mfspr_e500,
+	.create_vcpu_debugfs = kvmppc_create_vcpu_debugfs_e500,
 };
 
 static int __init kvmppc_e500_init(void)
diff --git a/arch/powerpc/kvm/e500mc.c b/arch/powerpc/kvm/e500mc.c
index 1c189b5aadcc..fa0d8dbbe484 100644
--- a/arch/powerpc/kvm/e500mc.c
+++ b/arch/powerpc/kvm/e500mc.c
@@ -381,6 +381,7 @@ static struct kvmppc_ops kvm_ops_e500mc = {
 	.emulate_op = kvmppc_core_emulate_op_e500,
 	.emulate_mtspr = kvmppc_core_emulate_mtspr_e500,
 	.emulate_mfspr = kvmppc_core_emulate_mfspr_e500,
+	.create_vcpu_debugfs = kvmppc_create_vcpu_debugfs_e500,
 };
 
 static int __init kvmppc_e500mc_init(void)
diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
index a72920f4f221..2ea73dfcebb2 100644
--- a/arch/powerpc/kvm/powerpc.c
+++ b/arch/powerpc/kvm/powerpc.c
@@ -763,7 +763,6 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu)
 		goto out_vcpu_uninit;
 
 	vcpu->arch.waitp = &vcpu->wait;
-	kvmppc_create_vcpu_debugfs(vcpu, vcpu->vcpu_id);
 	return 0;
 
 out_vcpu_uninit:
@@ -780,8 +779,6 @@ void kvm_arch_vcpu_destroy(struct kvm_vcpu *vcpu)
 	/* Make sure we're not using the vcpu anymore */
 	hrtimer_cancel(&vcpu->arch.dec_timer);
 
-	kvmppc_remove_vcpu_debugfs(vcpu);
-
 	switch (vcpu->arch.irq_type) {
 	case KVMPPC_IRQ_MPIC:
 		kvmppc_mpic_disconnect_vcpu(vcpu->arch.mpic, vcpu);
@@ -2505,3 +2502,16 @@ int kvm_arch_init(void *opaque)
 }
 
 EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_ppc_instr);
+
+void kvm_arch_create_vcpu_debugfs(struct kvm_vcpu *vcpu, struct dentry *debugfs_dentry)
+{
+	if (vcpu->kvm->arch.kvm_ops->create_vcpu_debugfs)
+		vcpu->kvm->arch.kvm_ops->create_vcpu_debugfs(vcpu, debugfs_dentry);
+}
+
+int kvm_arch_create_vm_debugfs(struct kvm *kvm)
+{
+	if (kvm->arch.kvm_ops->create_vm_debugfs)
+		kvm->arch.kvm_ops->create_vm_debugfs(kvm);
+	return 0;
+}
diff --git a/arch/powerpc/kvm/timing.c b/arch/powerpc/kvm/timing.c
index ba56a5cbba97..25071331f8c1 100644
--- a/arch/powerpc/kvm/timing.c
+++ b/arch/powerpc/kvm/timing.c
@@ -204,21 +204,10 @@ static const struct file_operations kvmppc_exit_timing_fops = {
 	.release = single_release,
 };
 
-void kvmppc_create_vcpu_debugfs(struct kvm_vcpu *vcpu, unsigned int id)
+int kvmppc_create_vcpu_debugfs_e500(struct kvm_vcpu *vcpu,
+				    struct dentry *debugfs_dentry)
 {
-	static char dbg_fname[50];
-	struct dentry *debugfs_file;
-
-	snprintf(dbg_fname, sizeof(dbg_fname), "vm%u_vcpu%u_timing",
-		 current->pid, id);
-	debugfs_file = debugfs_create_file(dbg_fname, 0666, kvm_debugfs_dir,
-						vcpu, &kvmppc_exit_timing_fops);
-
-	vcpu->arch.debugfs_exit_timing = debugfs_file;
-}
-
-void kvmppc_remove_vcpu_debugfs(struct kvm_vcpu *vcpu)
-{
-	debugfs_remove(vcpu->arch.debugfs_exit_timing);
-	vcpu->arch.debugfs_exit_timing = NULL;
+	debugfs_create_file("timing", 0666, debugfs_dentry,
+			    vcpu, &kvmppc_exit_timing_fops);
+	return 0;
 }
-- 
2.30.2


^ permalink raw reply related

* Re: [PATCH V2 11/17] riscv: compat: Add elf.h implementation
From: Guo Ren @ 2022-01-11  2:42 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: linux-s390, the arch/x86 maintainers, Drew Fustini, gregkh,
	Wang Junqiang, Anup Patel, Linux Kernel Mailing List, linux-csky,
	open list:BROADCOM NVRAM DRIVER, Christoph Hellwig, Guo Ren,
	Palmer Dabbelt, liush, sparclinux, linux-riscv, linuxppc-dev,
	inux-parisc, Linux ARM, Wei Fu
In-Reply-To: <CAK8P3a0H2Nq=bFdzWGzzGuFWP85JA7=Td6_rb8GqOF3bYCRJBw@mail.gmail.com>

On Mon, Jan 10, 2022 at 10:29 PM Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Tue, Dec 28, 2021 at 3:39 PM <guoren@kernel.org> wrote:
> >
> > From: Guo Ren <guoren@linux.alibaba.com>
> >
> > Implement necessary type and macro for compat elf. See the code
> > comment for detail.
> >
> > Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> > Signed-off-by: Guo Ren <guoren@kernel.org>
> > Cc: Arnd Bergmann <arnd@arndb.de>
>
> This looks mostly correct,
>
> > +/*
> > + * FIXME: not sure SET_PERSONALITY for compat process is right!
> > + */
> > +#define SET_PERSONALITY(ex)                                               \
> > +do {    if ((ex).e_ident[EI_CLASS] == ELFCLASS32)                         \
> > +               set_thread_flag(TIF_32BIT);                                \
> > +       else                                                               \
> > +               clear_thread_flag(TIF_32BIT);                              \
> > +       set_personality(PER_LINUX | (current->personality & (~PER_MASK))); \
> > +} while (0)
>
> This means the personality after exec is always set to PER_LINUX, not
> PER_LINUX32, which I think is wrong: you want the PER_LINUX32
> setting to stick, just like the upper bits do in the default implementation.
>
> What the other ones do is:
>
> | arch/parisc/include/asm/elf.h-
> set_personality((current->personality & ~PER_MASK) | PER_LINUX); \
>
> This looks like the same problem you introduce here: always forcing PER_LINUX
> instead of PER_LINUX32 makes it impossible to use PER_LINUX32.
>
> | arch/alpha/include/asm/elf.h:#define SET_PERSONALITY(EX)
>                            \
> | arch/alpha/include/asm/elf.h-   set_personality(((EX).e_flags &
> EF_ALPHA_32BIT)         \
> | arch/alpha/include/asm/elf.h-      ? PER_LINUX_32BIT : PER_LINUX)
> | arch/csky/include/asm/elf.h:#define SET_PERSONALITY(ex)
> set_personality(PER_LINUX)
> | arch/nds32/include/asm/elf.h:#define SET_PERSONALITY(ex)
> set_personality(PER_LINUX)
>
> These look even worse: instead of forcing the lower bits to
> PER_LINUX/PER_LINUX32 and
> leaving the upper bits untouched, these also clear the upper bits
> unconditionally.
>
> | arch/arm64/include/asm/elf.h:#define SET_PERSONALITY(ex)
>                                    \
> | arch/arm64/include/asm/elf.h-   current->personality &=
> ~READ_IMPLIES_EXEC;                     \
> | arch/x86/um/asm/elf.h:#define SET_PERSONALITY(ex) do {} while(0)
> | arch/x86/include/asm/elf.h:#define set_personality_64bit()      do {
> } while (0)
> | arch/x86/kernel/process_64.c:static void __set_personality_ia32(void)
> |         current->personality |= force_personality32;
>
> Inconsistent: does not enforce PER_LINUX/PER_LINUX32 as the default
> implementation
> does, but just leaves the value untouched (other than (re)setting
> READ_IMPLIES_EXEC).
> I think this is harmless otherwise, as we generally ignore the lower
> bits, except for the
> bit of code that checks for PER_LINUX32 in override_architecture() to mangle the
> output of sys_newuname() or in /proc/cpuinfo.
>
> | arch/s390/include/asm/elf.h-    if
> (personality(current->personality) != PER_LINUX32)   \
> | arch/s390/include/asm/elf.h-            set_personality(PER_LINUX |
>                    \
> | arch/s390/include/asm/elf.h-
> (current->personality & ~PER_MASK));    \
> | arch/powerpc/include/asm/elf.h- if
> (personality(current->personality) != PER_LINUX32)   \
> | arch/powerpc/include/asm/elf.h-         set_personality(PER_LINUX |
>                    \
> | arch/powerpc/include/asm/elf.h-
> (current->personality & (~PER_MASK)));  \
> | arch/sparc/include/asm/elf_64.h-        if
> (personality(current->personality) != PER_LINUX32)   \
> | arch/sparc/include/asm/elf_64.h-
> set_personality(PER_LINUX |             \
> | arch/sparc/include/asm/elf_64.h-
> (current->personality & (~PER_MASK)));  \
>
> This is probably the behavior you want to copy.
Thank you very much for your detailed explanation. Here is my modification.

+#define SET_PERSONALITY(ex)                                    \
+do {    if ((ex).e_ident[EI_CLASS] == ELFCLASS32)              \
+               set_thread_flag(TIF_32BIT);                     \
+       else                                                    \
+               clear_thread_flag(TIF_32BIT);                   \
+       if (personality(current->personality) != PER_LINUX32)   \
+               set_personality(PER_LINUX |                     \
+                       (current->personality & (~PER_MASK)));  \
+} while (0)

>
>       Arnd




--
Best Regards
 Guo Ren

ML: https://lore.kernel.org/linux-csky/

^ permalink raw reply

* Re: [PATCH V2 03/17] asm-generic: fcntl: compat: Remove duplicate definitions
From: Guo Ren @ 2022-01-11  2:43 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Guo Ren, open list:BROADCOM NVRAM DRIVER, J. Bruce Fields,
	sparclinux, linux-riscv, inux-parisc, linux-s390,
	the arch/x86 maintainers, linux-csky, Christoph Hellwig,
	Linux ARM, Drew Fustini, gregkh, Wang Junqiang, Anup Patel,
	Jeff Layton, Linux Kernel Mailing List, Palmer Dabbelt, liush,
	Linux FS-devel Mailing List, linuxppc-dev, Wei Fu
In-Reply-To: <CAK8P3a2zn9M6X09WsjJ9HYiS9WnO_YPCvJLSBk+HaH+yZHQqfA@mail.gmail.com>

On Mon, Jan 10, 2022 at 9:35 PM Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Tue, Dec 28, 2021 at 3:39 PM <guoren@kernel.org> wrote:
> >
> > From: Guo Ren <guoren@linux.alibaba.com>
> >
> > Remove duplicate F_GETLK64,F_SETLK64,F_SETLKW64 definitions in
> > arch/*/include/asm/compat.h.
> >
> > Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> > Signed-off-by: Guo Ren <guoren@kernel.org>
> > Cc: Arnd Bergmann <arnd@arndb.de>
>
> Unfortunately, this one does not look correct to me:
>
> > @@ -116,7 +116,7 @@
> >  #define F_GETSIG       11      /* for sockets. */
> >  #endif
> >
> > -#ifndef CONFIG_64BIT
> > +#if !defined(CONFIG_64BIT) || defined(CONFIG_COMPAT)
> >  #ifndef F_GETLK64
> >  #define F_GETLK64      12      /*  using 'struct flock64' */
> >  #define F_SETLK64      13
>
> The problem here is that include/uapi/ headers cannot contain checks for
> CONFIG_* symbols because those may have different meanings in user space
> compared to kernel.
>
> This is a preexisting problem in the header, but I think the change
> makes it worse.
>
> With the current behavior, user space will always see the definitions,
> unless it happens to have its own definition for CONFIG_64BIT already.
> On 64-bit parisc, this has the effect of defining the macros to the
> same values as F_SETOWN/F_SETSIG/F_GETSIG, which is potentially
> harmful. On MIPS, it uses values that are different from the 32-bit numbers
> but are otherwise unused. Everywhere else, we get the definition from
> the 32-bit architecture in user space, which will do nothing in the kernel.
>
> The correct check for a uapi header would be to test for
> __BITS_PER_LONG==32. We should probably do that here, but
> this won't help you move the definitions, and it is a user-visible change
> as the incorrect definition will no longer be visible. [Adding Jeff and Bruce
> (the flock mainainers) to Cc for additional feedback on this]
>
> For your series, I would suggest just moving the macro definitions to
> include/linux/compat.h along with the 'struct compat_flock64'
> definition, and leaving the duplicate one in the uapi header unchanged
> until we have decided on a solution.
Okay.

>
>         Arnd



-- 
Best Regards
 Guo Ren

ML: https://lore.kernel.org/linux-csky/

^ permalink raw reply

* Re: [PATCH 3/6] KVM: Remove opaque from kvm_arch_check_processor_compat
From: Chao Gao @ 2022-01-11  3:19 UTC (permalink / raw)
  To: Sean Christopherson
  Cc: x86, Wanpeng Li, kvm, David Hildenbrand, Dave Hansen, linux-mips,
	Atish Patra, Paul Mackerras, H. Peter Anvin, Alexander Gordeev,
	Claudio Imbrenda, Will Deacon, Maciej S. Szmigiero, linux-s390,
	Janosch Frank, Marc Zyngier, Joerg Roedel, Huacai Chen,
	linux-riscv, kvmarm, Aleksandar Markovic, Ingo Molnar,
	Catalin Marinas, Palmer Dabbelt, Christian Borntraeger,
	Ravi Bangoria, kevin.tian, Albert Ou, Vasily Gorbik,
	Suzuki K Poulose, Heiko Carstens, Nicholas Piggin,
	Borislav Petkov, Cédric Le Goater, Paul Walmsley, tglx,
	Alexandru Elisei, linux-arm-kernel, Jim Mattson,
	Thomas Bogendoerfer, Fabiano Rosas, Anup Patel, linux-kernel,
	Bharata B Rao, James Morse, kvm-riscv, pbonzini, Vitaly Kuznetsov,
	linuxppc-dev
In-Reply-To: <Ydy8BCfE0jhJd5uE@google.com>

On Mon, Jan 10, 2022 at 11:06:44PM +0000, Sean Christopherson wrote:
>On Mon, Dec 27, 2021, Chao Gao wrote:
>> No arch implementation uses this opaque now.
>
>Except for the RISC-V part, this can be a pure revert of commit b99040853738 ("KVM:
>Pass kvm_init()'s opaque param to additional arch funcs").  I think it makes sense
>to process it as a revert, with a short blurb in the changelog to note that RISC-V
>is manually modified as RISC-V support came along in the interim.

commit b99040853738 adds opaque param to kvm_arch_hardware_setup(), which isn't
reverted in this patch. I.e., this patch is a partial revert of b99040853738
plus manual changes to RISC-V. Given that, "process it as a revert" means
clearly say in changelog that this commit contains a partial revert of commit
b99040853738 ("KVM: Pass kvm_init()'s opaque param to additional arch funcs").

Right?

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox