LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Xie Yuanbin <xieyuanbin1@huawei.com>
To: <chleroy@kernel.org>, <andriy.shevchenko@intel.com>,
	<maddy@linux.ibm.com>, <mpe@ellerman.id.au>, <npiggin@gmail.com>,
	<kees@kernel.org>, <andy@kernel.org>
Cc: <linuxppc-dev@lists.ozlabs.org>, <linux-kernel@vger.kernel.org>,
	<linux-hardening@vger.kernel.org>, <lilinjie8@huawei.com>,
	<liaohua4@huawei.com>, <xieyuanbin1@huawei.com>
Subject: Re: [PATCH 2/2] powerpc/text-patching: Fix possible stringop-overread compilation error
Date: Thu, 30 Apr 2026 15:28:38 +0800	[thread overview]
Message-ID: <20260430072913.62348-1-xieyuanbin1@huawei.com> (raw)
In-Reply-To: <5cfae419-427a-471b-8bbe-645f56442e2c@kernel.org>

Hi, Christophe Leroy!

On Mon, 9 Feb 2026 14:41:51 +0100, Christophe Leroy (CS GROUP) wrote:
> On 09/02/2026 14:25, Xie Yuanbin wrote:
>> When CONFIG_PPC64_ELF_ABI_V1=y, it seems that the try of lookupinp
>> the original non-dot symbol is missing.
>>
>> What about this (Only the compilation test is performed):
>> ```c
>> static inline unsigned long __ppc_kallsyms_lookup_name(const char *name)
>> {
>> 	unsigned long addr = kallsyms_lookup_name(name);
>>
>> 	if (IS_ENABLED(CONFIG_PPC64_ELF_ABI_V2) && addr)
>> 		addr = ppc_function_entry((void *)addr);
>>
>> 	return addr;
>> }
>>
>> #define ppc_kallsyms_lookup_name(x) ({					\
>> 		unsigned long addr = 0;					\
>> 		if (IS_ENABLED(CONFIG_PPC64_ELF_ABI_V1))		\
>> 			addr = __ppc_kallsyms_lookup_name("." x);	\
>> 		if (!addr)						\
>> 			addr = __ppc_kallsyms_lookup_name(x);		\
>> 		addr;							\
>> 	})
>> ```
>
> Good point.
>
> To avoid duplicating the string I'd suggest:
>
> static inline unsigned long __ppc_kallsyms_lookup_name(const char *name)
> {
> 	unsigned long addr = kallsyms_lookup_name(name);
>
> 	if (IS_ENABLED(CONFIG_PPC64_ELF_ABI_V1) && !addr)
> 		addr = kallsyms_lookup_name(name + 1);
> 	if (IS_ENABLED(CONFIG_PPC64_ELF_ABI_V2) && addr)
> 		addr = ppc_function_entry((void *)addr);
>
> 	return addr;
> }
>
> #ifdef CONFIG_PPC64_ELF_ABI_V1
> #define ppc_kallsyms_lookup_name(x)	__ppc_kallsyms_lookup_name("." x);
> #else
> #define ppc_kallsyms_lookup_name(x)	__ppc_kallsyms_lookup_name(x)
> #endif

I would like to submit the modification as a new patch, and include you
as Co-developer. Do you agree?

Full of the patch:
```patch
From fbf07e5c1a97da7c8572435537f2b92213ede39d Mon Sep 17 00:00:00 2001
From: Xie Yuanbin <xieyuanbin1@huawei.com>
Date: Thu, 30 Apr 2026 14:15:26 +0800
Subject: [PATCH V2] powerpc/text-patching: simplify the implementation of ppc_kallsyms_lookup_name()

ppc_kallsyms_lookup_name() is called only twice in the kernel code, and
the parameters are all constant strings. strnlen(name, KSYM_NAME_LEN) is
called inside ppc_kallsyms_lookup_name(), when the compiler detects that
KSYM_NAME_LEN is larger then the constant strings,
the following error will be triggered:
```log
  CC      arch/powerpc/kernel/optprobes.o
In file included from ./arch/powerpc/include/asm/kprobes.h:24,
                 from ./include/linux/kprobes.h:31,
                 from arch/powerpc/kernel/optprobes.c:8:
In function ‘ppc_kallsyms_lookup_name’,
    inlined from ‘arch_prepare_optimized_kprobe’ at arch/powerpc/kernel/optprobes.c:209:21:
./arch/powerpc/include/asm/text-patching.h:232:13: error: ‘strnlen’ specified bound 512 exceeds source size 19 [-Werror=stringop-overread]
  232 |         if (strnlen(name, KSYM_NAME_LEN) >= KSYM_NAME_LEN)
      |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
In function ‘ppc_kallsyms_lookup_name’,
    inlined from ‘arch_prepare_optimized_kprobe’ at arch/powerpc/kernel/optprobes.c:210:22:
./arch/powerpc/include/asm/text-patching.h:232:13: error: ‘strnlen’ specified bound 512 exceeds source size 13 [-Werror=stringop-overread]
  232 |         if (strnlen(name, KSYM_NAME_LEN) >= KSYM_NAME_LEN)
      |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
