diff mbox series

[mptcp] mptcp: remove tcp ulp setsockopt support

Message ID 20211205192700.25396-1-fw@strlen.de (mailing list archive)
State Awaiting Upstream
Delegated to: Netdev Maintainers
Headers show
Series [mptcp] mptcp: remove tcp ulp setsockopt support | expand

Checks

Context Check Description
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix warning Target tree name not specified in the subject
netdev/cover_letter success Single patches do not need cover letters
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 2 this patch: 2
netdev/cc_maintainers warning 4 maintainers not CCed: mathew.j.martineau@linux.intel.com matthieu.baerts@tessares.net kuba@kernel.org davem@davemloft.net
netdev/build_clang success Errors and warnings before: 20 this patch: 20
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 4 this patch: 4
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 7 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
netdev/tree_selection success Guessing tree name failed - patch did not apply

Commit Message

Florian Westphal Dec. 5, 2021, 7:27 p.m. UTC
TCP_ULP setsockopt cannot be used for mptcp because its already
used internally to plumb subflow (tcp) sockets to the mptcp layer.

syzbot managed to trigger a crash for mptcp connections that are
in fallback mode:

KASAN: null-ptr-deref in range [0x0000000000000020-0x0000000000000027]
CPU: 1 PID: 1083 Comm: syz-executor.3 Not tainted 5.16.0-rc2-syzkaller #0
RIP: 0010:tls_build_proto net/tls/tls_main.c:776 [inline]
[..]
 __tcp_set_ulp net/ipv4/tcp_ulp.c:139 [inline]
 tcp_set_ulp+0x428/0x4c0 net/ipv4/tcp_ulp.c:160
 do_tcp_setsockopt+0x455/0x37c0 net/ipv4/tcp.c:3391
 mptcp_setsockopt+0x1b47/0x2400 net/mptcp/sockopt.c:638

Remove support for TCP_ULP setsockopt.

Reported-by: syzbot+1fd9b69cde42967d1add@syzkaller.appspotmail.com
Signed-off-by: Florian Westphal <fw@strlen.de>
---

Comments

Jakub Kicinski Dec. 6, 2021, 3:53 p.m. UTC | #1
On Sun,  5 Dec 2021 20:27:00 +0100 Florian Westphal wrote:
> TCP_ULP setsockopt cannot be used for mptcp because its already
> used internally to plumb subflow (tcp) sockets to the mptcp layer.
> 
> syzbot managed to trigger a crash for mptcp connections that are
> in fallback mode:

Fallback mode meaning ops are NULL? I'm slightly confused by this
report.

> KASAN: null-ptr-deref in range [0x0000000000000020-0x0000000000000027]
> CPU: 1 PID: 1083 Comm: syz-executor.3 Not tainted 5.16.0-rc2-syzkaller #0
> RIP: 0010:tls_build_proto net/tls/tls_main.c:776 [inline]
> [..]
>  __tcp_set_ulp net/ipv4/tcp_ulp.c:139 [inline]
>  tcp_set_ulp+0x428/0x4c0 net/ipv4/tcp_ulp.c:160
>  do_tcp_setsockopt+0x455/0x37c0 net/ipv4/tcp.c:3391
>  mptcp_setsockopt+0x1b47/0x2400 net/mptcp/sockopt.c:638
> 
> Remove support for TCP_ULP setsockopt.
> 
> Reported-by: syzbot+1fd9b69cde42967d1add@syzkaller.appspotmail.com
> Signed-off-by: Florian Westphal <fw@strlen.de>
Jakub Kicinski Dec. 6, 2021, 3:55 p.m. UTC | #2
On Mon, 6 Dec 2021 07:53:26 -0800 Jakub Kicinski wrote:
> On Sun,  5 Dec 2021 20:27:00 +0100 Florian Westphal wrote:
> > TCP_ULP setsockopt cannot be used for mptcp because its already
> > used internally to plumb subflow (tcp) sockets to the mptcp layer.
> > 
> > syzbot managed to trigger a crash for mptcp connections that are
> > in fallback mode:  
> 
> Fallback mode meaning ops are NULL? I'm slightly confused by this
> report.

