netdev - Re: [PATCH net-next v02 1/2] af_packet: allow fanout

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] [day] [month] [year] [list]

Message-ID: <20241009065837.354332-1-gur.stavi@huawei.com>
Date: Wed, 9 Oct 2024 09:58:37 +0300
From: Gur Stavi <gur.stavi@...wei.com>
To: <willemdebruijn.kernel@...il.com>
CC: <davem@...emloft.net>, <edumazet@...gle.com>, <gur.stavi@...wei.com>,
	<kuba@...nel.org>, <linux-kernel@...r.kernel.org>,
	<linux-kselftest@...r.kernel.org>, <netdev@...r.kernel.org>,
	<pabeni@...hat.com>, <shuah@...nel.org>
Subject: Re: [PATCH net-next v02 1/2] af_packet: allow fanout_add when socket is not RUNNING

>> @@ -1846,21 +1846,21 @@ static int fanout_add(struct sock *sk, struct fanout_args *args)
>>  	err = -EINVAL;
>>
>>  	spin_lock(&po->bind_lock);
>> -	if (packet_sock_flag(po, PACKET_SOCK_RUNNING) &&
>> -	    match->type == type &&
>> +	if (match->type == type &&
>>  	    match->prot_hook.type == po->prot_hook.type &&
>>  	    match->prot_hook.dev == po->prot_hook.dev) {
>
> Remaining unaddressed issue is that the socket can now be added
> before being bound. See comment in v1.

I extended the psock_fanout test with unbound fanout test.

As far as I understand, the easiest way to verify bind is to test that
po->prot_hook.dev != NULL, since we are under a bind_lock anyway.
But perhaps a more readable and direct approach to test "bind" would be
to test po->ifindex != -1, as ifindex is commented as "bound device".
However, at the moment ifindex is not initialized to -1, I can add such
initialization, but perhaps I do not fully understand all the logic.

Any preferences?

>
>>  		err = -ENOSPC;
>>  		if (refcount_read(&match->sk_ref) < match->max_num_members) {
>> -			__dev_remove_pack(&po->prot_hook);
>> -
>>  			/* Paired with packet_setsockopt(PACKET_FANOUT_DATA) */
>>  			WRITE_ONCE(po->fanout, match);
>>
>>  			po->rollover = rollover;
>>  			rollover = NULL;
>>  			refcount_set(&match->sk_ref, refcount_read(&match->sk_ref) + 1);
>> -			__fanout_link(sk, po);
>> +			if (packet_sock_flag(po, PACKET_SOCK_RUNNING)) {
>> +				__dev_remove_pack(&po->prot_hook);
>> +				__fanout_link(sk, po);
>> +			}
>>  			err = 0;
>>  		}
>>  	}
> --