2023-09-12 16:44:11

by Andy Shevchenko

[permalink] [raw]
Subject: [PATCH v1 1/6] params: Use sysfs_emit() to instead of scnprintf()

Follow the advice of the Documentation/filesystems/sysfs.rst and show()
should only use sysfs_emit() or sysfs_emit_at() when formatting the
value to be returned to user space.

Signed-off-by: Andy Shevchenko <[email protected]>
---
kernel/params.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/kernel/params.c b/kernel/params.c
index 2d4a0564697e..3efe6b98a600 100644
--- a/kernel/params.c
+++ b/kernel/params.c
@@ -222,8 +222,7 @@ char *parse_args(const char *doing,
} \
int param_get_##name(char *buffer, const struct kernel_param *kp) \
{ \
- return scnprintf(buffer, PAGE_SIZE, format "\n", \
- *((type *)kp->arg)); \
+ return sysfs_emit(buffer, format "\n", *((type *)kp->arg)); \
} \
const struct kernel_param_ops param_ops_##name = { \
.set = param_set_##name, \
@@ -287,7 +286,7 @@ EXPORT_SYMBOL(param_set_charp);

int param_get_charp(char *buffer, const struct kernel_param *kp)
{
- return scnprintf(buffer, PAGE_SIZE, "%s\n", *((char **)kp->arg));
+ return sysfs_emit(buffer, "%s\n", *((char **)kp->arg));
}
EXPORT_SYMBOL(param_get_charp);

@@ -318,7 +317,7 @@ EXPORT_SYMBOL(param_set_bool);
int param_get_bool(char *buffer, const struct kernel_param *kp)
{
/* Y and N chosen as being relatively non-coder friendly */
- return sprintf(buffer, "%c\n", *(bool *)kp->arg ? 'Y' : 'N');
+ return sysfs_emit(buffer, "%c\n", *(bool *)kp->arg ? 'Y' : 'N');
}
EXPORT_SYMBOL(param_get_bool);

@@ -377,7 +376,7 @@ EXPORT_SYMBOL(param_set_invbool);

int param_get_invbool(char *buffer, const struct kernel_param *kp)
{
- return sprintf(buffer, "%c\n", (*(bool *)kp->arg) ? 'N' : 'Y');
+ return sysfs_emit(buffer, "%c\n", (*(bool *)kp->arg) ? 'N' : 'Y');
}
EXPORT_SYMBOL(param_get_invbool);

@@ -525,7 +524,8 @@ EXPORT_SYMBOL(param_set_copystring);
int param_get_string(char *buffer, const struct kernel_param *kp)
{
const struct kparam_string *kps = kp->str;
- return scnprintf(buffer, PAGE_SIZE, "%s\n", kps->string);
+
+ return sysfs_emit(buffer, "%s\n", kps->string);
}
EXPORT_SYMBOL(param_get_string);

@@ -859,7 +859,7 @@ ssize_t __modver_version_show(struct module_attribute *mattr,
struct module_version_attribute *vattr =
container_of(mattr, struct module_version_attribute, mattr);

- return scnprintf(buf, PAGE_SIZE, "%s\n", vattr->version);
+ return sysfs_emit(buf, "%s\n", vattr->version);
}

extern const struct module_version_attribute __start___modver[];
--
2.40.0.1.gaa8946217a0b


2023-09-12 19:28:41

by Andy Shevchenko

[permalink] [raw]
Subject: [PATCH v1 5/6] params: Sort headers

Sort the headers in alphabetic order in order to ease
the maintenance for this part.

Signed-off-by: Andy Shevchenko <[email protected]>
---
kernel/params.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/kernel/params.c b/kernel/params.c
index fd11c731475f..bcd535200c31 100644
--- a/kernel/params.c
+++ b/kernel/params.c
@@ -3,18 +3,18 @@
Copyright (C) 2001 Rusty Russell.

