On 10/20/2018 12:51 AM, Daniel Borkmann wrote:
> On 10/18/2018 10:58 PM, John Fastabend wrote:
>> We check if the sk_user_data (the psock in skmsg) is in fact a sockmap
>> type to late, after we read the refcnt which is an error. This
>> series moves the check up before reading refcnt and also adds a test
>> to test_maps to test trying to add a KCM socket into a sockmap.
>>
>> While reviewig this code I also found an issue with KCM and kTLS
>> where each uses sk_data_ready hooks and associated stream parser
>> breaking expectations in kcm, ktls or both. But that fix will need
>> to go to net.
>>
>> Thanks to Eric for reporting.
>>
>> v2: Fix up file +/- my scripts lost track of them
>> v3: return EBUSY if refcnt is zero
>>
>> John Fastabend (2):
>>   bpf: skmsg, fix psock create on existing kcm/tls port
>>   bpf: test_maps add a test to catch kcm + sockmap
>>
>>  include/linux/skmsg.h                     | 25 +++++++++---
>>  net/core/sock_map.c                       | 11 +++---
>>  tools/testing/selftests/bpf/Makefile      |  2 +-
>>  tools/testing/selftests/bpf/sockmap_kcm.c | 14 +++++++
>>  tools/testing/selftests/bpf/test_maps.c   | 64 
>> ++++++++++++++++++++++++++++++-
>>  5 files changed, 103 insertions(+), 13 deletions(-)
>>  create mode 100644 tools/testing/selftests/bpf/sockmap_kcm.c
> 
> Applied, thanks!

Fyi, I've only applied patch 1/2 for now to get the bug fixed. The patch 2/2 
throws
a bunch of warnings that look like the below. Also, I think we leak kcm socket 
in
error paths and once we're done with testing, so would be good to close it once
unneeded. Please respin the test as a stand-alone commit, thanks:

[...]
bpf-next/tools/testing/selftests/bpf/libbpf.a -lcap -lelf -lrt -lpthread -o 
/home/darkstar/trees/bpf-next-ok/tools/testing/selftests/bpf/test_maps
test_maps.c: In function ‘test_sockmap’:
test_maps.c:869:0: warning: "AF_KCM" redefined
 #define AF_KCM 41

In file included from /usr/include/sys/socket.h:38:0,
                 from test_maps.c:21:
/usr/include/bits/socket.h:133:0: note: this is the location of the previous 
definition
 #define AF_KCM  PF_KCM

Reply via email to