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: [email protected] > Signed-off-by: Paul Moore <[email protected]> Reviewed-by: Richard Guy Briggs <[email protected]> > --- > 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 > [email protected] > https://www.redhat.com/mailman/listinfo/linux-audit - RGB -- Richard Guy Briggs <[email protected]> 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 -- Linux-audit mailing list [email protected] https://www.redhat.com/mailman/listinfo/linux-audit
