* [PATCH v4 02/11] auditsc: Replace memcpy() with __get_task_comm()
2024-06-28 9:05 ` [PATCH v4 01/11] fs/exec: Drop task_lock() inside __get_task_comm() Yafang Shao
@ 2024-06-28 9:05 ` Yafang Shao
2024-06-28 9:05 ` [PATCH v4 03/11] security: " Yafang Shao
` (8 subsequent siblings)
9 siblings, 0 replies; 13+ messages in thread
From: Yafang Shao @ 2024-06-28 9:05 UTC (permalink / raw)
To: torvalds, laoar.shao
Cc: akpm, alexei.starovoitov, audit, bpf, catalin.marinas, dri-devel,
ebiederm, linux-fsdevel, linux-mm, linux-security-module,
linux-trace-kernel, netdev, penguin-kernel, rostedt, selinux,
Paul Moore, Eric Paris
Using __get_task_comm() to read the task comm ensures that the name is
always NUL-terminated, regardless of the source string. This approach also
facilitates future extensions to the task comm.
Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
Acked-by: Paul Moore <paul@paul-moore.com>
Cc: Eric Paris <eparis@redhat.com>
---
kernel/auditsc.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 6f0d6fb6523f..0459a141dc86 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -2730,7 +2730,7 @@ void __audit_ptrace(struct task_struct *t)
context->target_uid = task_uid(t);
context->target_sessionid = audit_get_sessionid(t);
security_task_getsecid_obj(t, &context->target_sid);
- memcpy(context->target_comm, t->comm, TASK_COMM_LEN);
+ __get_task_comm(context->target_comm, TASK_COMM_LEN, t);
}
/**
@@ -2757,7 +2757,7 @@ int audit_signal_info_syscall(struct task_struct *t)
ctx->target_uid = t_uid;
ctx->target_sessionid = audit_get_sessionid(t);
security_task_getsecid_obj(t, &ctx->target_sid);
- memcpy(ctx->target_comm, t->comm, TASK_COMM_LEN);
+ __get_task_comm(ctx->target_comm, TASK_COMM_LEN, t);
return 0;
}
@@ -2778,7 +2778,7 @@ int audit_signal_info_syscall(struct task_struct *t)
axp->target_uid[axp->pid_count] = t_uid;
axp->target_sessionid[axp->pid_count] = audit_get_sessionid(t);
security_task_getsecid_obj(t, &axp->target_sid[axp->pid_count]);
- memcpy(axp->target_comm[axp->pid_count], t->comm, TASK_COMM_LEN);
+ __get_task_comm(axp->target_comm[axp->pid_count], TASK_COMM_LEN, t);
axp->pid_count++;
return 0;
--
2.43.5
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v4 03/11] security: Replace memcpy() with __get_task_comm()
2024-06-28 9:05 ` [PATCH v4 01/11] fs/exec: Drop task_lock() inside __get_task_comm() Yafang Shao
2024-06-28 9:05 ` [PATCH v4 02/11] auditsc: Replace memcpy() with __get_task_comm() Yafang Shao
@ 2024-06-28 9:05 ` Yafang Shao
2024-06-28 9:05 ` [PATCH v4 04/11] bpftool: Ensure task comm is always NUL-terminated Yafang Shao
` (7 subsequent siblings)
9 siblings, 0 replies; 13+ messages in thread
From: Yafang Shao @ 2024-06-28 9:05 UTC (permalink / raw)
To: torvalds, laoar.shao
Cc: akpm, alexei.starovoitov, audit, bpf, catalin.marinas, dri-devel,
ebiederm, linux-fsdevel, linux-mm, linux-security-module,
linux-trace-kernel, netdev, penguin-kernel, rostedt, selinux,
Paul Moore, James Morris, Serge E. Hallyn, Stephen Smalley,
Ondrej Mosnacek
Quoted from Linus [0]:
selinux never wanted a lock, and never wanted any kind of *consistent*
result, it just wanted a *stable* result.
Using __get_task_comm() to read the task comm ensures that the name is
always NUL-terminated, regardless of the source string. This approach also
facilitates future extensions to the task comm.
Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
LINK: https://lore.kernel.org/all/CAHk-=wivfrF0_zvf+oj6==Sh=-npJooP8chLPEfaFV0oNYTTBA@mail.gmail.com/ [0]
Acked-by: Paul Moore <paul@paul-moore.com>
Cc: James Morris <jmorris@namei.org>
Cc: "Serge E. Hallyn" <serge@hallyn.com>
Cc: Stephen Smalley <stephen.smalley.work@gmail.com>
Cc: Ondrej Mosnacek <omosnace@redhat.com>
---
security/lsm_audit.c | 4 ++--
security/selinux/selinuxfs.c | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/security/lsm_audit.c b/security/lsm_audit.c
index 849e832719e2..a922e4339dd5 100644
--- a/security/lsm_audit.c
+++ b/security/lsm_audit.c
@@ -207,7 +207,7 @@ static void dump_common_audit_data(struct audit_buffer *ab,
BUILD_BUG_ON(sizeof(a->u) > sizeof(void *)*2);
audit_log_format(ab, " pid=%d comm=", task_tgid_nr(current));
- audit_log_untrustedstring(ab, memcpy(comm, current->comm, sizeof(comm)));
+ audit_log_untrustedstring(ab, __get_task_comm(comm, sizeof(comm), current));
switch (a->type) {
case LSM_AUDIT_DATA_NONE:
@@ -302,7 +302,7 @@ static void dump_common_audit_data(struct audit_buffer *ab,
char comm[sizeof(tsk->comm)];
audit_log_format(ab, " opid=%d ocomm=", pid);
audit_log_untrustedstring(ab,
- memcpy(comm, tsk->comm, sizeof(comm)));
+ __get_task_comm(comm, sizeof(comm), tsk));
}
}
break;
diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c
index e172f182b65c..a8a2ec742576 100644
--- a/security/selinux/selinuxfs.c
+++ b/security/selinux/selinuxfs.c
@@ -708,7 +708,7 @@ static ssize_t sel_write_checkreqprot(struct file *file, const char __user *buf,
if (new_value) {
char comm[sizeof(current->comm)];
- memcpy(comm, current->comm, sizeof(comm));
+ __get_task_comm(comm, sizeof(comm), current);
pr_err("SELinux: %s (%d) set checkreqprot to 1. This is no longer supported.\n",
comm, current->pid);
}
--
2.43.5
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v4 04/11] bpftool: Ensure task comm is always NUL-terminated
2024-06-28 9:05 ` [PATCH v4 01/11] fs/exec: Drop task_lock() inside __get_task_comm() Yafang Shao
2024-06-28 9:05 ` [PATCH v4 02/11] auditsc: Replace memcpy() with __get_task_comm() Yafang Shao
2024-06-28 9:05 ` [PATCH v4 03/11] security: " Yafang Shao
@ 2024-06-28 9:05 ` Yafang Shao
2024-06-28 9:05 ` [PATCH v4 05/11] mm/util: Fix possible race condition in kstrdup() Yafang Shao
` (6 subsequent siblings)
9 siblings, 0 replies; 13+ messages in thread
From: Yafang Shao @ 2024-06-28 9:05 UTC (permalink / raw)
To: torvalds, laoar.shao
Cc: akpm, alexei.starovoitov, audit, bpf, catalin.marinas, dri-devel,
ebiederm, linux-fsdevel, linux-mm, linux-security-module,
linux-trace-kernel, netdev, penguin-kernel, rostedt, selinux,
Quentin Monnet
Let's explicitly ensure the destination string is NUL-terminated. This way,
it won't be affected by changes to the source string.
Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
Reviewed-by: Quentin Monnet <qmo@kernel.org>
---
tools/bpf/bpftool/pids.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/tools/bpf/bpftool/pids.c b/tools/bpf/bpftool/pids.c
index 9b898571b49e..23f488cf1740 100644
--- a/tools/bpf/bpftool/pids.c
+++ b/tools/bpf/bpftool/pids.c
@@ -54,6 +54,7 @@ static void add_ref(struct hashmap *map, struct pid_iter_entry *e)
ref = &refs->refs[refs->ref_cnt];
ref->pid = e->pid;
memcpy(ref->comm, e->comm, sizeof(ref->comm));
+ ref->comm[sizeof(ref->comm) - 1] = '\0';
refs->ref_cnt++;
return;
@@ -77,6 +78,7 @@ static void add_ref(struct hashmap *map, struct pid_iter_entry *e)
ref = &refs->refs[0];
ref->pid = e->pid;
memcpy(ref->comm, e->comm, sizeof(ref->comm));
+ ref->comm[sizeof(ref->comm) - 1] = '\0';
refs->ref_cnt = 1;
refs->has_bpf_cookie = e->has_bpf_cookie;
refs->bpf_cookie = e->bpf_cookie;
--
2.43.5
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v4 05/11] mm/util: Fix possible race condition in kstrdup()
2024-06-28 9:05 ` [PATCH v4 01/11] fs/exec: Drop task_lock() inside __get_task_comm() Yafang Shao
` (2 preceding siblings ...)
2024-06-28 9:05 ` [PATCH v4 04/11] bpftool: Ensure task comm is always NUL-terminated Yafang Shao
@ 2024-06-28 9:05 ` Yafang Shao
2024-06-28 9:05 ` [PATCH v4 06/11] mm/util: Deduplicate code in {kstrdup,kstrndup,kmemdup_nul} Yafang Shao
` (5 subsequent siblings)
9 siblings, 0 replies; 13+ messages in thread
From: Yafang Shao @ 2024-06-28 9:05 UTC (permalink / raw)
To: torvalds, laoar.shao
Cc: akpm, alexei.starovoitov, audit, bpf, catalin.marinas, dri-devel,
ebiederm, linux-fsdevel, linux-mm, linux-security-module,
linux-trace-kernel, netdev, penguin-kernel, rostedt, selinux
In kstrdup(), it is critical to ensure that the dest string is always
NUL-terminated. However, potential race condidtion can occur between a
writer and a reader.
Consider the following scenario involving task->comm:
reader writer
len = strlen(s) + 1;
strlcpy(tsk->comm, buf, sizeof(tsk->comm));
memcpy(buf, s, len);
In this case, there is a race condition between the reader and the
writer. The reader calculate the length of the string `s` based on the
old value of task->comm. However, during the memcpy(), the string `s`
might be updated by the writer to a new value of task->comm.
If the new task->comm is larger than the old one, the `buf` might not be
NUL-terminated. This can lead to undefined behavior and potential
security vulnerabilities.
Let's fix it by explicitly adding a NUL-terminator.
Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
---
mm/util.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/mm/util.c b/mm/util.c
index c9e519e6811f..41c7875572ed 100644
--- a/mm/util.c
+++ b/mm/util.c
@@ -60,8 +60,14 @@ char *kstrdup(const char *s, gfp_t gfp)
len = strlen(s) + 1;
buf = kmalloc_track_caller(len, gfp);
- if (buf)
+ if (buf) {
memcpy(buf, s, len);
+ /* During memcpy(), the string might be updated to a new value,
+ * which could be longer than the string when strlen() is
+ * called. Therefore, we need to add a null termimator.
+ */
+ buf[len - 1] = '\0';
+ }
return buf;
}
EXPORT_SYMBOL(kstrdup);
--
2.43.5
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v4 06/11] mm/util: Deduplicate code in {kstrdup,kstrndup,kmemdup_nul}
2024-06-28 9:05 ` [PATCH v4 01/11] fs/exec: Drop task_lock() inside __get_task_comm() Yafang Shao
` (3 preceding siblings ...)
2024-06-28 9:05 ` [PATCH v4 05/11] mm/util: Fix possible race condition in kstrdup() Yafang Shao
@ 2024-06-28 9:05 ` Yafang Shao
2024-06-28 9:05 ` [PATCH v4 07/11] mm/kmemleak: Replace strncpy() with __get_task_comm() Yafang Shao
` (4 subsequent siblings)
9 siblings, 0 replies; 13+ messages in thread
From: Yafang Shao @ 2024-06-28 9:05 UTC (permalink / raw)
To: torvalds, laoar.shao
Cc: akpm, alexei.starovoitov, audit, bpf, catalin.marinas, dri-devel,
ebiederm, linux-fsdevel, linux-mm, linux-security-module,
linux-trace-kernel, netdev, penguin-kernel, rostedt, selinux,
Simon Horman, Matthew Wilcox
These three functions follow the same pattern. To deduplicate the code,
let's introduce a common helper __kmemdup_nul().
Suggested-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
Cc: Simon Horman <horms@kernel.org>
Cc: Matthew Wilcox <willy@infradead.org>
---
mm/util.c | 67 +++++++++++++++++++++----------------------------------
1 file changed, 26 insertions(+), 41 deletions(-)
diff --git a/mm/util.c b/mm/util.c
index 41c7875572ed..62a4686352b9 100644
--- a/mm/util.c
+++ b/mm/util.c
@@ -43,33 +43,40 @@ void kfree_const(const void *x)
EXPORT_SYMBOL(kfree_const);
/**
- * kstrdup - allocate space for and copy an existing string
- * @s: the string to duplicate
+ * __kmemdup_nul - Create a NUL-terminated string from @s, which might be unterminated.
+ * @s: The data to copy
+ * @len: The size of the data, including the null terminator
* @gfp: the GFP mask used in the kmalloc() call when allocating memory
*
- * Return: newly allocated copy of @s or %NULL in case of error
+ * Return: newly allocated copy of @s with NUL-termination or %NULL in
+ * case of error
*/
-noinline
-char *kstrdup(const char *s, gfp_t gfp)
+static __always_inline char *__kmemdup_nul(const char *s, size_t len, gfp_t gfp)
{
- size_t len;
char *buf;
- if (!s)
+ buf = kmalloc_track_caller(len, gfp);
+ if (!buf)
return NULL;
- len = strlen(s) + 1;
- buf = kmalloc_track_caller(len, gfp);
- if (buf) {
- memcpy(buf, s, len);
- /* During memcpy(), the string might be updated to a new value,
- * which could be longer than the string when strlen() is
- * called. Therefore, we need to add a null termimator.
- */
- buf[len - 1] = '\0';
- }
+ memcpy(buf, s, len);
+ /* Ensure the buf is always NUL-terminated, regardless of @s. */
+ buf[len - 1] = '\0';
return buf;
}
+
+/**
+ * kstrdup - allocate space for and copy an existing string
+ * @s: the string to duplicate
+ * @gfp: the GFP mask used in the kmalloc() call when allocating memory
+ *
+ * Return: newly allocated copy of @s or %NULL in case of error
+ */
+noinline
+char *kstrdup(const char *s, gfp_t gfp)
+{
+ return s ? __kmemdup_nul(s, strlen(s) + 1, gfp) : NULL;
+}
EXPORT_SYMBOL(kstrdup);
/**
@@ -104,19 +111,7 @@ EXPORT_SYMBOL(kstrdup_const);
*/
char *kstrndup(const char *s, size_t max, gfp_t gfp)
{
- size_t len;
- char *buf;
-
- if (!s)
- return NULL;
-
- len = strnlen(s, max);
- buf = kmalloc_track_caller(len+1, gfp);
- if (buf) {
- memcpy(buf, s, len);
- buf[len] = '\0';
- }
- return buf;
+ return s ? __kmemdup_nul(s, strnlen(s, max) + 1, gfp) : NULL;
}
EXPORT_SYMBOL(kstrndup);
@@ -190,17 +185,7 @@ EXPORT_SYMBOL(kvmemdup);
*/
char *kmemdup_nul(const char *s, size_t len, gfp_t gfp)
{
- char *buf;
-
- if (!s)
- return NULL;
-
- buf = kmalloc_track_caller(len + 1, gfp);
- if (buf) {
- memcpy(buf, s, len);
- buf[len] = '\0';
- }
- return buf;
+ return s ? __kmemdup_nul(s, len + 1, gfp) : NULL;
}
EXPORT_SYMBOL(kmemdup_nul);
--
2.43.5
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v4 07/11] mm/kmemleak: Replace strncpy() with __get_task_comm()
2024-06-28 9:05 ` [PATCH v4 01/11] fs/exec: Drop task_lock() inside __get_task_comm() Yafang Shao
` (4 preceding siblings ...)
2024-06-28 9:05 ` [PATCH v4 06/11] mm/util: Deduplicate code in {kstrdup,kstrndup,kmemdup_nul} Yafang Shao
@ 2024-06-28 9:05 ` Yafang Shao
2024-06-28 9:05 ` [PATCH v4 08/11] tsacct: " Yafang Shao
` (3 subsequent siblings)
9 siblings, 0 replies; 13+ messages in thread
From: Yafang Shao @ 2024-06-28 9:05 UTC (permalink / raw)
To: torvalds, laoar.shao
Cc: akpm, alexei.starovoitov, audit, bpf, catalin.marinas, dri-devel,
ebiederm, linux-fsdevel, linux-mm, linux-security-module,
linux-trace-kernel, netdev, penguin-kernel, rostedt, selinux
Since task lock was dropped from __get_task_comm(), it's safe to call it
from kmemleak.
Using __get_task_comm() to read the task comm ensures that the name is
always NUL-terminated, regardless of the source string. This approach also
facilitates future extensions to the task comm.
Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
---
mm/kmemleak.c | 8 +-------
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/mm/kmemleak.c b/mm/kmemleak.c
index d5b6fba44fc9..ef29aaab88a0 100644
--- a/mm/kmemleak.c
+++ b/mm/kmemleak.c
@@ -663,13 +663,7 @@ static struct kmemleak_object *__alloc_object(gfp_t gfp)
strncpy(object->comm, "softirq", sizeof(object->comm));
} else {
object->pid = current->pid;
- /*
- * There is a small chance of a race with set_task_comm(),
- * however using get_task_comm() here may cause locking
- * dependency issues with current->alloc_lock. In the worst
- * case, the command line is not correct.
- */
- strncpy(object->comm, current->comm, sizeof(object->comm));
+ __get_task_comm(object->comm, sizeof(object->comm), current);
}
/* kernel backtrace */
--
2.43.5
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v4 08/11] tsacct: Replace strncpy() with __get_task_comm()
2024-06-28 9:05 ` [PATCH v4 01/11] fs/exec: Drop task_lock() inside __get_task_comm() Yafang Shao
` (5 preceding siblings ...)
2024-06-28 9:05 ` [PATCH v4 07/11] mm/kmemleak: Replace strncpy() with __get_task_comm() Yafang Shao
@ 2024-06-28 9:05 ` Yafang Shao
2024-06-28 9:05 ` [PATCH v4 09/11] tracing: " Yafang Shao
` (2 subsequent siblings)
9 siblings, 0 replies; 13+ messages in thread
From: Yafang Shao @ 2024-06-28 9:05 UTC (permalink / raw)
To: torvalds, laoar.shao
Cc: akpm, alexei.starovoitov, audit, bpf, catalin.marinas, dri-devel,
ebiederm, linux-fsdevel, linux-mm, linux-security-module,
linux-trace-kernel, netdev, penguin-kernel, rostedt, selinux
Using __get_task_comm() to read the task comm ensures that the name is
always NUL-terminated, regardless of the source string. This approach also
facilitates future extensions to the task comm.
Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
---
kernel/tsacct.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/tsacct.c b/kernel/tsacct.c
index 4252f0645b9e..6b094d5d9135 100644
--- a/kernel/tsacct.c
+++ b/kernel/tsacct.c
@@ -76,7 +76,7 @@ void bacct_add_tsk(struct user_namespace *user_ns,
stats->ac_minflt = tsk->min_flt;
stats->ac_majflt = tsk->maj_flt;
- strncpy(stats->ac_comm, tsk->comm, sizeof(stats->ac_comm));
+ __get_task_comm(stats->ac_comm, sizeof(stats->ac_comm), tsk);
}
--
2.43.5
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v4 09/11] tracing: Replace strncpy() with __get_task_comm()
2024-06-28 9:05 ` [PATCH v4 01/11] fs/exec: Drop task_lock() inside __get_task_comm() Yafang Shao
` (6 preceding siblings ...)
2024-06-28 9:05 ` [PATCH v4 08/11] tsacct: " Yafang Shao
@ 2024-06-28 9:05 ` Yafang Shao
2024-06-28 9:05 ` [PATCH v4 10/11] net: Replace strcpy() " Yafang Shao
2024-06-28 9:05 ` [PATCH v4 11/11] drm: " Yafang Shao
9 siblings, 0 replies; 13+ messages in thread
From: Yafang Shao @ 2024-06-28 9:05 UTC (permalink / raw)
To: torvalds, laoar.shao
Cc: akpm, alexei.starovoitov, audit, bpf, catalin.marinas, dri-devel,
ebiederm, linux-fsdevel, linux-mm, linux-security-module,
linux-trace-kernel, netdev, penguin-kernel, rostedt, selinux,
Masami Hiramatsu, Mathieu Desnoyers
Using __get_task_comm() to read the task comm ensures that the name is
always NUL-terminated, regardless of the source string. This approach also
facilitates future extensions to the task comm.
Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
---
kernel/trace/trace.c | 2 +-
kernel/trace/trace_events_hist.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 578a49ff5c32..ce94a86154a2 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -1907,7 +1907,7 @@ __update_max_tr(struct trace_array *tr, struct task_struct *tsk, int cpu)
max_data->critical_start = data->critical_start;
max_data->critical_end = data->critical_end;
- strncpy(max_data->comm, tsk->comm, TASK_COMM_LEN);
+ __get_task_comm(max_data->comm, TASK_COMM_LEN, tsk);
max_data->pid = tsk->pid;
/*
* If tsk == current, then use current_uid(), as that does not use
diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index 6ece1308d36a..721d4758a79f 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -1599,7 +1599,7 @@ static inline void save_comm(char *comm, struct task_struct *task)
return;
}
- strncpy(comm, task->comm, TASK_COMM_LEN);
+ __get_task_comm(comm, TASK_COMM_LEN, task);
}
static void hist_elt_data_free(struct hist_elt_data *elt_data)
--
2.43.5
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v4 10/11] net: Replace strcpy() with __get_task_comm()
2024-06-28 9:05 ` [PATCH v4 01/11] fs/exec: Drop task_lock() inside __get_task_comm() Yafang Shao
` (7 preceding siblings ...)
2024-06-28 9:05 ` [PATCH v4 09/11] tracing: " Yafang Shao
@ 2024-06-28 9:05 ` Yafang Shao
2024-06-28 9:05 ` [PATCH v4 11/11] drm: " Yafang Shao
9 siblings, 0 replies; 13+ messages in thread
From: Yafang Shao @ 2024-06-28 9:05 UTC (permalink / raw)
To: torvalds, laoar.shao
Cc: akpm, alexei.starovoitov, audit, bpf, catalin.marinas, dri-devel,
ebiederm, linux-fsdevel, linux-mm, linux-security-module,
linux-trace-kernel, netdev, penguin-kernel, rostedt, selinux,
David S. Miller, David Ahern, Eric Dumazet, Jakub Kicinski,
Paolo Abeni
To prevent errors from occurring when the src string is longer than the dst
string in strcpy(), we should use __get_task_comm() instead. This approach
also facilitates future extensions to the task comm.
Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: David Ahern <dsahern@kernel.org>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Paolo Abeni <pabeni@redhat.com>
---
net/ipv6/ndisc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c
index 254b192c5705..10d8e8c6ca02 100644
--- a/net/ipv6/ndisc.c
+++ b/net/ipv6/ndisc.c
@@ -1942,7 +1942,7 @@ static void ndisc_warn_deprecated_sysctl(const struct ctl_table *ctl,
static char warncomm[TASK_COMM_LEN];
static int warned;
if (strcmp(warncomm, current->comm) && warned < 5) {
- strcpy(warncomm, current->comm);
+ __get_task_comm(warncomm, TASK_COMM_LEN, current);
pr_warn("process `%s' is using deprecated sysctl (%s) net.ipv6.neigh.%s.%s - use net.ipv6.neigh.%s.%s_ms instead\n",
warncomm, func,
dev_name, ctl->procname,
--
2.43.5
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v4 11/11] drm: Replace strcpy() with __get_task_comm()
2024-06-28 9:05 ` [PATCH v4 01/11] fs/exec: Drop task_lock() inside __get_task_comm() Yafang Shao
` (8 preceding siblings ...)
2024-06-28 9:05 ` [PATCH v4 10/11] net: Replace strcpy() " Yafang Shao
@ 2024-06-28 9:05 ` Yafang Shao
9 siblings, 0 replies; 13+ messages in thread
From: Yafang Shao @ 2024-06-28 9:05 UTC (permalink / raw)
To: torvalds, laoar.shao
Cc: akpm, alexei.starovoitov, audit, bpf, catalin.marinas, dri-devel,
ebiederm, linux-fsdevel, linux-mm, linux-security-module,
linux-trace-kernel, netdev, penguin-kernel, rostedt, selinux,
Daniel Vetter, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie
To prevent erros from occurring when the src string is longer than the
dst string in strcpy(), we should use __get_task_comm() instead. This
approach also facilitates future extensions to the task comm.
Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Maxime Ripard <mripard@kernel.org>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: David Airlie <airlied@gmail.com>
---
drivers/gpu/drm/drm_framebuffer.c | 2 +-
drivers/gpu/drm/i915/i915_gpu_error.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_framebuffer.c b/drivers/gpu/drm/drm_framebuffer.c
index 888aadb6a4ac..25262b07ffaf 100644
--- a/drivers/gpu/drm/drm_framebuffer.c
+++ b/drivers/gpu/drm/drm_framebuffer.c
@@ -868,7 +868,7 @@ int drm_framebuffer_init(struct drm_device *dev, struct drm_framebuffer *fb,
INIT_LIST_HEAD(&fb->filp_head);
fb->funcs = funcs;
- strcpy(fb->comm, current->comm);
+ __get_task_comm(fb->comm, sizeof(fb->comm), current);
ret = __drm_mode_object_add(dev, &fb->base, DRM_MODE_OBJECT_FB,
false, drm_framebuffer_free);
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
index 625b3c024540..b2c16a53bd24 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -1411,7 +1411,7 @@ static bool record_context(struct i915_gem_context_coredump *e,
rcu_read_lock();
task = pid_task(ctx->pid, PIDTYPE_PID);
if (task) {
- strcpy(e->comm, task->comm);
+ __get_task_comm(e->comm, sizeof(e->comm), task);
e->pid = task->pid;
}
rcu_read_unlock();
--
2.43.5
^ permalink raw reply related [flat|nested] 13+ messages in thread