Fixes for raw diag sockets handling
Subsystems (2)
cover Fixes for raw diag sockets handling
Hi! Here are a few fixes for raw-diag sockets handling: missing sock_put call and jump for exiting from nested cycle. I made patches for iproute2 as well so will send them out soon.
Also I have a question about sockets lookup not for raw diag only (though I didn't modify lookup procedure) but in general: the structure inet_diag_req_v2 has inet_diag_sockid::idiag_if member which supposed to carry interface index from userspace request.
Then for example in INET_MATCH (include/net/inet_hashtables.h), the __dif parameter (which is @idiag_if) compared with @sk_bound_dev_if *iif* the sk_bound_dev_if has been ever set. Thus if say someone looks up for paticular device with specified index if the rest of parameters match and SO_BINDTODEVICE never been called for this device we return the socket even if idiag_if is not zero. Is it supposed to be so? Or I miss something obvious?
I mean this snippet
(!(__sk)->sk_bound_dev_if || \ ((__sk)->sk_bound_dev_if == (__dif))) && \
when someone calls for destory sockets on particular interface and @__dif != 0 the match may return socket where sk_bound_dev_if = 0 instead of completely matching one. Isn't it?
Cyrill
Quoted 16 lines · Cyrill Gorcunov
Also I have a question about sockets lookup not for raw diag only (though I didn't modify lookup procedure) but in general: the structure
inet_diag_req_v2hasinet_diag_sockid::idiag_ifmember which supposed to carry interface index from userspace request.Then for example in
INET_MATCH(include/net/inet_hashtables.h), the__difparameter (which is @idiag_if) compared with @sk_bound_dev_if *iif* thesk_bound_dev_ifhas been ever set. Thus if say someone looks up for paticular device with specified index if the rest of parameters match andSO_BINDTODEVICEnever been called for this device we return the socket even ifidiag_ifis not zero. Is it supposed to be so? Or I miss something obvious?I mean this snippet
Quoted 6 lines
(!(__sk)->sk_bound_dev_if || \ ((__sk)->sk_bound_dev_if == (__dif))) && \when someone calls for destory sockets on particular interface and @__dif != 0 the match may return socket where
sk_bound_dev_if= 0 instead of completely matching one. Isn't it?
yes. I recently added an exact_dif to the lookup for listener sockets (see compute_score). Something like that could be added to INET_MATCH.
Quoted 5 lines · David Ahern
Quoted 2 lines
@__dif != 0 the match may return socket where
sk_bound_dev_if= 0 instead of completely matching one. Isn't it?yes. I recently added an
exact_difto the lookup for listener sockets (seecompute_score). Something like that could be added toINET_MATCH.
Seem so. I need to revisit this moment. Because with current lookup code iproute2 patches I made and been testing do not kill all sockets bound to particular device in one pass (because request from userspace asks for index 15 in my case but kernel return one with index 0). At first I thought I made a mistake in userspace code but once I added printk's into kernel I found that here some strange results over lookup.
Cyrill
Quoted 7 lines · Cyrill Gorcunov
Quoted 5 lines · David Ahern
Quoted 2 lines
@__dif != 0 the match may return socket where
sk_bound_dev_if= 0 instead of completely matching one. Isn't it?yes. I recently added an
exact_difto the lookup for listener sockets (seecompute_score). Something like that could be added toINET_MATCH.
Quoted 6 lines
Seem so. I need to revisit this moment. Because with current lookup code iproute2 patches I made and been testing do not kill all sockets bound to particular device in one pass (because request from userspace asks for index 15 in my case but kernel return one with index 0). At first I thought I made a mistake in userspace code but once I added printk's into kernel I found that here some strange results over lookup.
Limited to raw sockets or are you looking at multiple spec options (dev, address, port)?
I have not seen issues with tcp or udp. Running:
ss -aK 'dev == red'
drops all sockets bound to device 'red' (or at least signaling the socket failure for the app to handle):
root@jessie4:~# ss -ap 'dev == red'
Netid State Recv-Q Send-Q Local Address:Port Peer Address:Port
udp UNCONN 0 0 *%red:12345 *:* users:(("vrf-test",pid=765,fd=3))
tcp LISTEN 0 1 *%red:12345 *:* users:(("vrf-test",pid=766,fd=3))
tcp ESTAB 0 0 10.100.1.4%red:ssh 10.100.1.254:60298 users:(("sshd",pid=738,fd=3))root@jessie4:~# ss -aKp 'dev == red'
Netid State Recv-Q Send-Q Local Address:Port Peer Address:Port
udp UNCONN 0 0 *%red:12345 *:* users:(("vrf-test",pid=765,fd=3))
tcp LISTEN 0 1 *%red:12345 *:* users:(("vrf-test",pid=766,fd=3))
tcp ESTAB 0 0 10.100.1.4%red:ssh 10.100.1.254:60298 users:(("sshd",pid=738,fd=3))root@jessie4:~# ss -ap 'dev == red' Netid State Recv-Q Send-Q Local Address:Port Peer Address:Port
Quoted 8 lines · David Ahern
Limited to raw sockets or are you looking at multiple spec options (dev, address, port)?
I have not seen issues with tcp or udp. Running:
ss -aK 'dev == red'drops all sockets bound to device 'red' (or at least signaling the socket failure for the app to handle):
Limited to raw socket. I didn't modify lookup kernel code but use already existing helpers. The tcp/udp sockets do use port value in lookup (iirc, don't have code under my hand at moment), in turn raw lookup uses only net,raw-protocol, src/dst and device index. In my test case the sokets were unconnected so the have no address but bound to device and I hit mismatch. Then looking into inet matching code I found this weird snippet I posted previously.
Quoted 7 lines
root@jessie4:~# ss -ap 'dev == red'
Netid State Recv-Q Send-Q Local Address:Port Peer Address:Port
udp UNCONN 0 0 *%red:12345 *:* users:(("vrf-test",pid=765,fd=3))
tcp LISTEN 0 1 *%red:12345 *:* users:(("vrf-test",pid=766,fd=3))
tcp ESTAB 0 0 10.100.1.4%red:ssh 10.100.1.254:60298 users:(("sshd",pid=738,fd=3))Quoted 8 lines
root@jessie4:~# ss -aKp 'dev == red' Netid State Recv-Q Send-Q Local Address:Port Peer Address:Port udp UNCONN 0 0 *%red:12345 *:* users:(("vrf-test",pid=765,fd=3)) tcp LISTEN 0 1 *%red:12345 *:* users:(("vrf-test",pid=766,fd=3)) tcp ESTAB 0 0 10.100.1.4%red:ssh 10.100.1.254:60298 users:(("sshd",pid=738,fd=3))root@jessie4:~# ss -ap 'dev == red' Netid State Recv-Q Send-Q Local Address:Port Peer Address:Port
Cyrill
David Miller
maintainerFrom: Cyrill Gorcunov <[email protected]>
Date: Wed, 02 Nov 2016 15:36:30 +0300
Quoted 3 lines
Hi! Here are a few fixes for raw-diag sockets handling: missing
sock_putcall and jump for exiting from nested cycle. I made patches for iproute2 as well so will send them out soon.
Series applied, thanks.