Re: [RFC PATCH 2/4] audit: kernel generated netlink traffic should have a portid of 0

[Richard Guy Briggs <rgb@redhat.com>]

On 2017-03-21 14:58, Paul Moore wrote:
> From: Paul Moore <paul@paul-moore.com>
> 
> We were setting the portid incorrectly in the netlink message headers,
> fix that to always be 0 (nlmsg_pid = 0).
> 
> Signed-off-by: Paul Moore <paul@paul-moore.com>

Reviewed-by: Richard Guy Briggs <rgb@redhat.com>

> ---
>  include/linux/audit.h |    3 +--
>  kernel/audit.c        |   23 ++++++-----------------
>  kernel/audit.h        |    3 +--
>  kernel/auditfilter.c  |   14 ++++++--------
>  4 files changed, 14 insertions(+), 29 deletions(-)
> 
> diff --git a/include/linux/audit.h b/include/linux/audit.h
> index 504e784b7ffa..cc0497c39472 100644
> --- a/include/linux/audit.h
> +++ b/include/linux/audit.h
> @@ -163,8 +163,7 @@ extern void audit_log_task_info(struct audit_buffer *ab,
>  extern int		    audit_update_lsm_rules(void);
>  
>  				/* Private API (for audit.c only) */
> -extern int audit_rule_change(int type, __u32 portid, int seq,
> -				void *data, size_t datasz);
> +extern int audit_rule_change(int type, int seq, void *data, size_t datasz);
>  extern int audit_list_rules_send(struct sk_buff *request_skb, int seq);
>  
>  extern u32 audit_enabled;
> diff --git a/kernel/audit.c b/kernel/audit.c
> index 4037869b4b64..6cbf47a372e8 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -251,14 +251,6 @@ static struct sock *audit_get_sk(const struct net *net)
>  	return aunet->sk;
>  }
>  
> -static void audit_set_portid(struct audit_buffer *ab, __u32 portid)
> -{
> -	if (ab) {
> -		struct nlmsghdr *nlh = nlmsg_hdr(ab->skb);
> -		nlh->nlmsg_pid = portid;
> -	}
> -}
> -
>  void audit_panic(const char *message)
>  {
>  	switch (audit_failure) {
> @@ -819,7 +811,7 @@ int audit_send_list(void *_dest)
>  	return 0;
>  }
>  
> -struct sk_buff *audit_make_reply(__u32 portid, int seq, int type, int done,
> +struct sk_buff *audit_make_reply(int seq, int type, int done,
>  				 int multi, const void *payload, int size)
>  {
>  	struct sk_buff	*skb;
> @@ -832,7 +824,7 @@ struct sk_buff *audit_make_reply(__u32 portid, int seq, int type, int done,
>  	if (!skb)
>  		return NULL;
>  
> -	nlh	= nlmsg_put(skb, portid, seq, t, size, flags);
> +	nlh	= nlmsg_put(skb, 0, seq, t, size, flags);
>  	if (!nlh)
>  		goto out_kfree_skb;
>  	data = nlmsg_data(nlh);
> @@ -876,7 +868,6 @@ static int audit_send_reply_thread(void *arg)
>  static void audit_send_reply(struct sk_buff *request_skb, int seq, int type, int done,
>  			     int multi, const void *payload, int size)
>  {
> -	u32 portid = NETLINK_CB(request_skb).portid;
>  	struct net *net = sock_net(NETLINK_CB(request_skb).sk);
>  	struct sk_buff *skb;
>  	struct task_struct *tsk;
> @@ -886,12 +877,12 @@ static void audit_send_reply(struct sk_buff *request_skb, int seq, int type, int
>  	if (!reply)
>  		return;
>  
> -	skb = audit_make_reply(portid, seq, type, done, multi, payload, size);
> +	skb = audit_make_reply(seq, type, done, multi, payload, size);
>  	if (!skb)
>  		goto out;
>  
>  	reply->net = get_net(net);
> -	reply->portid = portid;
> +	reply->portid = NETLINK_CB(request_skb).portid;
>  	reply->skb = skb;
>  
>  	tsk = kthread_run(audit_send_reply_thread, reply, "audit_send_reply");
> @@ -1075,7 +1066,7 @@ static int audit_replace(pid_t pid)
>  {
>  	struct sk_buff *skb;
>  
> -	skb = audit_make_reply(0, 0, AUDIT_REPLACE, 0, 0, &pid, sizeof(pid));
> +	skb = audit_make_reply(0, AUDIT_REPLACE, 0, 0, &pid, sizeof(pid));
>  	if (!skb)
>  		return -ENOMEM;
>  	return auditd_send_unicast_skb(skb);
> @@ -1245,7 +1236,6 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
>  					size--;
>  				audit_log_n_untrustedstring(ab, data, size);
>  			}
> -			audit_set_portid(ab, NETLINK_CB(skb).portid);
>  			audit_log_end(ab);
>  		}
>  		break;
> @@ -1259,8 +1249,7 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
>  			audit_log_end(ab);
>  			return -EPERM;
>  		}
> -		err = audit_rule_change(msg_type, NETLINK_CB(skb).portid,
> -					   seq, data, nlmsg_len(nlh));
> +		err = audit_rule_change(msg_type, seq, data, nlmsg_len(nlh));
>  		break;
>  	case AUDIT_LIST_RULES:
>  		err = audit_list_rules_send(skb, seq);
> diff --git a/kernel/audit.h b/kernel/audit.h
> index 0f1cf6d1878a..c21b74dd7ff2 100644
> --- a/kernel/audit.h
> +++ b/kernel/audit.h
> @@ -237,8 +237,7 @@ extern int audit_uid_comparator(kuid_t left, u32 op, kuid_t right);
>  extern int audit_gid_comparator(kgid_t left, u32 op, kgid_t right);
>  extern int parent_len(const char *path);
>  extern int audit_compare_dname_path(const char *dname, const char *path, int plen);
> -extern struct sk_buff *audit_make_reply(__u32 portid, int seq, int type,
> -					int done, int multi,
> +extern struct sk_buff *audit_make_reply(int seq, int type, int done, int multi,
>  					const void *payload, int size);
>  extern void		    audit_panic(const char *message);
>  
> diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
> index 880519d6cf2a..81cdf8d8f319 100644
> --- a/kernel/auditfilter.c
> +++ b/kernel/auditfilter.c
> @@ -1033,7 +1033,7 @@ int audit_del_rule(struct audit_entry *entry)
>  }
>  
>  /* List rules using struct audit_rule_data. */
> -static void audit_list_rules(__u32 portid, int seq, struct sk_buff_head *q)
> +static void audit_list_rules(int seq, struct sk_buff_head *q)
>  {
>  	struct sk_buff *skb;
>  	struct audit_krule *r;
> @@ -1048,15 +1048,15 @@ static void audit_list_rules(__u32 portid, int seq, struct sk_buff_head *q)
>  			data = audit_krule_to_data(r);
>  			if (unlikely(!data))
>  				break;
> -			skb = audit_make_reply(portid, seq, AUDIT_LIST_RULES,
> -					       0, 1, data,
> +			skb = audit_make_reply(seq, AUDIT_LIST_RULES, 0, 1,
> +					       data,
>  					       sizeof(*data) + data->buflen);
>  			if (skb)
>  				skb_queue_tail(q, skb);
>  			kfree(data);
>  		}
>  	}
> -	skb = audit_make_reply(portid, seq, AUDIT_LIST_RULES, 1, 1, NULL, 0);
> +	skb = audit_make_reply(seq, AUDIT_LIST_RULES, 1, 1, NULL, 0);
>  	if (skb)
>  		skb_queue_tail(q, skb);
>  }
> @@ -1085,13 +1085,11 @@ static void audit_log_rule_change(char *action, struct audit_krule *rule, int re
>  /**
>   * audit_rule_change - apply all rules to the specified message type
>   * @type: audit message type
> - * @portid: target port id for netlink audit messages
>   * @seq: netlink audit message sequence (serial) number
>   * @data: payload data
>   * @datasz: size of payload data
>   */
> -int audit_rule_change(int type, __u32 portid, int seq, void *data,
> -			size_t datasz)
> +int audit_rule_change(int type, int seq, void *data, size_t datasz)
>  {
>  	int err = 0;
>  	struct audit_entry *entry;
> @@ -1150,7 +1148,7 @@ int audit_list_rules_send(struct sk_buff *request_skb, int seq)
>  	skb_queue_head_init(&dest->q);
>  
>  	mutex_lock(&audit_filter_mutex);
> -	audit_list_rules(portid, seq, &dest->q);
> +	audit_list_rules(seq, &dest->q);
>  	mutex_unlock(&audit_filter_mutex);
>  
>  	tsk = kthread_run(audit_send_list, dest, "audit_send_list");
> 
> --
> Linux-audit mailing list
> Linux-audit@redhat.com
> https://www.redhat.com/mailman/listinfo/linux-audit

- RGB

--
Richard Guy Briggs <rgb@redhat.com>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635