Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Newer Kernels log error "recv short packet" for every broker packet #160

Closed
kaechele opened this issue Feb 3, 2021 · 7 comments
Closed

Comments

@kaechele
Copy link
Contributor

kaechele commented Feb 3, 2021

This is due to the fact that the control packets we use to communicate with the broker are shorter than 14 bytes (the maximum header size of an L2TP packet).
Recently (I upgraded from 5.9.16 to 5.10.11, haven't had time to pinpoint exactly) the kernel throws a warning in dmesg when a short packet is received leading to lots of spam in dmesg: https://github.com/torvalds/linux/blob/master/net/l2tp/l2tp_core.c#L811 and torvalds/linux@5ee759c

Functionality is not impacted because the error case for this scenario is to forward the packet to user space where the broker is able to pick it up and process it as usual.
The only thing that changed is that the warning is now visible in dmesg.

My first idea for a fix would be to pad our control packets to 14 bytes. Thoughts?

Sample error:

[  630.674133] l2tp_udp_recv_core: 20 callbacks suppressed
[  630.683240] l2tp_core: tunl 100: recv short packet (len=12)
@RalfJung
Copy link
Member

RalfJung commented Feb 3, 2021

My first idea for a fix would be to pad our control packets to 14 bytes. Thoughts?

Yeah, that sounds like the most pragmatic solution. However, if we do this client change, will this be compatible with old brokers (and vice versa)?

idosch pushed a commit to jpirko/linux_mlxsw that referenced this issue Mar 4, 2021
…instead

Commit 5ee759c ("l2tp: use standard API for warning log messages")
changed a number of warnings about invalid packets in the receive path
so that they are always shown, instead of only when a special L2TP debug
flag is set. Even with rate limiting these warnings can easily cause
significant log spam - potentially triggered by a malicious party
sending invalid packets on purpose.

In addition these warnings were noticed by projects like Tunneldigger [1],
which uses L2TP for its data path, but implements its own control
protocol (which is sufficiently different from L2TP data packets that it
would always be passed up to userspace even with future extensions of
L2TP).

Some of the warnings were already redundant, as l2tp_stats has a counter
for these packets. This commit adds one additional counter for invalid
packets that are passed up to userspace. Packets with unknown session are
not counted as invalid, as there is nothing wrong with the format of
these packets.

With the additional counter, all of these messages are either redundant
or benign, so we reduce them to pr_debug_ratelimited().

[1] wlanslovenija/tunneldigger#160

Fixes: 5ee759c ("l2tp: use standard API for warning log messages")
Signed-off-by: Matthias Schiffer <mschiffer@universe-factory.net>
Signed-off-by: David S. Miller <davem@davemloft.net>
@RalfJung
Copy link
Member

RalfJung commented Mar 4, 2021

@NeoRaider that's great, thanks. :)

woodsts pushed a commit to woodsts/linux-stable that referenced this issue Mar 17, 2021
…instead

commit 3e59e88 upstream.

Commit 5ee759c ("l2tp: use standard API for warning log messages")
changed a number of warnings about invalid packets in the receive path
so that they are always shown, instead of only when a special L2TP debug
flag is set. Even with rate limiting these warnings can easily cause
significant log spam - potentially triggered by a malicious party
sending invalid packets on purpose.

In addition these warnings were noticed by projects like Tunneldigger [1],
which uses L2TP for its data path, but implements its own control
protocol (which is sufficiently different from L2TP data packets that it
would always be passed up to userspace even with future extensions of
L2TP).

Some of the warnings were already redundant, as l2tp_stats has a counter
for these packets. This commit adds one additional counter for invalid
packets that are passed up to userspace. Packets with unknown session are
not counted as invalid, as there is nothing wrong with the format of
these packets.

With the additional counter, all of these messages are either redundant
or benign, so we reduce them to pr_debug_ratelimited().

[1] wlanslovenija/tunneldigger#160

Fixes: 5ee759c ("l2tp: use standard API for warning log messages")
Signed-off-by: Matthias Schiffer <mschiffer@universe-factory.net>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
woodsts pushed a commit to woodsts/linux-stable that referenced this issue Mar 17, 2021
…instead

commit 3e59e88 upstream.

Commit 5ee759c ("l2tp: use standard API for warning log messages")
changed a number of warnings about invalid packets in the receive path
so that they are always shown, instead of only when a special L2TP debug
flag is set. Even with rate limiting these warnings can easily cause
significant log spam - potentially triggered by a malicious party
sending invalid packets on purpose.

In addition these warnings were noticed by projects like Tunneldigger [1],
which uses L2TP for its data path, but implements its own control
protocol (which is sufficiently different from L2TP data packets that it
would always be passed up to userspace even with future extensions of
L2TP).

Some of the warnings were already redundant, as l2tp_stats has a counter
for these packets. This commit adds one additional counter for invalid
packets that are passed up to userspace. Packets with unknown session are
not counted as invalid, as there is nothing wrong with the format of
these packets.

With the additional counter, all of these messages are either redundant
or benign, so we reduce them to pr_debug_ratelimited().

[1] wlanslovenija/tunneldigger#160

Fixes: 5ee759c ("l2tp: use standard API for warning log messages")
Signed-off-by: Matthias Schiffer <mschiffer@universe-factory.net>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
@neocturne
Copy link

The patch has been backported to Linux 5.10.24 and 5.11.7.

@kaechele
Copy link
Contributor Author

Confirmed fixed on Fedora 33 with 5.11.7.

it-is-a-robot pushed a commit to openeuler-mirror/kernel that referenced this issue Apr 10, 2021
…instead

stable inclusion
from stable-5.10.24
commit fa5d019c56e78e0b33f585d23149f2553568b998
bugzilla: 51348

--------------------------------

commit 3e59e88 upstream.

Commit 5ee759c ("l2tp: use standard API for warning log messages")
changed a number of warnings about invalid packets in the receive path
so that they are always shown, instead of only when a special L2TP debug
flag is set. Even with rate limiting these warnings can easily cause
significant log spam - potentially triggered by a malicious party
sending invalid packets on purpose.

In addition these warnings were noticed by projects like Tunneldigger [1],
which uses L2TP for its data path, but implements its own control
protocol (which is sufficiently different from L2TP data packets that it
would always be passed up to userspace even with future extensions of
L2TP).

Some of the warnings were already redundant, as l2tp_stats has a counter
for these packets. This commit adds one additional counter for invalid
packets that are passed up to userspace. Packets with unknown session are
not counted as invalid, as there is nothing wrong with the format of
these packets.

With the additional counter, all of these messages are either redundant
or benign, so we reduce them to pr_debug_ratelimited().

[1] wlanslovenija/tunneldigger#160

Fixes: 5ee759c ("l2tp: use standard API for warning log messages")
Signed-off-by: Matthias Schiffer <mschiffer@universe-factory.net>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Chen Jun <chenjun102@huawei.com>
Acked-by:  Weilong Chen <chenweilong@huawei.com>
Signed-off-by: Zheng Zengkai <zhengzengkai@huawei.com>
@mitar
Copy link
Member

mitar commented Oct 27, 2021

What is the status of this?

@kaechele
Copy link
Contributor Author

kaechele commented Nov 3, 2021

It's fixed upstream in the kernel.

@kaechele kaechele closed this as completed Nov 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants