On 2016-11-29 18:24, Florian Westphal wrote:
> Richard Guy Briggs <r...@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 <r...@redhat.com>
- RGB
--
Richard Guy Briggs <r...@redhat.com>
Kernel Security Engineering, Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635
