On Fri, Mar 16, 2018 at 5:00 AM, Richard Guy Briggs <rgb@xxxxxxxxxx> wrote:
> Add container ID support to ptrace and signals. In particular, the "op"
> field provides a way to label the auxiliary record to which it is
> associated.
>
> Signed-off-by: Richard Guy Briggs <rgb@xxxxxxxxxx>
> ---
> include/linux/audit.h | 16 +++++++++++-----
> kernel/audit.c | 12 ++++++++----
> kernel/audit.h | 2 ++
> kernel/auditsc.c | 19 +++++++++++++++----
> 4 files changed, 36 insertions(+), 13 deletions(-)
...
> diff --git a/kernel/audit.c b/kernel/audit.c
> index a12f21f..b238be5 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -142,6 +142,7 @@ struct audit_net {
> kuid_t audit_sig_uid = INVALID_UID;
> pid_t audit_sig_pid = -1;
> u32 audit_sig_sid = 0;
> +u64 audit_sig_cid = INVALID_CID;
>
> /* Records can be lost in several ways:
> 0) [suppressed in audit_alloc]
> @@ -1438,6 +1439,7 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
> memcpy(sig_data->ctx, ctx, len);
> security_release_secctx(ctx, len);
> }
> + sig_data->cid = audit_sig_cid;
> audit_send_reply(skb, seq, AUDIT_SIGNAL_INFO, 0, 0,
> sig_data, sizeof(*sig_data) + len);
> kfree(sig_data);
> @@ -2051,20 +2053,22 @@ void audit_log_session_info(struct audit_buffer *ab)
>
> /*
> * audit_log_container_info - report container info
> - * @tsk: task to be recorded
> * @context: task or local context for record
> + * @op: containerid string description
> + * @containerid: container ID to report
> */
> -int audit_log_container_info(struct task_struct *tsk, struct audit_context *context)
> +int audit_log_container_info(struct audit_context *context,
> + char *op, u64 containerid)
> {
> struct audit_buffer *ab;
>
> - if (!audit_containerid_set(tsk))
> + if (!cid_valid(containerid))
> return 0;
> /* Generate AUDIT_CONTAINER_INFO with container ID */
> ab = audit_log_start(context, GFP_KERNEL, AUDIT_CONTAINER_INFO);
> if (!ab)
> return -ENOMEM;
> - audit_log_format(ab, "contid=%llu", audit_get_containerid(tsk));
> + audit_log_format(ab, "op=%s contid=%llu", op, containerid);
> audit_log_end(ab);
> return 0;
> }
Let's get these changes into the first patch where
audit_log_container_info() is defined. Why? This inserts a new field
into the record which is a no-no. Yes, it is one single patchset, but
they are still separate patches and who knows which patches a given
distribution and/or tree may decide to backport.
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index 2bba324..2932ef1 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -113,6 +113,7 @@ struct audit_aux_data_pids {
> kuid_t target_uid[AUDIT_AUX_PIDS];
> unsigned int target_sessionid[AUDIT_AUX_PIDS];
> u32 target_sid[AUDIT_AUX_PIDS];
> + u64 target_cid[AUDIT_AUX_PIDS];
> char target_comm[AUDIT_AUX_PIDS][TASK_COMM_LEN];
> int pid_count;
> };
> @@ -1422,21 +1423,27 @@ static void audit_log_exit(struct audit_context *context, struct task_struct *ts
> for (aux = context->aux_pids; aux; aux = aux->next) {
> struct audit_aux_data_pids *axs = (void *)aux;
>
> - for (i = 0; i < axs->pid_count; i++)
> + for (i = 0; i < axs->pid_count; i++) {
> + char axsn[sizeof("aux0xN ")];
> +
> + sprintf(axsn, "aux0x%x", i);
> if (audit_log_pid_context(context, axs->target_pid[i],
> axs->target_auid[i],
> axs->target_uid[i],
> axs->target_sessionid[i],
> axs->target_sid[i],
> - axs->target_comm[i]))
> + axs->target_comm[i])
> + && audit_log_container_info(context, axsn, axs->target_cid[i]))
Shouldn't this be an OR instead of an AND?
> call_panic = 1;
> + }
> }
>
> if (context->target_pid &&
> audit_log_pid_context(context, context->target_pid,
> context->target_auid, context->target_uid,
> context->target_sessionid,
> - context->target_sid, context->target_comm))
> + context->target_sid, context->target_comm)
> + && audit_log_container_info(context, "target", context->target_cid))
Same question.
> call_panic = 1;
>
> if (context->pwd.dentry && context->pwd.mnt) {
--
paul moore
www.paul-moore.com
