* [PATCH] audit: fix a net reference leak in audit_send_reply()
@ 2020-04-20 20:04 Paul Moore
2020-04-20 22:27 ` Richard Guy Briggs
0 siblings, 1 reply; 3+ messages in thread
From: Paul Moore @ 2020-04-20 20:04 UTC (permalink / raw)
To: linux-audit; +Cc: teroincn
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.
Reported-by: teroincn@gmail.com
Signed-off-by: Paul Moore <paul@paul-moore.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@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] audit: fix a net reference leak in audit_send_reply()
2020-04-20 20:04 [PATCH] audit: fix a net reference leak in audit_send_reply() Paul Moore
@ 2020-04-20 22:27 ` Richard Guy Briggs
2020-04-20 23:35 ` Paul Moore
0 siblings, 1 reply; 3+ messages in thread
From: Richard Guy Briggs @ 2020-04-20 22:27 UTC (permalink / raw)
To: Paul Moore; +Cc: linux-audit, teroincn
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@gmail.com
> Signed-off-by: Paul Moore <paul@paul-moore.com>
Reviewed-by: Richard Guy Briggs <rgb@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@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
--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] audit: fix a net reference leak in audit_send_reply()
2020-04-20 22:27 ` Richard Guy Briggs
@ 2020-04-20 23:35 ` Paul Moore
0 siblings, 0 replies; 3+ messages in thread
From: Paul Moore @ 2020-04-20 23:35 UTC (permalink / raw)
To: Richard Guy Briggs; +Cc: linux-audit, teroincn
On Mon, Apr 20, 2020 at 6:27 PM Richard Guy Briggs <rgb@redhat.com> wrote:
> 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@gmail.com
> > Signed-off-by: Paul Moore <paul@paul-moore.com>
>
> Reviewed-by: Richard Guy Briggs <rgb@redhat.com>
Thanks. I just merged this into audit/next.
--
paul moore
www.paul-moore.com
--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-04-20 23:36 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-20 20:04 [PATCH] audit: fix a net reference leak in audit_send_reply() Paul Moore
2020-04-20 22:27 ` Richard Guy Briggs
2020-04-20 23:35 ` Paul Moore
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).