Created attachment 288953 [details]
patch with debug output
Discovered in: linux-4.14.171 (OpenWRT 19.07.2)
Still present in: linux-5.6.10 (judging by code review)
The pppoe code ignores the destination address of incoming PPPOE(D) packets.
Therefore it sometimes acts on a packet belonging to other that own PPP session.
For example it takes a PADT packet meant to other connection and terminates own connection (when it shouldn't).
In practice it means PPPOE connections sporadically disconnect (when a "stray" PADT packet arrives on the ethernet interface).
OpenWRT forum discussion:
"PPPoE disconnects every few hours"
linux-ppp mail list:
"PPPoE Modem hangup after random time - how to debug?"
Here  is Dianne Skoll's (rp-ppoe , on their mail list) take on it:
> What is the angle from the standards point of view? Should/must the
> dst addr be checked?
I would say yes, although the standard does not explicitly spell this out.
It says this:
This packet may be sent anytime after a session is established to
indicate that a PPPoE session has been terminated. It may be sent by
either the Host or the Access Concentrator. The DESTINATION_ADDR
field is a unicast Ethernet address, the CODE field is set to 0xa7
and the SESSION_ID MUST be set to indicate which session is to be
terminated. No TAGs are required.
Since is says that the DESTINATION_ADDR field is a unicast Ethernet
address, I think that's highly suggestive that the receiver of a PADT
should ignore it if the destination address doesn't match its interface
address. I cannot see any rationale for accepting a PADT that is *not*
directed towards our interface, because it may be for a completely
unrelated session. The standard also says [Sec 4]:
The SESSION_ID field is sixteen bits. It is an unsigned value in
network byte order. It's value is defined below for Discovery
packets. The value is fixed for a given PPP session and, in fact,
defines a PPP session along with the Ethernet SOURCE_ADDR and
> IOW: why does the rp-ppoe code do this? (following the standard, or
> fixing a user reported problem, something else?)
I did it that way because the standard says that all three of these
items are needed to define a session: Session ID, SOURCE_ADDR *and*
DESTINATION_ADDR. If the DESTINATION_ADDR does not match, then it's
by definition a different session.
Therefore, IMO, this is a bug in the Linux kernel.
A quick fix (attached, changes only handling of pppoed PADT packets) is to compare the dst addr to the current net device addr and ignore the packet if not matching.
A proper fix would be to record the local ends address (similar as the remote address is recorded, in include/uapi/linux/if_pppox.h struct pppoe_addr.remote) and compare to it for each received packet.
That would also require changes in the user space (ppp.samba.org and possibly others) IINM.
Created attachment 288955 [details]
The same patch but without printk, just the code
note: this is a quickfix, probably not a correct solution
This was fied in May 2020, for kernel versions 4.19, 4.14 (and 4.9, 4.4):
From: Guillaume Nault <firstname.lastname@example.org>
[ Upstream commit b8c158395119be62294da73646a3953c29ac974b ]
We don't want to disconnect a session because of a stray PADT arriving
while the interface is in promiscuous mode.
Furthermore, multicast and broadcast packets make no sense here, so
only PACKET_HOST is accepted.
Reported-by: David Balažic <email@example.com>
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Guillaume Nault <firstname.lastname@example.org>
Signed-off-by: David S. Miller <email@example.com>
Signed-off-by: Greg Kroah-Hartman <firstname.lastname@example.org>
drivers/net/ppp/pppoe.c | 3 +++
1 file changed, 3 insertions(+)
@@ -497,6 +497,9 @@ static int pppoe_disc_rcv(struct sk_buff
+ if (skb->pkt_type != PACKET_HOST)
+ goto abort;
if (!pskb_may_pull(skb, sizeof(struct pppoe_hdr)))