netdev - Re: [PATCH net-next 2/6] neigh: fix a possible leak issue of neigh entry

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20150515.113950.1408734283430265055.davem@davemloft.net>
Date:	Fri, 15 May 2015 11:39:50 -0400 (EDT)
From:	David Miller <davem@...emloft.net>
To:	eric.dumazet@...il.com
Cc:	ying.xue@...driver.com, netdev@...r.kernel.org,
	alexei@...estorage.com, joern@...estorage.com, ja@....bg
Subject: Re: [PATCH net-next 2/6] neigh: fix a possible leak issue of neigh
 entry

From: Eric Dumazet <eric.dumazet@...il.com>
Date: Fri, 15 May 2015 05:12:42 -0700

> On Fri, 2015-05-15 at 14:55 +0800, Ying Xue wrote:
>> Once modifying a pending timer of a neighbour, it's insufficient to
>> post a warning message. Instead we should not take the neighbour's
>> reference count at the same time, otherwise, it causes an issue that
>> the neighbour cannot be freed forever.
>> 
>> Signed-off-by: Ying Xue <ying.xue@...driver.com>
>> ---
>>  net/core/neighbour.c |    9 +++++----
>>  1 file changed, 5 insertions(+), 4 deletions(-)
>> 
>> diff --git a/net/core/neighbour.c b/net/core/neighbour.c
>> index 3de6542..5595db3 100644
>> --- a/net/core/neighbour.c
>> +++ b/net/core/neighbour.c
>> @@ -164,10 +164,11 @@ static int neigh_forced_gc(struct neigh_table *tbl)
>>  
>>  static void neigh_add_timer(struct neighbour *n, unsigned long when)
>>  {
>> -	neigh_hold(n);
>> -	if (unlikely(mod_timer(&n->timer, when))) {
>> -		printk("NEIGH: BUG, double timer add, state is %x\n",
>> -		       n->nud_state);
>> +	if (likely(!mod_timer(&n->timer, when))) {
>> +		neigh_hold(n);
>> +	} else {
>> +		pr_warn("NEIGH: BUG, double timer add, state is %x\n",
>> +			n->nud_state);
>>  		dump_stack();
>>  	}
>>  }
> 
> 
> NACK

Indeed, major NACK.  And you've been told this change is unacceptable
multiple times already, and you've been told exactly why as well.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html