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]
Date:   Mon, 1 Aug 2022 17:44:16 +0200
From:   "Denis V. Lunev" <>
To:     David Ahern <>,
        Alexander Mikhalitsyn <>,
Cc:     "David S. Miller" <>,
        Eric Dumazet <>,
        Jakub Kicinski <>,
        Paolo Abeni <>,
        Daniel Borkmann <>,
        Yajun Deng <>,
        Roopa Prabhu <>,,
        Alexey Kuznetsov <>,
        Konstantin Khorenko <>,
Subject: Re: [PATCH 1/2] neigh: fix possible DoS due to net iface start/stop

On 01.08.2022 17:08, David Ahern wrote:
> On 7/29/22 4:35 AM, Alexander Mikhalitsyn wrote:
>> The patch proposed doing very simple thing. It drops only packets from
> it does 2 things - adds a namespace check and a performance based change
> with the way the list is walked.
I believe no. All items are removed before the patch and
all items are walked after the patch, similar O(n) operation.

>> the same namespace in the pneigh_queue_purge() where network interface
>> state change is detected. This is enough to prevent the problem for the
>> whole node preserving original semantics of the code.
>> diff --git a/net/core/neighbour.c b/net/core/neighbour.c
>> index 54625287ee5b..213ec0be800b 100644
>> --- a/net/core/neighbour.c
>> +++ b/net/core/neighbour.c
>> @@ -386,8 +396,7 @@ static int __neigh_ifdown(struct neigh_table *tbl, struct net_device *dev,
>>   	neigh_flush_dev(tbl, dev, skip_perm);
>>   	pneigh_ifdown_and_unlock(tbl, dev);
>> -	del_timer_sync(&tbl->proxy_timer);
> why are you removing this line too?

Because we have packets in the queue after the purge and they have to be 
May be we should better do

if (skb_queue_empty(&tbl->proxy_queue))

after the purge. That could be more correct.

>> -	pneigh_queue_purge(&tbl->proxy_queue);
>> +	pneigh_queue_purge(&tbl->proxy_queue, dev_net(dev));
>>   	return 0;
>>   }

Powered by blists - more mailing lists