Re: [PATCH net-next] sctp: avoid list_del_init if it's freeing the memory right away

[David Miller <davem@davemloft.net>]

From: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Date: Tue, 7 Feb 2017 17:45:09 -0200

> On Tue, Feb 07, 2017 at 02:21:21PM -0500, David Miller wrote:
>> From: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
>> Date: Tue,  7 Feb 2017 17:03:21 -0200
>> 
>> > There is no reason to use list_del_init() in these places as we are
>> > going to free/destroy the memory in a few lines below.
>> > 
>> > Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
>> > ---
>> >  net/sctp/associola.c     | 14 ++++----------
>> >  net/sctp/auth.c          |  8 ++------
>> >  net/sctp/chunk.c         |  4 ++--
>> >  net/sctp/outqueue.c      | 14 +++++++-------
>> >  net/sctp/sm_make_chunk.c |  3 +--
>> >  5 files changed, 16 insertions(+), 27 deletions(-)
>> > 
>> > diff --git a/net/sctp/associola.c b/net/sctp/associola.c
>> > index e50dc6d7543fd6acfa7442f3a9ee575203c7718d..7eb9dacfa53a438b20a34319cf01c6c9a591f0c3 100644
>> > --- a/net/sctp/associola.c
>> > +++ b/net/sctp/associola.c
>> > @@ -1638,25 +1638,19 @@ int sctp_assoc_set_id(struct sctp_association *asoc, gfp_t gfp)
>> >  static void sctp_assoc_free_asconf_queue(struct sctp_association *asoc)
>> >  {
>> >  	struct sctp_chunk *asconf;
>> > -	struct sctp_chunk *tmp;
>> >  
>> > -	list_for_each_entry_safe(asconf, tmp, &asoc->addip_chunk_list, list) {
>> > -		list_del_init(&asconf->list);
>> > +	list_for_each_entry(asconf, &asoc->addip_chunk_list, list)
>> >  		sctp_chunk_free(asconf);
>> > -	}
>> >  }
>> 
>> This leave freed memory on the asoc->addip_chunk_list, in fact why aren't you seeing
> 
> This should be alright, because here we are purging the entire list and
> the asoc will also be free right after.

I understand that contextually this should be true.  And you can
probably prove to me that nobody can access this list at this point
in the code.

But, this certainly is not defensive programming, that's for sure.