From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755099AbcK3Eoe (ORCPT ); Tue, 29 Nov 2016 23:44:34 -0500 Received: from mx1.redhat.com ([209.132.183.28]:52560 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753639AbcK3EoZ (ORCPT ); Tue, 29 Nov 2016 23:44:25 -0500 Date: Tue, 29 Nov 2016 23:44:22 -0500 From: Richard Guy Briggs To: Florian Westphal Cc: linux-audit@redhat.com, linux-kernel@vger.kernel.org Subject: Re: [PATCH] audit: remove the audit freelist Message-ID: <20161130044422.GH6897@madcap2.tricolour.ca> References: <1479215774-29810-1-git-send-email-fw@strlen.de> <20161129161233.GG6897@madcap2.tricolour.ca> <20161129172450.GD26930@breakpoint.cc> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20161129172450.GD26930@breakpoint.cc> User-Agent: Mutt/1.5.21 (2010-09-15) X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.38]); Wed, 30 Nov 2016 04:44:25 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2016-11-29 18:24, Florian Westphal wrote: > Richard Guy Briggs 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 -- Richard Guy Briggs Kernel Security Engineering, Base Operating Systems, Red Hat Remote, Ottawa, Canada Voice: +1.647.777.2635, Internal: (81) 32635