*/
+#include <linux/ctype.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/errno.h>
#include <linux/kernel.h>
#include <linux/kstrtox.h>
-#include <linux/string.h>
-#include <linux/errno.h>
#include <linux/module.h>
#include <linux/moduleparam.h>
-#include <linux/device.h>
-#include <linux/err.h>
#include <linux/overflow.h>
-#include <linux/slab.h>
-#include <linux/ctype.h>
#include <linux/security.h>
+#include <linux/slab.h>
+#include <linux/string.h>

#ifdef CONFIG_SYSFS
/* Protects all built-in parameters, modules use their own param_lock */
--
2.40.0.1.gaa8946217a0b

2023-09-13 07:23:41

by Andy Shevchenko

[permalink] [raw]
Subject: [PATCH v1 2/6] params: Introduce the param_unknown_fn type

Introduce a new type for the callback to parse an unknown argument.
This unifies function prototypes which takes that as a parameter.

Signed-off-by: Andy Shevchenko <[email protected]>
---
include/linux/moduleparam.h | 6 +++---
kernel/params.c | 8 ++------
2 files changed, 5 insertions(+), 9 deletions(-)

diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h
index 962cd41a2cb5..3800d1e2a3ab 100644
--- a/include/linux/moduleparam.h
+++ b/include/linux/moduleparam.h
@@ -381,6 +381,8 @@ extern bool parameq(const char *name1, const char *name2);
*/
extern bool parameqn(const char *name1, const char *name2, size_t n);

+typedef int (*parse_unknown_fn)(char *param, char *val, const char *doing, void *arg);
+
/* Called on module insert or kernel boot */
extern char *parse_args(const char *name,
char *args,
@@ -388,9 +390,7 @@ extern char *parse_args(const char *name,
unsigned num,
s16 level_min,
s16 level_max,
- void *arg,
- int (*unknown)(char *param, char *val,
- const char *doing, void *arg));
+ void *arg, parse_unknown_fn unknown);

