On Sun, Dec 1, 2024 at 8:42 PM <[email protected]> wrote:
>
> From: lihaojie <[email protected]>
>
> 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 <[email protected]>
> ---
>  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

-- 
paul-moore.com

Reply via email to