Ah, it's the socket not the ops.

> > KASAN: null-ptr-deref in range [0x0000000000000020-0x0000000000000027]
> > CPU: 1 PID: 1083 Comm: syz-executor.3 Not tainted 5.16.0-rc2-syzkaller #0
> > RIP: 0010:tls_build_proto net/tls/tls_main.c:776 [inline]
> > [..]
> >  __tcp_set_ulp net/ipv4/tcp_ulp.c:139 [inline]
> >  tcp_set_ulp+0x428/0x4c0 net/ipv4/tcp_ulp.c:160
> >  do_tcp_setsockopt+0x455/0x37c0 net/ipv4/tcp.c:3391
> >  mptcp_setsockopt+0x1b47/0x2400 net/mptcp/sockopt.c:638
> > 
> > Remove support for TCP_ULP setsockopt.
> > 
> > Reported-by: syzbot+1fd9b69cde42967d1add@syzkaller.appspotmail.com
> > Signed-off-by: Florian Westphal <fw@strlen.de>
Mat Martineau Dec. 6, 2021, 6:44 p.m. UTC | #3
On Mon, 6 Dec 2021, Jakub Kicinski wrote:

> On Mon, 6 Dec 2021 07:53:26 -0800 Jakub Kicinski wrote:
>> On Sun,  5 Dec 2021 20:27:00 +0100 Florian Westphal wrote:
>>> TCP_ULP setsockopt cannot be used for mptcp because its already
>>> used internally to plumb subflow (tcp) sockets to the mptcp layer.
>>>
>>> syzbot managed to trigger a crash for mptcp connections that are
>>> in fallback mode:
>>
>> Fallback mode meaning ops are NULL? I'm slightly confused by this
>> report.
>
> Ah, it's the socket not the ops.
>
>>> KASAN: null-ptr-deref in range [0x0000000000000020-0x0000000000000027]
>>> CPU: 1 PID: 1083 Comm: syz-executor.3 Not tainted 5.16.0-rc2-syzkaller #0
>>> RIP: 0010:tls_build_proto net/tls/tls_main.c:776 [inline]
>>> [..]
>>>  __tcp_set_ulp net/ipv4/tcp_ulp.c:139 [inline]
>>>  tcp_set_ulp+0x428/0x4c0 net/ipv4/tcp_ulp.c:160
>>>  do_tcp_setsockopt+0x455/0x37c0 net/ipv4/tcp.c:3391
>>>  mptcp_setsockopt+0x1b47/0x2400 net/mptcp/sockopt.c:638
>>>
>>> Remove support for TCP_ULP setsockopt.
>>>
>>> Reported-by: syzbot+1fd9b69cde42967d1add@syzkaller.appspotmail.com
>>> Signed-off-by: Florian Westphal <fw@strlen.de>

Jakub -

If you could mark this as "Not Applicable" in netdevbpf patchwork, we'll 
apply it to the mptcp tree and resubmit to netdev with some related 
patches.

Thanks,

--
Mat Martineau
Intel
Jakub Kicinski Dec. 6, 2021, 6:57 p.m. UTC | #4
On Mon, 6 Dec 2021 10:44:16 -0800 (PST) Mat Martineau wrote:
> If you could mark this as "Not Applicable" in netdevbpf patchwork, we'll 
> apply it to the mptcp tree and resubmit to netdev with some related 
> patches.

For sure, I was asking because technically I added the code that hit
the oops to tls a week or so ago, so I wanted to make sure it's not 
my fault ;) Marked in pw appropriately now.
diff mbox series

Patch

diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c
index 3c3db22fd36a..aa3fcd86dbe2 100644
--- a/net/mptcp/sockopt.c
+++ b/net/mptcp/sockopt.c
@@ -543,7 +543,6 @@  static bool mptcp_supported_sockopt(int level, int optname)
 		case TCP_NODELAY:
 		case TCP_THIN_LINEAR_TIMEOUTS:
 		case TCP_CONGESTION:
-		case TCP_ULP:
 		case TCP_CORK:
 		case TCP_KEEPIDLE:
 		case TCP_KEEPINTVL: