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: Mon, 4 Jul 2022 02:55:08 +0200 From: Francois Romieu <romieu@...zoreil.com> To: Heiner Kallweit <hkallweit1@...il.com> Cc: Jakub Kicinski <kuba@...nel.org>, David Miller <davem@...emloft.net>, Realtek linux nic maintainers <nic_swsd@...ltek.com>, "netdev@...r.kernel.org" <netdev@...r.kernel.org>, "Erhard F." <erhard_f@...lbox.org> Subject: Re: [PATCH net] r8169: fix accessing unset transport header Heiner Kallweit <hkallweit1@...il.com> : > 66e4c8d95008 ("net: warn if transport header was not set") added > a check that triggers a warning in r8169, see [0]. > > [0] https://bugzilla.kernel.org/show_bug.cgi?id=216157 > > Fixes: 8d520b4de3ed ("r8169: work around RTL8125 UDP hw bug") > Reported-by: Erhard F. <erhard_f@...lbox.org> > Tested-by: Erhard F. <erhard_f@...lbox.org> > Signed-off-by: Heiner Kallweit <hkallweit1@...il.com> /me wonders... - bz216157 experiences a (2nd) warning because the rtl8169_tso_csum_v2 ARP path shares the factored read of the (unset) transport offset but said ARP path does not use the transport offset. -> ok, the warning is mostly harmless. - rtl8169_tso_csum_v2 non-ARP paths own WARN_ON_ONCE will always complain before Eric's transport specific warning triggers. -> ok, the warning is redundant. - rtl8169_features_check > diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c > index 3098d6672..1b7fdb4f0 100644 > --- a/drivers/net/ethernet/realtek/r8169_main.c > +++ b/drivers/net/ethernet/realtek/r8169_main.c [...] > @@ -4402,14 +4401,13 @@ static netdev_features_t rtl8169_features_check(struct sk_buff *skb, > struct net_device *dev, > netdev_features_t features) > { > - int transport_offset = skb_transport_offset(skb); > struct rtl8169_private *tp = netdev_priv(dev); > > if (skb_is_gso(skb)) { > if (tp->mac_version == RTL_GIGA_MAC_VER_34) > features = rtl8168evl_fix_tso(skb, features); > > - if (transport_offset > GTTCPHO_MAX && > + if (skb_transport_offset(skb) > GTTCPHO_MAX && > rtl_chip_supports_csum_v2(tp)) > features &= ~NETIF_F_ALL_TSO; > } else if (skb->ip_summed == CHECKSUM_PARTIAL) { > @@ -4420,7 +4418,7 @@ static netdev_features_t rtl8169_features_check(struct sk_buff *skb, > if (rtl_quirk_packet_padto(tp, skb)) > features &= ~NETIF_F_CSUM_MASK; > > - if (transport_offset > TCPHO_MAX && > + if (skb_transport_offset(skb) > TCPHO_MAX && > rtl_chip_supports_csum_v2(tp)) > features &= ~NETIF_F_CSUM_MASK; > } Neither skb_is_gso nor CHECKSUM_PARTIAL implies a transport header so the warning may still trigger, right ? Btw it's a bit unexpected to see a "Fixes" tag related to a RTL8125 bug as well as a "Tested-by" by the bugzilla submitter when the dmesg included in bz216157 exibits a RTL8168e/8111e. -- Ueimor
Powered by blists - more mailing lists