2022-02-23 12:40:32

by tangmeng

[permalink] [raw]
Subject: [PATCH] fs/proc: Optimize arrays defined by struct ctl_path

Previously, arrays defined by struct ctl_path is terminated
with an empty one. For example, when we actually only register
one ctl_path, we've gone from 8 bytes to 16 bytes.

So, I use ARRAY_SIZE() as a boundary condition to optimize it.

Signed-off-by: Meng Tang <[email protected]>
---
arch/csky/abiv1/alignment.c | 8 ++++----
arch/nds32/mm/alignment.c | 8 ++++----
fs/proc/proc_sysctl.c | 12 +++++++-----
fs/verity/signature.c | 7 ++++---
include/linux/sysctl.h | 6 +++---
kernel/pid_namespace.c | 5 +++--
kernel/seccomp.c | 7 ++++---
security/apparmor/lsm.c | 7 ++++---
security/loadpin/loadpin.c | 7 ++++---
security/yama/yama_lsm.c | 6 +++---
10 files changed, 40 insertions(+), 33 deletions(-)

diff --git a/arch/csky/abiv1/alignment.c b/arch/csky/abiv1/alignment.c
index 2df115d0e210..f2fe6a4d2388 100644
--- a/arch/csky/abiv1/alignment.c
+++ b/arch/csky/abiv1/alignment.c
@@ -340,14 +340,14 @@ static struct ctl_table sysctl_table[2] = {
{}
};

