| Message ID | 1539896316-13403-1-git-send-email-john.fastabend@gmail.com |
|---|---|
| Headers | show |
| Series | Fix kcm + sockmap by checking psock type | expand |
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!
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
On 10/19/2018 03:57 PM, Daniel Borkmann wrote: > 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: > Thanks, I didn't see the warnings below locally but will look into spinning a good version tonight with the closing sock fix as well. John > [...] > 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 >