kernel: audit.c: is it a memleak of net in function audit_send_reply

[Paul Moore]

On Mon, Apr 20, 2020 at 10:24 AM Paul Moore <paul at paul-moore.com> wrote:
>
> On Mon, Apr 20, 2020 at 3:54 AM 亿一 <teroincn at gmail.com> wrote:
> >
> > Hi, all:
> >
> > when reviewing code in function audit_send_reply, I notice that if
> > kthread_run return failure, the net reference would not be released
> > because reply has been kfree directly.
>
> Thanks for reporting this.
>
> Looking at the code, it's a little worse than that.  If kthread_run()
> fails then audit_send_reply() will return early, holding both a
> reference to @net as well as leaking @reply.

EDIT: Ignore the above, reply should get cleaned up ... I was
mis-reading "!ERR()" as "ERR".

> Let me finish getting through my mail and I'll put together a quick
> patch to resolve this (I'm seeing a few other related things we should
> fix in audit_send_reply()).
>
> > 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);
> >
> >     if (!reply)
> >         return;
> >
> >     skb = audit_make_reply(seq, type, done, multi, payload, size);
> >     if (!skb)
> >         goto out;
> >
> >     reply->net = get_net(net);     // hold a reference of net here
> >     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);      // kfree reply without release the net reference.
> > }
>
>
>
> --
> paul moore
> www.paul-moore.com



-- 
paul moore
www.paul-moore.com