-static struct ctl_path sysctl_path[2] = {
- {.procname = "csky"},
- {}
+static struct ctl_path sysctl_path[1] = {
+ {.procname = "csky"}
};
+#define SYSCTL_PATH_SIZE ARRAY_SIZE(sysctl_path)

static int __init csky_alignment_init(void)
{
- register_sysctl_paths(sysctl_path, sysctl_table);
+ register_sysctl_paths(sysctl_path, sysctl_table, SYSCTL_PATH_SIZE);
return 0;
}

diff --git a/arch/nds32/mm/alignment.c b/arch/nds32/mm/alignment.c
index 1eb7ded6992b..ee073f6b5822 100644
--- a/arch/nds32/mm/alignment.c
+++ b/arch/nds32/mm/alignment.c
@@ -560,17 +560,17 @@ static struct ctl_table nds32_sysctl_table[2] = {
{}
};

-static struct ctl_path nds32_path[2] = {
- {.procname = "nds32"},
- {}
+static struct ctl_path nds32_path[1] = {
+ {.procname = "nds32"}
};
+#define NDS32_PATH_SIZE ARRAY_SIZE(nds32_path)

/*
* Initialize nds32 alignment-correction interface
*/
static int __init nds32_sysctl_init(void)
{
- register_sysctl_paths(nds32_path, nds32_sysctl_table);
+ register_sysctl_paths(nds32_path, nds32_sysctl_table, NDS32_PATH_SIZE);
return 0;
}

diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
index 7d9cfc730bd4..d6f6d9d5d3f8 100644
--- a/fs/proc/proc_sysctl.c
+++ b/fs/proc/proc_sysctl.c
@@ -1552,20 +1552,21 @@ static int register_leaf_sysctl_tables(const char *path, char *pos,
*/
struct ctl_table_header *__register_sysctl_paths(
struct ctl_table_set *set,
- const struct ctl_path *path, struct ctl_table *table)
+ const struct ctl_path *path, struct ctl_table *table, int table_size)
{
struct ctl_table *ctl_table_arg = table;
int nr_subheaders = count_subheaders(table);
struct ctl_table_header *header = NULL, **subheaders, **subheader;
const struct ctl_path *component;
char *new_path, *pos;
+ int i;

pos = new_path = kmalloc(PATH_MAX, GFP_KERNEL);
if (!new_path)
return NULL;

pos[0] = '\0';
- for (component = path; component->procname; component++) {
+ for (component = path, i = 0; component->procname && i < table_size; component++, i++) {
pos = append_path(new_path, pos, component->procname);
if (!pos)
goto out;
@@ -1622,10 +1623,11 @@ struct ctl_table_header *__register_sysctl_paths(
* See __register_sysctl_paths for more details.
*/
struct ctl_table_header *register_sysctl_paths(const struct ctl_path *path,
- struct ctl_table *table)
+ struct ctl_table *table,
+ int table_size)
{
return __register_sysctl_paths(&sysctl_table_root.default_set,
- path, table);
+ path, table, table_size);
}
EXPORT_SYMBOL(register_sysctl_paths);

@@ -1642,7 +1644,7 @@ struct ctl_table_header *register_sysctl_table(struct ctl_table *table)
{
static const struct ctl_path null_path[] = { {} };

- return register_sysctl_paths(null_path, table);
+ return register_sysctl_paths(null_path, table, ARRAY_SIZE(null_path));
}
EXPORT_SYMBOL(register_sysctl_table);

diff --git a/fs/verity/signature.c b/fs/verity/signature.c
index 143a530a8008..e04be57da6ab 100644
--- a/fs/verity/signature.c
+++ b/fs/verity/signature.c
@@ -92,9 +92,9 @@ static struct ctl_table_header *fsverity_sysctl_header;

static const struct ctl_path fsverity_sysctl_path[] = {
{ .procname = "fs", },
- { .procname = "verity", },
- { }
+ { .procname = "verity", }
};
+#define FSVERITY_SYSCTL_PATH_SIZE ARRAY_SIZE(fsverity_sysctl_path)

static struct ctl_table fsverity_sysctl_table[] = {
{
@@ -112,7 +112,8 @@ static struct ctl_table fsverity_sysctl_table[] = {
static int __init fsverity_sysctl_init(void)
{
fsverity_sysctl_header = register_sysctl_paths(fsverity_sysctl_path,
- fsverity_sysctl_table);
+ fsverity_sysctl_table,
+ FSVERITY_SYSCTL_PATH_SIZE);
if (!fsverity_sysctl_header) {
pr_err("sysctl registration failed!\n");
return -ENOMEM;
diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
index 6353d6db69b2..781874ed9179 100644
--- a/include/linux/sysctl.h
+++ b/include/linux/sysctl.h
@@ -220,11 +220,11 @@ struct ctl_table_header *__register_sysctl_table(
const char *path, struct ctl_table *table);
struct ctl_table_header *__register_sysctl_paths(
struct ctl_table_set *set,
- const struct ctl_path *path, struct ctl_table *table);
+ const struct ctl_path *path, struct ctl_table *table, int table_size);
struct ctl_table_header *register_sysctl(const char *path, struct ctl_table *table);
struct ctl_table_header *register_sysctl_table(struct ctl_table * table);
struct ctl_table_header *register_sysctl_paths(const struct ctl_path *path,
- struct ctl_table *table);
+ struct ctl_table *table, int table_size);

void unregister_sysctl_table(struct ctl_table_header * table);

@@ -271,7 +271,7 @@ static inline struct ctl_table_header *register_sysctl_mount_point(const char *p
}

static inline struct ctl_table_header *register_sysctl_paths(
- const struct ctl_path *path, struct ctl_table *table)
+ const struct ctl_path *path, struct ctl_table *table, int table_size)
{
return NULL;
}
diff --git a/kernel/pid_namespace.c b/kernel/pid_namespace.c
index a46a3723bc66..18a1a3bb37a0 100644
--- a/kernel/pid_namespace.c
+++ b/kernel/pid_namespace.c
@@ -294,7 +294,8 @@ static struct ctl_table pid_ns_ctl_table[] = {
},
{ }
};
-static struct ctl_path kern_path[] = { { .procname = "kernel", }, { } };
+static struct ctl_path kern_path[] = { { .procname = "kernel", } };
+#define KERN_PATH_SIZE ARRAY_SIZE(kern_path)
#endif /* CONFIG_CHECKPOINT_RESTORE */

int reboot_pid_ns(struct pid_namespace *pid_ns, int cmd)
@@ -453,7 +454,7 @@ static __init int pid_namespaces_init(void)
pid_ns_cachep = KMEM_CACHE(pid_namespace, SLAB_PANIC | SLAB_ACCOUNT);

#ifdef CONFIG_CHECKPOINT_RESTORE
- register_sysctl_paths(kern_path, pid_ns_ctl_table);
+ register_sysctl_paths(kern_path, pid_ns_ctl_table, KERN_PATH_SIZE);
#endif
return 0;
}
diff --git a/kernel/seccomp.c b/kernel/seccomp.c
index db10e73d06e0..d4759cdb0335 100644
--- a/kernel/seccomp.c
+++ b/kernel/seccomp.c
@@ -2333,9 +2333,9 @@ static int seccomp_actions_logged_handler(struct ctl_table *ro_table, int write,

static struct ctl_path seccomp_sysctl_path[] = {
{ .procname = "kernel", },
- { .procname = "seccomp", },
- { }
+ { .procname = "seccomp", }
};
+#define SECCOMP_SYSCTL_PATH_SIZE ARRAY_SIZE(seccomp_sysctl_path)

static struct ctl_table seccomp_sysctl_table[] = {
{
@@ -2357,7 +2357,8 @@ static int __init seccomp_sysctl_init(void)
{
struct ctl_table_header *hdr;

- hdr = register_sysctl_paths(seccomp_sysctl_path, seccomp_sysctl_table);
+ hdr = register_sysctl_paths(seccomp_sysctl_path, seccomp_sysctl_table,
+ SECCOMP_SYSCTL_PATH_SIZE);
if (!hdr)
pr_warn("sysctl registration failed\n");
else
diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index 4f0eecb67dde..09a0461db6b1 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -1729,9 +1729,9 @@ static int apparmor_dointvec(struct ctl_table *table, int write,
}

static struct ctl_path apparmor_sysctl_path[] = {
- { .procname = "kernel", },
- { }
+ { .procname = "kernel", }
};
+#define APPARMOR_SYSCTL_PATH_SIZE ARRAY_SIZE(apparmor_sysctl_path)

static struct ctl_table apparmor_sysctl_table[] = {
{
@@ -1747,7 +1747,8 @@ static struct ctl_table apparmor_sysctl_table[] = {
static int __init apparmor_init_sysctl(void)
{
return register_sysctl_paths(apparmor_sysctl_path,
- apparmor_sysctl_table) ? 0 : -ENOMEM;
+ apparmor_sysctl_table,
+ APPARMOR_SYSCTL_PATH_SIZE) ? 0 : -ENOMEM;
}
#else
static inline int apparmor_init_sysctl(void)
diff --git a/security/loadpin/loadpin.c b/security/loadpin/loadpin.c
index b12f7d986b1e..45e0b8952a1d 100644
--- a/security/loadpin/loadpin.c
+++ b/security/loadpin/loadpin.c
@@ -48,9 +48,9 @@ static DEFINE_SPINLOCK(pinned_root_spinlock);

static struct ctl_path loadpin_sysctl_path[] = {
{ .procname = "kernel", },
- { .procname = "loadpin", },
- { }
+ { .procname = "loadpin", }
};
+#define LOADPIN_SYSCTL_PATH_SIZE ARRAY_SIZE(loadpin_sysctl_path)

static struct ctl_table loadpin_sysctl_table[] = {
{
@@ -91,7 +91,8 @@ static void check_pinning_enforcement(struct super_block *mnt_sb)

if (!ro) {
if (!register_sysctl_paths(loadpin_sysctl_path,
- loadpin_sysctl_table))
+ loadpin_sysctl_table,
+ LOADPIN_SYSCTL_PATH_SIZE))
pr_notice("sysctl registration failed!\n");
else
pr_info("enforcement can be disabled.\n");
diff --git a/security/yama/yama_lsm.c b/security/yama/yama_lsm.c
index 06e226166aab..c4472d17b46d 100644
--- a/security/yama/yama_lsm.c
+++ b/security/yama/yama_lsm.c
@@ -449,9 +449,9 @@ static int max_scope = YAMA_SCOPE_NO_ATTACH;

static struct ctl_path yama_sysctl_path[] = {
{ .procname = "kernel", },
- { .procname = "yama", },
- { }
+ { .procname = "yama", }
};
+#define YAMA_SYSCTL_PATH_SIZE ARRAY_SIZE(yama_sysctl_path)

static struct ctl_table yama_sysctl_table[] = {
{
@@ -467,7 +467,7 @@ static struct ctl_table yama_sysctl_table[] = {
};
static void __init yama_init_sysctl(void)
{
- if (!register_sysctl_paths(yama_sysctl_path, yama_sysctl_table))
+ if (!register_sysctl_paths(yama_sysctl_path, yama_sysctl_table, YAMA_SYSCTL_PATH_SIZE))
panic("Yama: sysctl registration failed.\n");
}
#else
--
2.20.1




2022-02-25 04:41:09

by Oliver Sang

[permalink] [raw]
Subject: [fs/proc] 99f251040f: BUG:KASAN:global-out-of-bounds_in__register_sysctl_paths



Greeting,

FYI, we noticed the following commit (built with clang-15):

commit: 99f251040f9933fcdb4d9a6bf9af130a53fb5556 ("[PATCH] fs/proc: Optimize arrays defined by struct ctl_path")
url: https://github.com/0day-ci/linux/commits/Meng-Tang/fs-proc-Optimize-arrays-defined-by-struct-ctl_path/20220223-175046
base: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git 5c1ee569660d4a205dced9cb4d0306b907fb7599
patch link: https://lore.kernel.org/linux-csky/[email protected]

in testcase: boot

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

caused below changes (please refer to attached dmesg/kmsg for entire log/backtrace):



If you fix the issue, kindly add following tag
Reported-by: kernel test robot <[email protected]>


[ 63.585952][ T1] BUG: KASAN: global-out-of-bounds in __register_sysctl_paths (??:?)
[ 63.585952][ T1] Read of size 8 at addr ffffffff851eaab0 by task swapper/0/1
[ 63.585952][ T1]
[ 63.585952][ T1] CPU: 1 PID: 1 Comm: swapper/0 Not tainted 5.17.0-rc5-00012-g99f251040f99 #1 d6725855da240bf400bb169b9fd6300ffdd774d1
[ 63.585952][ T1] Call Trace:
[ 63.585952][ T1] <TASK>
[ 63.585952][ T1] dump_stack_lvl (??:?)
[ 63.585952][ T1] print_address_description (report.c:?)
[ 63.585952][ T1] ? __register_sysctl_paths (??:?)
[ 63.585952][ T1] __kasan_report (report.c:?)
[ 63.585952][ T1] ? __register_sysctl_paths (??:?)
[ 63.585952][ T1] kasan_report (??:?)
[ 63.585952][ T1] __asan_report_load8_noabort (??:?)
[ 63.585952][ T1] __register_sysctl_paths (??:?)
[ 63.585952][ T1] register_sysctl_paths (??:?)
[ 63.585952][ T1] ? hardlockup_detector_perf_init (seccomp.c:?)
[ 63.585952][ T1] seccomp_sysctl_init (seccomp.c:?)
[ 63.585952][ T1] do_one_initcall (??:?)
[ 63.585952][ T1] do_initcall_level (main.c:?)
[ 63.585952][ T1] do_initcalls (main.c:?)
[ 63.585952][ T1] do_basic_setup (main.c:?)
[ 63.585952][ T1] kernel_init_freeable (main.c:?)
[ 63.585952][ T1] ? rest_init (main.c:?)
[ 63.585952][ T1] kernel_init (main.c:?)
[ 63.585952][ T1] ? rest_init (main.c:?)
[ 63.585952][ T1] ret_from_fork (??:?)
[ 63.585952][ T1] </TASK>
[ 63.585952][ T1]
[ 63.585952][ T1] The buggy address belongs to the variable:
[ 63.585952][ T1] seccomp_sysctl_path+0x10/0x20
[ 63.585952][ T1]
[ 63.585952][ T1] Memory state around the buggy address:
[ 63.585952][ T1] ffffffff851ea980: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[ 63.585952][ T1] ffffffff851eaa00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[ 63.585952][ T1] >ffffffff851eaa80: 04 f9 f9 f9 00 00 f9 f9 00 00 00 00 00 00 00 00
[ 63.585952][ T1] ^
[ 63.585952][ T1] ffffffff851eab00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[ 63.585952][ T1] ffffffff851eab80: f9 f9 f9 f9 00 00 00 00 00 00 00 00 00 00 00 00
[ 63.585952][ T1] ==================================================================
[ 63.585952][ T1] Disabling lock debugging due to kernel taint
[ 63.619393][ T1] Initialise system trusted keyrings
[ 63.620021][ T1] _warn_unseeded_randomness: 6 callbacks suppressed


To reproduce:

# build kernel
cd linux
cp config-5.17.0-rc5-00012-g99f251040f99 .config
make HOSTCC=clang-15 CC=clang-15 ARCH=x86_64 olddefconfig prepare modules_prepare bzImage modules
make HOSTCC=clang-15 CC=clang-15 ARCH=x86_64 INSTALL_MOD_PATH=<mod-install-dir> modules_install
cd <mod-install-dir>
find lib/ | cpio -o -H newc --quiet | gzip > modules.cgz


git clone https://github.com/intel/lkp-tests.git
cd lkp-tests
bin/lkp qemu -k <bzImage> -m modules.cgz job-script # job-script is attached in this email

# if come across any failure that blocks the test,
# please remove ~/.lkp and /lkp dir to run from a clean state.



---
0DAY/LKP+ Test Infrastructure Open Source Technology Center
https://lists.01.org/hyperkitty/list/[email protected] Intel Corporation

Thanks,
Oliver Sang


Attachments:
(No filename) (4.08 kB)
config-5.17.0-rc5-00012-g99f251040f99 (114.26 kB)
job-script (4.80 kB)
dmesg.xz (10.66 kB)
Download all attachments