[PATCH] server: Use SO_BINDTODEVICE in bind_to_index() if possible.

[Paul Gofman]

On 10/12/21 20:58, Zebediah Figura (she/her) wrote:
>
>
> On 10/12/21 11:55, Paul Gofman wrote:
>> Signed-off-by: Paul Gofman <pgofman at codeweavers.com>
>> ---
>>      Using SO_REUSEADDR is currently problematic when more than one 
>> UDP socket is bound
>>      to different interfaces. While that succeeds, it is unspecified 
>> which socket will
>>      receive a packet coming to INADDR_ANY. The filter being 
>> installed for each socket
>>      with SO_ATTACH_FILTER can only reject the packet coming for 
>> another interface but
>>      that rejected packed doesn't arrive to another socket.
>>
>>      SO_BINDTODEVICE seems to do exactly what we want without 
>> installing any socket
>>      filters and without requiring SO_REUSEADDR or SO_REUSEPORT. It 
>> originally considered
>>      for implementing broadcast listen on interface bound sockets but 
>> it wasn't much
>>      useful by that time as the SO_BINDTODEVICE required CAP_NET_RAW 
>> privilege which
>>      is normally not available for a non-root user. However, that 
>> changed since Linux 5.7
>>      [2].
>>
>>      This patch uses SO_BINDTODEVICE but falls back to the previous 
>> way if that fails.
>>
>>      1. 
>> https://www.winehq.org/pipermail/wine-devel/2011-October/092681.html
>>      2. 
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/patch/?id=c427bfec18f2190b8f4718785ee8ed2db4f84ee6
>>
>>   server/sock.c | 28 ++++++++++++++++++++++------
>>   1 file changed, 22 insertions(+), 6 deletions(-)
>>
>> diff --git a/server/sock.c b/server/sock.c
>> index 03716cba90f..af08cd6be24 100644
>> --- a/server/sock.c
>> +++ b/server/sock.c
>> @@ -1756,12 +1756,14 @@ static int accept_into_socket( struct sock 
>> *sock, struct sock *acceptsock )
>>     #ifdef IP_BOUND_IF
>>   -static int bind_to_index( int fd, in_addr_t bind_addr, unsigned 
>> int index )
>> +static int bind_to_index( int fd, in_addr_t bind_addr, const char 
>> *name, unsigned int index, BOOL *need_reuse_addr )
>
> Perhaps we should rename bind_to_index to bind_to_iface or something, 
> then?
>
> Actually, for that matter, should we just pass the name, and call 
> if_nametoindex() only as needed?
>
> Also, do we really need to return a bool? Can we just call 
> SO_REUSEADDR in bind_to_index() instead? That strikes me as cleaner, 
> despite duplicating the call.
>
I thought of that but was tempted to avoid duplication. I will resend 
this way.