lkml.org 
[lkml]   [2016]   [Nov]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] audit: remove the audit freelist
On 2016-11-29 18:24, Florian Westphal wrote:
> Richard Guy Briggs <rgb@redhat.com> wrote:
> > > static void audit_buffer_free(struct audit_buffer *ab)
> > > {
> > > - unsigned long flags;
> > > -
> > > if (!ab)
> > > return;
> > >
> > > kfree_skb(ab->skb);
> > > - spin_lock_irqsave(&audit_freelist_lock, flags);
> > > - if (audit_freelist_count > AUDIT_MAXFREE)
> > > - kfree(ab);
> > > - else {
> > > - audit_freelist_count++;
> > > - list_add(&ab->list, &audit_freelist);
> > > - }
> > > - spin_unlock_irqrestore(&audit_freelist_lock, flags);
> > > + kfree(ab);
> > > }
> [..]
>
> > > nlh = nlmsg_put(ab->skb, 0, 0, type, 0, 0);
> > > if (!nlh)
> > > - goto out_kfree_skb;
> > > + goto err;
> > >
> > > return ab;
> > >
> > > -out_kfree_skb:
> > > - kfree_skb(ab->skb);
> > > - ab->skb = NULL;
> >
> > Why is the kfree_skb() skipped on error from nlmsg_put()? I don't see
> > much risk in nlmsg_put() failing considering the very simple arguments,
> > however the code path is not trivial either.
>
> if nlmsg_put fails we jump to err and ...
>
> > > err:
> > > audit_buffer_free(ab);
> > > return NULL;
>
> ... ab->skb gets free'd by audit_buffer_free() here.

Duh, thank you! It was already redundant in plain sight in your patch.
Sorry for the brain fart. :)

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

- RGB

--
Richard Guy Briggs <rgb@redhat.com>
Kernel Security Engineering, Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635

\
 
 \ /
  Last update: 2016-11-30 05:44    [W:0.820 / U:0.980 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site