From: Greg Kroah-Hartman Date: Sat, 7 Nov 2020 15:03:08 +0000 (+0100) Subject: 4.9-stable patches X-Git-Tag: v4.4.242~42 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=978bcab8512c877f6923dc8dfa63dcc30bd44bf7;p=thirdparty%2Fkernel%2Fstable-queue.git 4.9-stable patches added patches: gianfar-account-for-tx-ptp-timestamp-in-the-skb-headroom.patch gianfar-replace-skb_realloc_headroom-with-skb_cow_head-for-ptp.patch --- diff --git a/queue-4.9/gianfar-account-for-tx-ptp-timestamp-in-the-skb-headroom.patch b/queue-4.9/gianfar-account-for-tx-ptp-timestamp-in-the-skb-headroom.patch new file mode 100644 index 00000000000..d842473afd2 --- /dev/null +++ b/queue-4.9/gianfar-account-for-tx-ptp-timestamp-in-the-skb-headroom.patch @@ -0,0 +1,55 @@ +From foo@baz Sat Nov 7 04:02:18 PM CET 2020 +From: Claudiu Manoil +Date: Tue, 20 Oct 2020 20:36:05 +0300 +Subject: gianfar: Account for Tx PTP timestamp in the skb headroom + +From: Claudiu Manoil + +[ Upstream commit d6a076d68c6b5d6a5800f3990a513facb7016dea ] + +When PTP timestamping is enabled on Tx, the controller +inserts the Tx timestamp at the beginning of the frame +buffer, between SFD and the L2 frame header. This means +that the skb provided by the stack is required to have +enough headroom otherwise a new skb needs to be created +by the driver to accommodate the timestamp inserted by h/w. +Up until now the driver was relying on the second option, +using skb_realloc_headroom() to create a new skb to accommodate +PTP frames. Turns out that this method is not reliable, as +reallocation of skbs for PTP frames along with the required +overhead (skb_set_owner_w, consume_skb) is causing random +crashes in subsequent skb_*() calls, when multiple concurrent +TCP streams are run at the same time on the same device +(as seen in James' report). +Note that these crashes don't occur with a single TCP stream, +nor with multiple concurrent UDP streams, but only when multiple +TCP streams are run concurrently with the PTP packet flow +(doing skb reallocation). +This patch enforces the first method, by requesting enough +headroom from the stack to accommodate PTP frames, and so avoiding +skb_realloc_headroom() & co, and the crashes no longer occur. +There's no reason not to set needed_headroom to a large enough +value to accommodate PTP frames, so in this regard this patch +is a fix. + +Reported-by: James Jurack +Fixes: bee9e58c9e98 ("gianfar:don't add FCB length to hard_header_len") +Signed-off-by: Claudiu Manoil +Link: https://lore.kernel.org/r/20201020173605.1173-1-claudiu.manoil@nxp.com +Signed-off-by: Jakub Kicinski +Signed-off-by: Greg Kroah-Hartman +--- + drivers/net/ethernet/freescale/gianfar.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +--- a/drivers/net/ethernet/freescale/gianfar.c ++++ b/drivers/net/ethernet/freescale/gianfar.c +@@ -1385,7 +1385,7 @@ static int gfar_probe(struct platform_de + + if (dev->features & NETIF_F_IP_CSUM || + priv->device_flags & FSL_GIANFAR_DEV_HAS_TIMER) +- dev->needed_headroom = GMAC_FCB_LEN; ++ dev->needed_headroom = GMAC_FCB_LEN + GMAC_TXPAL_LEN; + + /* Initializing some of the rx/tx queue level parameters */ + for (i = 0; i < priv->num_tx_queues; i++) { diff --git a/queue-4.9/gianfar-replace-skb_realloc_headroom-with-skb_cow_head-for-ptp.patch b/queue-4.9/gianfar-replace-skb_realloc_headroom-with-skb_cow_head-for-ptp.patch new file mode 100644 index 00000000000..e755cecd21b --- /dev/null +++ b/queue-4.9/gianfar-replace-skb_realloc_headroom-with-skb_cow_head-for-ptp.patch @@ -0,0 +1,70 @@ +From foo@baz Sat Nov 7 04:02:18 PM CET 2020 +From: Claudiu Manoil +Date: Thu, 29 Oct 2020 10:10:56 +0200 +Subject: gianfar: Replace skb_realloc_headroom with skb_cow_head for PTP + +From: Claudiu Manoil + +[ Upstream commit d145c9031325fed963a887851d9fa42516efd52b ] + +When PTP timestamping is enabled on Tx, the controller +inserts the Tx timestamp at the beginning of the frame +buffer, between SFD and the L2 frame header. This means +that the skb provided by the stack is required to have +enough headroom otherwise a new skb needs to be created +by the driver to accommodate the timestamp inserted by h/w. +Up until now the driver was relying on skb_realloc_headroom() +to create new skbs to accommodate PTP frames. Turns out that +this method is not reliable in this context at least, as +skb_realloc_headroom() for PTP frames can cause random crashes, +mostly in subsequent skb_*() calls, when multiple concurrent +TCP streams are run at the same time with the PTP flow +on the same device (as seen in James' report). I also noticed +that when the system is loaded by sending multiple TCP streams, +the driver receives cloned skbs in large numbers. +skb_cow_head() instead proves to be stable in this scenario, +and not only handles cloned skbs too but it's also more efficient +and widely used in other drivers. +The commit introducing skb_realloc_headroom in the driver +goes back to 2009, commit 93c1285c5d92 +("gianfar: reallocate skb when headroom is not enough for fcb"). +For practical purposes I'm referencing a newer commit (from 2012) +that brings the code to its current structure (and fixes the PTP +case). + +Fixes: 9c4886e5e63b ("gianfar: Fix invalid TX frames returned on error queue when time stamping") +Reported-by: James Jurack +Suggested-by: Jakub Kicinski +Signed-off-by: Claudiu Manoil +Link: https://lore.kernel.org/r/20201029081057.8506-1-claudiu.manoil@nxp.com +Signed-off-by: Jakub Kicinski +Signed-off-by: Greg Kroah-Hartman +--- + drivers/net/ethernet/freescale/gianfar.c | 12 ++---------- + 1 file changed, 2 insertions(+), 10 deletions(-) + +--- a/drivers/net/ethernet/freescale/gianfar.c ++++ b/drivers/net/ethernet/freescale/gianfar.c +@@ -2367,20 +2367,12 @@ static int gfar_start_xmit(struct sk_buf + fcb_len = GMAC_FCB_LEN + GMAC_TXPAL_LEN; + + /* make space for additional header when fcb is needed */ +- if (fcb_len && unlikely(skb_headroom(skb) < fcb_len)) { +- struct sk_buff *skb_new; +- +- skb_new = skb_realloc_headroom(skb, fcb_len); +- if (!skb_new) { ++ if (fcb_len) { ++ if (unlikely(skb_cow_head(skb, fcb_len))) { + dev->stats.tx_errors++; + dev_kfree_skb_any(skb); + return NETDEV_TX_OK; + } +- +- if (skb->sk) +- skb_set_owner_w(skb_new, skb->sk); +- dev_consume_skb_any(skb); +- skb = skb_new; + } + + /* total number of fragments in the SKB */ diff --git a/queue-4.9/series b/queue-4.9/series index 5cb66beab0e..fe3e8b4555a 100644 --- a/queue-4.9/series +++ b/queue-4.9/series @@ -91,3 +91,5 @@ staging-octeon-repair-fixed-link-support.patch staging-octeon-drop-on-uncorrectable-alignment-or-fcs-error.patch xen-events-don-t-use-chip_data-for-legacy-irqs.patch tipc-fix-use-after-free-in-tipc_bcast_get_mode.patch +gianfar-replace-skb_realloc_headroom-with-skb_cow_head-for-ptp.patch +gianfar-account-for-tx-ptp-timestamp-in-the-skb-headroom.patch