lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Tue, 14 Jan 2020 09:10:33 +0100 From: Martin Schiller <ms@....tdt.de> To: Jakub Kicinski <kubakici@...pl> Cc: khc@...waw.pl, davem@...emloft.net, linux-x25@...r.kernel.org, netdev@...r.kernel.org, linux-kernel@...r.kernel.org Subject: Re: [PATCH 2/2] wan/hdlc_x25: fix skb handling On 2020-01-13 14:44, Jakub Kicinski wrote: > On Mon, 13 Jan 2020 13:45:51 +0100, Martin Schiller wrote: >> o call skb_reset_network_header() before hdlc->xmit() >> o change skb proto to HDLC (0x0019) before hdlc->xmit() >> o call dev_queue_xmit_nit() before hdlc->xmit() >> >> This changes make it possible to trace (tcpdump) outgoing layer2 >> (ETH_P_HDLC) packets >> >> o use a copy of the skb for lapb_data_request() in x25_xmit() > > It's not clear to me why Well, this patch is ported form an older environment which is based on linux-3.4. I can't reproduce the misbehavior with actual version, so I will drop this part of the patch. > >> This fixes the problem, that tracing layer3 (ETH_P_X25) packets >> results in a malformed first byte of the packets. >> >> Signed-off-by: Martin Schiller <ms@....tdt.de> >> --- >> drivers/net/wan/hdlc_x25.c | 15 +++++++++++---- >> 1 file changed, 11 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/net/wan/hdlc_x25.c b/drivers/net/wan/hdlc_x25.c >> index b28051eba736..434e5263eddf 100644 >> --- a/drivers/net/wan/hdlc_x25.c >> +++ b/drivers/net/wan/hdlc_x25.c >> @@ -72,6 +72,7 @@ static int x25_data_indication(struct net_device >> *dev, struct sk_buff *skb) >> unsigned char *ptr; >> >> skb_push(skb, 1); >> + skb_reset_network_header(skb); >> >> if (skb_cow(skb, 1)) > > This skb_cow() here is for the next handler down to have a 1 byte of > headroom guaranteed? It'd seem more natural to have skb_cow before the > push.. not that it's related to your patch. Thanks for the hint. I will move the skb_cow() before the skb_push(). > >> return NET_RX_DROP; >> @@ -88,6 +89,9 @@ static int x25_data_indication(struct net_device >> *dev, struct sk_buff *skb) >> static void x25_data_transmit(struct net_device *dev, struct sk_buff >> *skb) >> { >> hdlc_device *hdlc = dev_to_hdlc(dev); > > Please insert a new line after the variable declaration since you're > touching this one. OK, will do. > >> + skb_reset_network_header(skb); >> + skb->protocol = hdlc_type_trans(skb, dev); I will also insert an "if (dev_nit_active(dev))" here. >> + dev_queue_xmit_nit(skb, dev); >> hdlc->xmit(skb, dev); /* Ignore return value :-( */ >> } >>
Powered by blists - more mailing lists