| Message ID | 20241202014217.34580-1-15074444048@163.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Paul Moore |
| Headers | show |
| Series | kernel/audit: Use struct_group() for memcpy() region | expand |
On Sun, Dec 1, 2024 at 8:42 PM <15074444048@163.com> wrote: > > From: lihaojie <lihaojie@kylinos.cn> > > In preparation for FORTIFY_SOURCE performing compile-time and run-time > field bounds checking for memcpy(), memmove(), and memset(), avoid > intentionally writing across neighboring fields. > > Use struct_group() in struct audit_context around members target_comm[], > This will allow memcpy() and sizeof() to more easily reason about sizes, > improve readability, and avoid future warnings about writing beyond the > end of target_comm[]. > > "pahole" shows no size nor member offset changes to struct vlan_ethhdr. > "objdump -d" shows no object code changes. That's obviously a cut-n-paste error above, please fix that. You also sent this patch three times, that's very annoying, please don't do that in the future. Finally, can you provide a link with an explanation as to how the struct_group() union/annotations is the only way to do this? It's kinda ugly and if there is another way to do this I would like to understand what it entails. > Signed-off-by: lihaojie <lihaojie@kylinos.cn> > --- > kernel/audit.h | 5 ++++- > kernel/auditsc.c | 4 ++-- > 2 files changed, 6 insertions(+), 3 deletions(-) > > diff --git a/kernel/audit.h b/kernel/audit.h > index 0211cb307d30..20483670ea02 100644 > --- a/kernel/audit.h > +++ b/kernel/audit.h > @@ -145,7 +145,10 @@ struct audit_context { > kuid_t target_uid; > unsigned int target_sessionid; > struct lsm_prop target_ref; > - char target_comm[TASK_COMM_LEN]; > + > + struct_group(comm, > + char target_comm[TASK_COMM_LEN]; > + ); > > struct audit_tree_refs *trees, *first_trees; > struct list_head killed_trees; > diff --git a/kernel/auditsc.c b/kernel/auditsc.c > index 91afdd0d036e..e279762463b0 100644 > --- a/kernel/auditsc.c > +++ b/kernel/auditsc.c > @@ -2729,7 +2729,7 @@ void __audit_ptrace(struct task_struct *t) > context->target_uid = task_uid(t); > context->target_sessionid = audit_get_sessionid(t); > security_task_getlsmprop_obj(t, &context->target_ref); > - memcpy(context->target_comm, t->comm, TASK_COMM_LEN); > + memcpy(&context->comm, t->comm, TASK_COMM_LEN); > } > > /** > @@ -2756,7 +2756,7 @@ int audit_signal_info_syscall(struct task_struct *t) > ctx->target_uid = t_uid; > ctx->target_sessionid = audit_get_sessionid(t); > security_task_getlsmprop_obj(t, &ctx->target_ref); > - memcpy(ctx->target_comm, t->comm, TASK_COMM_LEN); > + memcpy(&ctx->comm, t->comm, TASK_COMM_LEN); > return 0; > } > > -- > 2.25.1
diff --git a/kernel/audit.h b/kernel/audit.h index 0211cb307d30..20483670ea02 100644 --- a/kernel/audit.h +++ b/kernel/audit.h @@ -145,7 +145,10 @@ struct audit_context { kuid_t target_uid; unsigned int target_sessionid; struct lsm_prop target_ref; - char target_comm[TASK_COMM_LEN]; + + struct_group(comm, + char target_comm[TASK_COMM_LEN]; + ); struct audit_tree_refs *trees, *first_trees; struct list_head killed_trees; diff --git a/kernel/auditsc.c b/kernel/auditsc.c index 91afdd0d036e..e279762463b0 100644 --- a/kernel/auditsc.c +++ b/kernel/auditsc.c @@ -2729,7 +2729,7 @@ void __audit_ptrace(struct task_struct *t) context->target_uid = task_uid(t); context->target_sessionid = audit_get_sessionid(t); security_task_getlsmprop_obj(t, &context->target_ref); - memcpy(context->target_comm, t->comm, TASK_COMM_LEN); + memcpy(&context->comm, t->comm, TASK_COMM_LEN); } /** @@ -2756,7 +2756,7 @@ int audit_signal_info_syscall(struct task_struct *t) ctx->target_uid = t_uid; ctx->target_sessionid = audit_get_sessionid(t); security_task_getlsmprop_obj(t, &ctx->target_ref); - memcpy(ctx->target_comm, t->comm, TASK_COMM_LEN); + memcpy(&ctx->comm, t->comm, TASK_COMM_LEN); return 0; }