|
Message-ID: <24bcbaee40a4174cb5d9fa876f88b2a1869a4870.camel@perches.com> Date: Mon, 22 Jul 2019 21:42:51 -0700 From: Joe Perches <joe@...ches.com> To: Andrew Morton <akpm@...ux-foundation.org> Cc: Linus Torvalds <torvalds@...ux-foundation.org>, linux-kernel@...r.kernel.org, Jonathan Corbet <corbet@....net>, Stephen Kitt <steve@....org>, Kees Cook <keescook@...omium.org>, Nitin Gote <nitin.r.gote@...el.com>, jannh@...gle.com, kernel-hardening@...ts.openwall.com, Rasmus Villemoes <rasmus.villemoes@...vas.dk> Subject: Re: [PATCH 1/2] string: Add stracpy and stracpy_pad mechanisms On Mon, 2019-07-22 at 21:35 -0700, Andrew Morton wrote: > On Mon, 22 Jul 2019 17:38:15 -0700 Joe Perches <joe@...ches.com> wrote: > > > Several uses of strlcpy and strscpy have had defects because the > > last argument of each function is misused or typoed. > > > > Add macro mechanisms to avoid this defect. > > > > stracpy (copy a string to a string array) must have a string > > array as the first argument (to) and uses sizeof(to) as the > > size. > > > > These mechanisms verify that the to argument is an array of > > char or other compatible types like u8 or unsigned char. > > > > A BUILD_BUG is emitted when the type of to is not compatible. > > > > It would be nice to include some conversions. To demonstrate the need, > to test the code, etc. How about all the kernel/ ? --- kernel/acct.c | 2 +- kernel/cgroup/cgroup-v1.c | 3 +-- kernel/debug/gdbstub.c | 4 ++-- kernel/debug/kdb/kdb_support.c | 2 +- kernel/events/core.c | 4 ++-- kernel/module.c | 2 +- kernel/printk/printk.c | 2 +- kernel/time/clocksource.c | 2 +- 8 files changed, 10 insertions(+), 11 deletions(-) diff --git a/kernel/acct.c b/kernel/acct.c index 81f9831a7859..5ad29248b654 100644 --- a/kernel/acct.c +++ b/kernel/acct.c @@ -425,7 +425,7 @@ static void fill_ac(acct_t *ac) memset(ac, 0, sizeof(acct_t)); ac->ac_version = ACCT_VERSION | ACCT_BYTEORDER; - strlcpy(ac->ac_comm, current->comm, sizeof(ac->ac_comm)); + stracpy(ac->ac_comm, current->comm); /* calculate run_time in nsec*/ run_time = ktime_get_ns(); diff --git a/kernel/cgroup/cgroup-v1.c b/kernel/cgroup/cgroup-v1.c index 88006be40ea3..dd4f041e4179 100644 --- a/kernel/cgroup/cgroup-v1.c +++ b/kernel/cgroup/cgroup-v1.c @@ -571,8 +571,7 @@ static ssize_t cgroup_release_agent_write(struct kernfs_open_file *of, if (!cgrp) return -ENODEV; spin_lock(&release_agent_path_lock); - strlcpy(cgrp->root->release_agent_path, strstrip(buf), - sizeof(cgrp->root->release_agent_path)); + stracpy(cgrp->root->release_agent_path, strstrip(buf)); spin_unlock(&release_agent_path_lock); cgroup_kn_unlock(of->kn); return nbytes; diff --git a/kernel/debug/gdbstub.c b/kernel/debug/gdbstub.c index 4b280fc7dd67..a263f27f51ad 100644 --- a/kernel/debug/gdbstub.c +++ b/kernel/debug/gdbstub.c @@ -1095,10 +1095,10 @@ int gdbstub_state(struct kgdb_state *ks, char *cmd) return error; case 's': case 'c': - strscpy(remcom_in_buffer, cmd, sizeof(remcom_in_buffer)); + stracpy(remcom_in_buffer, cmd); return 0; case '$': - strscpy(remcom_in_buffer, cmd, sizeof(remcom_in_buffer)); + stracpy(remcom_in_buffer, cmd); gdbstub_use_prev_in_buf = strlen(remcom_in_buffer); gdbstub_prev_in_buf_pos = 0; return 0; diff --git a/kernel/debug/kdb/kdb_support.c b/kernel/debug/kdb/kdb_support.c index b8e6306e7e13..b49b6c3976c7 100644 --- a/kernel/debug/kdb/kdb_support.c +++ b/kernel/debug/kdb/kdb_support.c @@ -192,7 +192,7 @@ int kallsyms_symbol_complete(char *prefix_name, int max_len) while ((name = kdb_walk_kallsyms(&pos))) { if (strncmp(name, prefix_name, prefix_len) == 0) { - strscpy(ks_namebuf, name, sizeof(ks_namebuf)); + stracpy(ks_namebuf, name); /* Work out the longest name that matches the prefix */ if (++number == 1) { prev_len = min_t(int, max_len-1, diff --git a/kernel/events/core.c b/kernel/events/core.c index 026a14541a38..25bd8c777270 100644 --- a/kernel/events/core.c +++ b/kernel/events/core.c @@ -7049,7 +7049,7 @@ static void perf_event_comm_event(struct perf_comm_event *comm_event) unsigned int size; memset(comm, 0, sizeof(comm)); - strlcpy(comm, comm_event->task->comm, sizeof(comm)); + stracpy(comm, comm_event->task->comm); size = ALIGN(strlen(comm)+1, sizeof(u64)); comm_event->comm = comm; @@ -7394,7 +7394,7 @@ static void perf_event_mmap_event(struct perf_mmap_event *mmap_event) } cpy_name: - strlcpy(tmp, name, sizeof(tmp)); + stracpy(tmp, name); name = tmp; got_name: /* diff --git a/kernel/module.c b/kernel/module.c index 5933395af9a0..39384b0c90b8 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -1021,7 +1021,7 @@ SYSCALL_DEFINE2(delete_module, const char __user *, name_user, async_synchronize_full(); /* Store the name of the last unloaded module for diagnostic purposes */ - strlcpy(last_unloaded_module, mod->name, sizeof(last_unloaded_module)); + stracpy(last_unloaded_module, mod->name); free_module(mod); return 0; diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c index 424abf802f02..029633052be4 100644 --- a/kernel/printk/printk.c +++ b/kernel/printk/printk.c @@ -2127,7 +2127,7 @@ static int __add_preferred_console(char *name, int idx, char *options, return -E2BIG; if (!brl_options) preferred_console = i; - strlcpy(c->name, name, sizeof(c->name)); + stracpy(c->name, name); c->options = options; braille_set_options(c, brl_options); diff --git a/kernel/time/clocksource.c b/kernel/time/clocksource.c index fff5f64981c6..f0c833d89ace 100644 --- a/kernel/time/clocksource.c +++ b/kernel/time/clocksource.c @@ -1203,7 +1203,7 @@ static int __init boot_override_clocksource(char* str) { mutex_lock(&clocksource_mutex); if (str) - strlcpy(override_name, str, sizeof(override_name)); + stracpy(override_name, str); mutex_unlock(&clocksource_mutex); return 1; }
Powered by blists - more mailing lists
Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.