/* Called by module remove. */
#ifdef CONFIG_SYSFS
diff --git a/kernel/params.c b/kernel/params.c
index 3efe6b98a600..fb594132ffc7 100644
--- a/kernel/params.c
+++ b/kernel/params.c
@@ -120,9 +120,7 @@ static int parse_one(char *param,
unsigned num_params,
s16 min_level,
s16 max_level,
- void *arg,
- int (*handle_unknown)(char *param, char *val,
- const char *doing, void *arg))
+ void *arg, parse_unknown_fn handle_unknown)
{
unsigned int i;
int err;
@@ -165,9 +163,7 @@ char *parse_args(const char *doing,
unsigned num,
s16 min_level,
s16 max_level,
- void *arg,
- int (*unknown)(char *param, char *val,
- const char *doing, void *arg))
+ void *arg, parse_unknown_fn unknown)
{
char *param, *val, *err = NULL;

--
2.40.0.1.gaa8946217a0b

2023-09-13 13:05:54

by Andy Shevchenko

[permalink] [raw]
Subject: [PATCH v1 3/6] params: Do not go over the limit when getting the string length

We can use strnlen() even on early stages and it prevents from
going over the string boundaries in case it's already too long.

Signed-off-by: Andy Shevchenko <[email protected]>
---
kernel/params.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/kernel/params.c b/kernel/params.c
index fb594132ffc7..930a5dc2f004 100644
--- a/kernel/params.c
+++ b/kernel/params.c
@@ -259,7 +259,10 @@ EXPORT_SYMBOL_GPL(param_set_uint_minmax);

int param_set_charp(const char *val, const struct kernel_param *kp)
{
- if (strlen(val) > 1024) {
+ size_t len, maxlen = 1024;
+
+ len = strnlen(val, maxlen + 1);
+ if (len == maxlen + 1) {
pr_err("%s: string parameter too long\n", kp->name);
return -ENOSPC;
}
@@ -269,7 +272,7 @@ int param_set_charp(const char *val, const struct kernel_param *kp)
/* This is a hack. We can't kmalloc in early boot, and we
* don't need to; this mangled commandline is preserved. */
if (slab_is_available()) {
- *(char **)kp->arg = kmalloc_parameter(strlen(val)+1);
+ *(char **)kp->arg = kmalloc_parameter(len + 1);
if (!*(char **)kp->arg)
return -ENOMEM;
strcpy(*(char **)kp->arg, val);
@@ -507,7 +510,7 @@ int param_set_copystring(const char *val, const struct kernel_param *kp)
{
const struct kparam_string *kps = kp->str;

- if (strlen(val)+1 > kps->maxlen) {
+ if (strnlen(val, kps->maxlen) == kps->maxlen) {
pr_err("%s: string doesn't fit in %u chars.\n",
kp->name, kps->maxlen-1);
return -ENOSPC;
--
2.40.0.1.gaa8946217a0b

2023-09-13 14:42:58

by Andy Shevchenko

[permalink] [raw]
Subject: [PATCH v1 6/6] params: Fix multi-line comment style

The multi-line comment style in the file is rather arbitrary.
Make it follow the standard one.

Signed-off-by: Andy Shevchenko <[email protected]>
---
kernel/params.c | 20 ++++++++++++--------
1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/kernel/params.c b/kernel/params.c
index bcd535200c31..a6d7023dc993 100644
--- a/kernel/params.c
+++ b/kernel/params.c
@@ -1,8 +1,8 @@
// SPDX-License-Identifier: GPL-2.0-or-later
-/* Helpers for initial module or kernel cmdline parsing
- Copyright (C) 2001 Rusty Russell.
-
-*/
+/*
+ * Helpers for initial module or kernel cmdline parsing
+ * Copyright (C) 2001 Rusty Russell.
+ */
#include <linux/ctype.h>
#include <linux/device.h>
#include <linux/err.h>
@@ -270,8 +270,10 @@ int param_set_charp(const char *val, const struct kernel_param *kp)

maybe_kfree_parameter(*(char **)kp->arg);

- /* This is a hack. We can't kmalloc in early boot, and we
- * don't need to; this mangled commandline is preserved. */
+ /*
+ * This is a hack. We can't kmalloc() in early boot, and we
+ * don't need to; this mangled commandline is preserved.
+ */
if (slab_is_available()) {
*(char **)kp->arg = kmalloc_parameter(len + 1);
if (!*(char **)kp->arg)
@@ -743,8 +745,10 @@ void module_param_sysfs_remove(struct module *mod)
{
if (mod->mkobj.mp) {
sysfs_remove_group(&mod->mkobj.kobj, &mod->mkobj.mp->grp);
- /* We are positive that no one is using any param
- * attrs at this point. Deallocate immediately. */
+ /*
+ * We are positive that no one is using any param
+ * attrs at this point. Deallocate immediately.
+ */
free_module_param_attrs(&mod->mkobj);
}
}
--
2.40.0.1.gaa8946217a0b

2023-09-14 01:52:58

by Andy Shevchenko

[permalink] [raw]
Subject: [PATCH v1 4/6] params: Use size_add() for kmalloc()

Prevent allocations from integer overflow by using size_add().

Signed-off-by: Andy Shevchenko <[email protected]>
---
kernel/params.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/kernel/params.c b/kernel/params.c
index 930a5dc2f004..fd11c731475f 100644
--- a/kernel/params.c
+++ b/kernel/params.c
@@ -11,6 +11,7 @@
#include <linux/moduleparam.h>
#include <linux/device.h>
#include <linux/err.h>
+#include <linux/overflow.h>
#include <linux/slab.h>
#include <linux/ctype.h>
#include <linux/security.h>
@@ -48,7 +49,7 @@ static void *kmalloc_parameter(unsigned int size)
{
struct kmalloced_param *p;

- p = kmalloc(sizeof(*p) + size, GFP_KERNEL);
+ p = kmalloc(size_add(sizeof(*p), size), GFP_KERNEL);
if (!p)
return NULL;

--
2.40.0.1.gaa8946217a0b

2023-09-18 18:23:02

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v1 1/6] params: Use sysfs_emit() to instead of scnprintf()

On Tue, Sep 12, 2023 at 06:05:46PM +0300, Andy Shevchenko wrote:
> Follow the advice of the Documentation/filesystems/sysfs.rst and show()
> should only use sysfs_emit() or sysfs_emit_at() when formatting the
> value to be returned to user space.

Any comments?

--
With Best Regards,
Andy Shevchenko


2023-09-21 13:31:15

by Luis Chamberlain

[permalink] [raw]
Subject: Re: [PATCH v1 1/6] params: Use sysfs_emit() to instead of scnprintf()

On Mon, Sep 18, 2023 at 07:57:41PM +0300, Andy Shevchenko wrote:
> On Tue, Sep 12, 2023 at 06:05:46PM +0300, Andy Shevchenko wrote:
> > Follow the advice of the Documentation/filesystems/sysfs.rst and show()
> > should only use sysfs_emit() or sysfs_emit_at() when formatting the
> > value to be returned to user space.
>
> Any comments?

What tree were you taretting, looks sane to me.

Luis

2023-09-21 18:45:45

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v1 1/6] params: Use sysfs_emit() to instead of scnprintf()

On Thu, Sep 21, 2023 at 06:36:38PM +0300, Andy Shevchenko wrote:
> On Thu, Sep 21, 2023 at 09:34:13PM +0800, kernel test robot wrote:
> >
> > Hello,
> >
> > kernel test robot noticed "WARNING:at_fs/sysfs/file.c:#sysfs_emit" on:
> >
> > commit: d4004295e5502a1eb3e361e97ea4dd1686046af6 ("[PATCH v1 1/6] params: Use sysfs_emit() to instead of scnprintf()")
> > url: https://github.com/intel-lab-lkp/linux/commits/Andy-Shevchenko/params-Introduce-the-param_unknown_fn-type/20230912-231033
> > base: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git 0bb80ecc33a8fb5a682236443c1e740d5c917d1d
> > patch link: https://lore.kernel.org/all/[email protected]/
> > patch subject: [PATCH v1 1/6] params: Use sysfs_emit() to instead of scnprintf()
> >
> > in testcase: trinity
> > version: trinity-i386-abe9de86-1_20230429
> > with following parameters:
> >
> > runtime: 300s
> > group: group-04
> > nr_groups: 5
>
>
> > what we observed is this issue doesn't always happen. we run the test upon
> > this commit almost 500 times, it happened 42 times.
> > however, the parent keeps clean.
> >
> > v6.6-rc1 d4004295e5502a1eb3e361e97ea
> > ---------------- ---------------------------
> > fail:runs %reproduction fail:runs
> > | | |
> > :497 8% 42:496 dmesg.EIP:sysfs_emit
> > :497 8% 42:496 dmesg.WARNING:at_fs/sysfs/file.c:#sysfs_emit
>
> Cool! I will check this, thank you for the report.

Oh, my gosh... This reveals a nice overflow bug for some getters that expect
buffer to be PAGE_SIZE, but an array can be bigger than that.

So, basically this is a flaw in param_array_get() which is a wrapper on top of
getter and calls ->get() without any proper alignment or buffer size guarantee!

While ->get() is by nature suppose to get an aligned buffer of PAGE_SIZE.

Ideally we need to have an additional ->get_array_element() callback which will
take an offset. Less intrusive one is to have an allocated buffer of PAGE_SIZE
in the param_array_get() and ->get() to it, then copy to the real one with the
offset. Any other proposals?

Luis, which solution would you prefer?

--
With Best Regards,
Andy Shevchenko


2023-09-21 21:10:35

by Oliver Sang

[permalink] [raw]
Subject: Re: [PATCH v1 1/6] params: Use sysfs_emit() to instead of scnprintf()



Hello,

kernel test robot noticed "WARNING:at_fs/sysfs/file.c:#sysfs_emit" on:

commit: d4004295e5502a1eb3e361e97ea4dd1686046af6 ("[PATCH v1 1/6] params: Use sysfs_emit() to instead of scnprintf()")
url: https://github.com/intel-lab-lkp/linux/commits/Andy-Shevchenko/params-Introduce-the-param_unknown_fn-type/20230912-231033
base: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git 0bb80ecc33a8fb5a682236443c1e740d5c917d1d
patch link: https://lore.kernel.org/all/[email protected]/
patch subject: [PATCH v1 1/6] params: Use sysfs_emit() to instead of scnprintf()

in testcase: trinity
version: trinity-i386-abe9de86-1_20230429
with following parameters:

runtime: 300s
group: group-04
nr_groups: 5

test-description: Trinity is a linux system call fuzz tester.
test-url: http://codemonkey.org.uk/projects/trinity/


compiler: gcc-12
test machine: qemu-system-x86_64 -enable-kvm -cpu SandyBridge -smp 2 -m 16G

(please refer to attached dmesg/kmsg for entire log/backtrace)


what we observed is this issue doesn't always happen. we run the test upon
this commit almost 500 times, it happened 42 times.
however, the parent keeps clean.

v6.6-rc1 d4004295e5502a1eb3e361e97ea
---------------- ---------------------------
fail:runs %reproduction fail:runs
| | |
:497 8% 42:496 dmesg.EIP:sysfs_emit
:497 8% 42:496 dmesg.WARNING:at_fs/sysfs/file.c:#sysfs_emit



If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-lkp/[email protected]


[ 243.129633][ T4012] ------------[ cut here ]------------
[ 243.130401][ T4012] invalid sysfs_emit: buf:94f9d7f6
[ 243.130980][ T4012] WARNING: CPU: 1 PID: 4012 at fs/sysfs/file.c:734 sysfs_emit (fs/sysfs/file.c:734)
[ 243.131846][ T4012] Modules linked in: rtc_cmos aesni_intel evbug parport_pc qemu_fw_cfg
[ 243.132786][ T4012] CPU: 1 PID: 4012 Comm: trinity-c5 Not tainted 6.6.0-rc1-00001-gd4004295e550 #1
[ 243.133731][ T4012] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-debian-1.16.2-1 04/01/2014
[ 243.134826][ T4012] EIP: sysfs_emit (fs/sysfs/file.c:734)
[ 243.135330][ T4012] Code: 5b 5e 5f 5d 31 d2 31 c9 c3 3e 8d 74 26 00 55 89 e5 8b 45 08 85 c0 74 07 a9 ff 0f 00 00 74 13 50 68 f9 89 e2 c1 e8 6e ce e0 ff <0f> 0b 59 58 31 c0 eb 12 8d 55 10 52 8b 4d 0c ba 00 10 00 00 e8 01
All code
========
0: 5b pop %rbx
1: 5e pop %rsi
2: 5f pop %rdi
3: 5d pop %rbp
4: 31 d2 xor %edx,%edx
6: 31 c9 xor %ecx,%ecx
8: c3 ret
9: 3e 8d 74 26 00 ds lea 0x0(%rsi,%riz,1),%esi
e: 55 push %rbp
f: 89 e5 mov %esp,%ebp
11: 8b 45 08 mov 0x8(%rbp),%eax
14: 85 c0 test %eax,%eax
16: 74 07 je 0x1f
18: a9 ff 0f 00 00 test $0xfff,%eax
1d: 74 13 je 0x32
1f: 50 push %rax
20: 68 f9 89 e2 c1 push $0xffffffffc1e289f9
25: e8 6e ce e0 ff call 0xffffffffffe0ce98
2a:* 0f 0b ud2 <-- trapping instruction
2c: 59 pop %rcx
2d: 58 pop %rax
2e: 31 c0 xor %eax,%eax
30: eb 12 jmp 0x44
32: 8d 55 10 lea 0x10(%rbp),%edx
35: 52 push %rdx
36: 8b 4d 0c mov 0xc(%rbp),%ecx
39: ba 00 10 00 00 mov $0x1000,%edx
3e: e8 .byte 0xe8
3f: 01 .byte 0x1

Code starting with the faulting instruction
===========================================
0: 0f 0b ud2
2: 59 pop %rcx
3: 58 pop %rax
4: 31 c0 xor %eax,%eax
6: eb 12 jmp 0x1a
8: 8d 55 10 lea 0x10(%rbp),%edx
b: 52 push %rdx
c: 8b 4d 0c mov 0xc(%rbp),%ecx
f: ba 00 10 00 00 mov $0x1000,%edx
14: e8 .byte 0xe8
15: 01 .byte 0x1
[ 243.137360][ T4012] EAX: 00000000 EBX: c1aa8260 ECX: 00000000 EDX: 00000000
[ 243.138145][ T4012] ESI: 00000002 EDI: 00000001 EBP: eb36be20 ESP: eb36be18
[ 243.138905][ T4012] DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068 EFLAGS: 00010246
[ 243.139712][ T4012] CR0: 80050033 CR2: 00000004 CR3: 2b263000 CR4: 00040690
[ 243.142408][ T4012] DR0: 00000000 DR1: 00000000 DR2: 00000000 DR3: 00000000
[ 243.143135][ T4012] DR6: fffe0ff0 DR7: 00000400
[ 243.143614][ T4012] Call Trace:
[ 243.143991][ T4012] ? show_regs (arch/x86/kernel/dumpstack.c:479 arch/x86/kernel/dumpstack.c:465)
[ 243.144451][ T4012] ? sysfs_emit (fs/sysfs/file.c:734)
[ 243.144908][ T4012] ? __warn (kernel/panic.c:673)
[ 243.145339][ T4012] ? report_bug (lib/bug.c:201 lib/bug.c:219)
[ 243.145854][ T4012] ? sysfs_emit (fs/sysfs/file.c:734)
[ 243.146345][ T4012] ? exc_overflow (arch/x86/kernel/traps.c:250)
[ 243.146841][ T4012] ? handle_bug (arch/x86/kernel/traps.c:237)
[ 243.147327][ T4012] ? exc_invalid_op (arch/x86/kernel/traps.c:258 (discriminator 1))
[ 243.147820][ T4012] ? handle_exception (arch/x86/entry/entry_32.S:1049)
[ 243.148398][ T4012] ? rwlock_bug (kernel/locking/spinlock_debug.c:147)
[ 243.148866][ T4012] ? exc_overflow (arch/x86/kernel/traps.c:250)
[ 243.149344][ T4012] ? sysfs_emit (fs/sysfs/file.c:734)
[ 243.149806][ T4012] ? exc_overflow (arch/x86/kernel/traps.c:250)
[ 243.150299][ T4012] ? sysfs_emit (fs/sysfs/file.c:734)
[ 243.150759][ T4012] param_get_int (kernel/params.c:239)
[ 243.151232][ T4012] param_array_get (kernel/params.c:485)
[ 243.151757][ T4012] param_attr_show (kernel/params.c:568)
[ 243.152295][ T4012] ? param_attr_store (kernel/params.c:560)
[ 243.152814][ T4012] ? func_ptr_is_kernel_text (kernel/params.c:890)
[ 243.153400][ T4012] module_attr_show (kernel/params.c:903)
[ 243.153930][ T4012] sysfs_kf_seq_show (fs/sysfs/file.c:60)
[ 243.154456][ T4012] kernfs_seq_show (fs/kernfs/file.c:206)
[ 243.154966][ T4012] seq_read_iter (fs/seq_file.c:230)
[ 243.155453][ T4012] ? fsnotify_perm+0x3b/0x40
[ 243.156039][ T4012] kernfs_fop_read_iter (fs/kernfs/file.c:279)
[ 243.156570][ T4012] call_read_iter+0x12/0x19
[ 243.157109][ T4012] vfs_read (fs/read_write.c:389 fs/read_write.c:470)
[ 243.157571][ T4012] ksys_read (fs/read_write.c:613)
[ 243.160726][ T4012] __ia32_sys_read (fs/read_write.c:621)
[ 243.161225][ T4012] do_int80_syscall_32 (arch/x86/entry/common.c:112 arch/x86/entry/common.c:132)
[ 243.161784][ T4012] entry_INT80_32 (arch/x86/entry/entry_32.S:944)
[ 243.162309][ T4012] EIP: 0xb7f8e092
[ 243.162720][ T4012] Code: 00 00 00 e9 90 ff ff ff ff a3 24 00 00 00 68 30 00 00 00 e9 80 ff ff ff ff a3 f8 ff ff ff 66 90 00 00 00 00 00 00 00 00 cd 80 <c3> 8d b4 26 00 00 00 00 8d b6 00 00 00 00 8b 1c 24 c3 8d b4 26 00
All code
========
0: 00 00 add %al,(%rax)
2: 00 e9 add %ch,%cl
4: 90 nop
5: ff (bad)
6: ff (bad)
7: ff (bad)
8: ff a3 24 00 00 00 jmp *0x24(%rbx)
e: 68 30 00 00 00 push $0x30
13: e9 80 ff ff ff jmp 0xffffffffffffff98
18: ff a3 f8 ff ff ff jmp *-0x8(%rbx)
1e: 66 90 xchg %ax,%ax
...
28: cd 80 int $0x80
2a:* c3 ret <-- trapping instruction
2b: 8d b4 26 00 00 00 00 lea 0x0(%rsi,%riz,1),%esi
32: 8d b6 00 00 00 00 lea 0x0(%rsi),%esi
38: 8b 1c 24 mov (%rsp),%ebx
3b: c3 ret
3c: 8d .byte 0x8d
3d: b4 26 mov $0x26,%ah
...

Code starting with the faulting instruction
===========================================
0: c3 ret
1: 8d b4 26 00 00 00 00 lea 0x0(%rsi,%riz,1),%esi
8: 8d b6 00 00 00 00 lea 0x0(%rsi),%esi
e: 8b 1c 24 mov (%rsp),%ebx
11: c3 ret
12: 8d .byte 0x8d
13: b4 26 mov $0x26,%ah


The kernel config and materials to reproduce are available at:
https://download.01.org/0day-ci/archive/20230921/[email protected]



--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

2023-09-21 23:11:56

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v1 1/6] params: Use sysfs_emit() to instead of scnprintf()

On Thu, Sep 21, 2023 at 09:34:13PM +0800, kernel test robot wrote:
>
> Hello,
>
> kernel test robot noticed "WARNING:at_fs/sysfs/file.c:#sysfs_emit" on:
>
> commit: d4004295e5502a1eb3e361e97ea4dd1686046af6 ("[PATCH v1 1/6] params: Use sysfs_emit() to instead of scnprintf()")
> url: https://github.com/intel-lab-lkp/linux/commits/Andy-Shevchenko/params-Introduce-the-param_unknown_fn-type/20230912-231033
> base: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git 0bb80ecc33a8fb5a682236443c1e740d5c917d1d
> patch link: https://lore.kernel.org/all/[email protected]/
> patch subject: [PATCH v1 1/6] params: Use sysfs_emit() to instead of scnprintf()
>
> in testcase: trinity
> version: trinity-i386-abe9de86-1_20230429
> with following parameters:
>
> runtime: 300s
> group: group-04
> nr_groups: 5


> what we observed is this issue doesn't always happen. we run the test upon
> this commit almost 500 times, it happened 42 times.
> however, the parent keeps clean.
>
> v6.6-rc1 d4004295e5502a1eb3e361e97ea
> ---------------- ---------------------------
> fail:runs %reproduction fail:runs
> | | |
> :497 8% 42:496 dmesg.EIP:sysfs_emit
> :497 8% 42:496 dmesg.WARNING:at_fs/sysfs/file.c:#sysfs_emit

Cool! I will check this, thank you for the report.

--
With Best Regards,
Andy Shevchenko


2023-09-22 05:57:32

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v1 1/6] params: Use sysfs_emit() to instead of scnprintf()

On Wed, Sep 20, 2023 at 05:32:55PM -0700, Luis Chamberlain wrote:
> On Mon, Sep 18, 2023 at 07:57:41PM +0300, Andy Shevchenko wrote:
> > On Tue, Sep 12, 2023 at 06:05:46PM +0300, Andy Shevchenko wrote:
> > > Follow the advice of the Documentation/filesystems/sysfs.rst and show()
> > > should only use sysfs_emit() or sysfs_emit_at() when formatting the
> > > value to be returned to user space.
> >
> > Any comments?
>
> What tree were you taretting, looks sane to me.

I see that you was a person with last SoB in late patches in that area.
Whatever it went through in your case.

P.S. TBH I thought you have your own tree for things like this...

--
With Best Regards,
Andy Shevchenko