On 2020-04-20 16:04, Paul Moore wrote:
 If audit_send_reply() fails when trying to create a new thread to
 send the reply it also fails to cleanup properly, leaking a reference
 to a net structure.  This patch fixes the error path and makes a
 handful of other cleanups that came up while fixing the code. 
Looks good to me.
 Reported-by: teroincn(a)gmail.com
 Signed-off-by: Paul Moore <paul(a)paul-moore.com> 
Reviewed-by: Richard Guy Briggs <rgb(a)redhat.com>
 ---
  kernel/audit.c |   50 +++++++++++++++++++++++++++++---------------------
  1 file changed, 29 insertions(+), 21 deletions(-)
 
 diff --git a/kernel/audit.c b/kernel/audit.c
 index b69c8b460341..66b81358b64f 100644
 --- a/kernel/audit.c
 +++ b/kernel/audit.c
 @@ -924,19 +924,30 @@ struct sk_buff *audit_make_reply(int seq, int type, int done,
  	return NULL;
  }
  
 +static void audit_free_reply(struct audit_reply *reply)
 +{
 +	if (!reply)
 +		return;
 +
 +	if (reply->skb)
 +		kfree_skb(reply->skb);
 +	if (reply->net)
 +		put_net(reply->net);
 +	kfree(reply);
 +}
 +
  static int audit_send_reply_thread(void *arg)
  {
  	struct audit_reply *reply = (struct audit_reply *)arg;
 -	struct sock *sk = audit_get_sk(reply->net);
  
  	audit_ctl_lock();
  	audit_ctl_unlock();
  
  	/* Ignore failure. It'll only happen if the sender goes away,
  	   because our timeout is set to infinite. */
 -	netlink_unicast(sk, reply->skb, reply->portid, 0);
 -	put_net(reply->net);
 -	kfree(reply);
 +	netlink_unicast(audit_get_sk(reply->net), reply->skb, reply->portid, 0);
 +	reply->skb = NULL;
 +	audit_free_reply(reply);
  	return 0;
  }
  
 @@ -950,35 +961,32 @@ static int audit_send_reply_thread(void *arg)
   * @payload: payload data
   * @size: payload size
   *
 - * Allocates an skb, builds the netlink message, and sends it to the port id.
 - * No failure notifications.
 + * Allocates a skb, builds the netlink message, and sends it to the port id.
   */
  static void audit_send_reply(struct sk_buff *request_skb, int seq, int type, int done,
  			     int multi, const void *payload, int size)
  {
 -	struct net *net = sock_net(NETLINK_CB(request_skb).sk);
 -	struct sk_buff *skb;
  	struct task_struct *tsk;
 -	struct audit_reply *reply = kmalloc(sizeof(struct audit_reply),
 -					    GFP_KERNEL);
 +	struct audit_reply *reply;
  
 +	reply = kzalloc(sizeof(*reply), GFP_KERNEL);
  	if (!reply)
  		return;
  
 -	skb = audit_make_reply(seq, type, done, multi, payload, size);
 -	if (!skb)
 -		goto out;
 -
 -	reply->net = get_net(net);
 +	reply->skb = audit_make_reply(seq, type, done, multi, payload, size);
 +	if (!reply->skb)
 +		goto err;
 +	reply->net = get_net(sock_net(NETLINK_CB(request_skb).sk));
  	reply->portid = NETLINK_CB(request_skb).portid;
 -	reply->skb = skb;
  
  	tsk = kthread_run(audit_send_reply_thread, reply, "audit_send_reply");
 -	if (!IS_ERR(tsk))
 -		return;
 -	kfree_skb(skb);
 -out:
 -	kfree(reply);
 +	if (IS_ERR(tsk))
 +		goto err;
 +
 +	return;
 +
 +err:
 +	audit_free_reply(reply);
  }
  
  /*
 
 --
 Linux-audit mailing list
 Linux-audit(a)redhat.com
 
https://www.redhat.com/mailman/listinfo/linux-audit 
- RGB
--
Richard Guy Briggs <rgb(a)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