```

The error can be reproduced in the following ways:
Use lastest linux-next source, change ppc_kallsyms_lookup_name() to
__always_inline, use default ppc64_defconfig, set CONFIG_EXPERT=y,
CONFIG_PPC64_BIG_ENDIAN_ELF_ABI_V2=n, CONFIG_CC_OPTIMIZE_FOR_SIZE=y,
and use gcc-14 or a later version for compilation.

Since ppc_kallsyms_lookup_name() is called only twice in the kernel,
and the parameters are all constant strins, simplify the implementation
of ppc_kallsyms_lookup_name() and avoid calling strnlen().

Cc: Andy Shevchenko <andriy.shevchenko@intel.com>
Cc: Kees Cook <kees@kernel.org>
Co-developed-by: Christophe Leroy (CS GROUP) <chleroy@kernel.org>
Signed-off-by: Christophe Leroy (CS GROUP) <chleroy@kernel.org>
Signed-off-by: Xie Yuanbin <xieyuanbin1@huawei.com>
---
v1->v2: https://lore.kernel.org/20260205100517.292858-2-xieyuanbin1@huawei.com
  - Not use strlen()

 arch/powerpc/include/asm/text-patching.h | 42 ++++++++----------------
 1 file changed, 17 insertions(+), 33 deletions(-)

diff --git a/arch/powerpc/include/asm/text-patching.h b/arch/powerpc/include/asm/text-patching.h
index e7f14720f630..2d3f698cb4f1 100644
--- a/arch/powerpc/include/asm/text-patching.h
+++ b/arch/powerpc/include/asm/text-patching.h
@@ -221,39 +221,23 @@ static inline unsigned long ppc_global_function_entry(void *func)
  * - For ABIv1, we lookup the dot variant.
  * - For ABIv2, we return the local entry point.
  */
-static inline unsigned long ppc_kallsyms_lookup_name(const char *name)
-{
-	unsigned long addr;
-#ifdef CONFIG_PPC64_ELF_ABI_V1
-	/* check for dot variant */
-	char dot_name[1 + KSYM_NAME_LEN];
-	bool dot_appended = false;
-
-	if (strnlen(name, KSYM_NAME_LEN) >= KSYM_NAME_LEN)
-		return 0;
-
-	if (name[0] != '.') {
-		dot_name[0] = '.';
-		dot_name[1] = '\0';
-		strlcat(dot_name, name, sizeof(dot_name));
-		dot_appended = true;
-	} else {
-		dot_name[0] = '\0';
-		strlcat(dot_name, name, sizeof(dot_name));
-	}
-	addr = kallsyms_lookup_name(dot_name);
-	if (!addr && dot_appended)
-		/* Let's try the original non-dot symbol lookup	*/
-		addr = kallsyms_lookup_name(name);
-#elif defined(CONFIG_PPC64_ELF_ABI_V2)
-	addr = kallsyms_lookup_name(name);
-	if (addr)
-		addr = ppc_function_entry((void *)addr);
-#else
-	addr = kallsyms_lookup_name(name);
-#endif
-	return addr;
-}
+static inline unsigned long __ppc_kallsyms_lookup_name(const char *name)
+{
+	unsigned long addr = kallsyms_lookup_name(name);
+
+	if (IS_ENABLED(CONFIG_PPC64_ELF_ABI_V1) && !addr)
+		addr = kallsyms_lookup_name(name + 1);
+	if (IS_ENABLED(CONFIG_PPC64_ELF_ABI_V2) && addr)
+		addr = ppc_function_entry((void *)addr);
+
+	return addr;
+}
+
+#ifdef CONFIG_PPC64_ELF_ABI_V1
+#define ppc_kallsyms_lookup_name(x)	__ppc_kallsyms_lookup_name("." x)
+#else
+#define ppc_kallsyms_lookup_name(x)	__ppc_kallsyms_lookup_name(x)
+#endif
 
 /*
  * Some instruction encodings commonly used in dynamic ftracing
-- 
2.53.0
```


  parent reply	other threads:[~2026-04-30  7:29 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-05 10:05 [PATCH 1/2] string: move __compiletime_strlen() to string.h Xie Yuanbin
2026-02-05 10:05 ` [PATCH 2/2] powerpc/text-patching: Fix possible stringop-overread compilation error Xie Yuanbin
2026-02-05 16:40   ` Andy Shevchenko
2026-02-06 11:14     ` Xie Yuanbin
2026-02-06 18:26   ` Kees Cook
2026-02-06 19:53     ` Christophe Leroy (CS GROUP)
2026-02-09 13:25       ` Xie Yuanbin
2026-02-09 13:41         ` Christophe Leroy (CS GROUP)
2026-02-09 14:11           ` Xie Yuanbin
2026-04-30  7:28           ` Xie Yuanbin [this message]
2026-04-30  9:41             ` Christophe Leroy (CS GROUP)

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260430072913.62348-1-xieyuanbin1@huawei.com \
    --to=xieyuanbin1@huawei.com \
    --cc=andriy.shevchenko@intel.com \
    --cc=andy@kernel.org \
    --cc=chleroy@kernel.org \
    --cc=kees@kernel.org \
    --cc=liaohua4@huawei.com \
    --cc=lilinjie8@huawei.com \
    --cc=linux-hardening@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=maddy@linux.ibm.com \
    --cc=mpe@ellerman.id.au \
    --cc=npiggin@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox