[PATCH AUTOSEL 5.17 08/12] net: af_key: check encryption module availability consistency

[PATCH AUTOSEL 5.17 08/12] net: af_key: check encryption module availability consistency

[Sasha Levin]

From: Thomas Bartschies <thomas.bartschies@cvk.de>

[ Upstream commit 015c44d7bff3f44d569716117becd570c179ca32 ]

Since the recent introduction supporting the SM3 and SM4 hash algos for IPsec, the kernel
produces invalid pfkey acquire messages, when these encryption modules are disabled. This
happens because the availability of the algos wasn't checked in all necessary functions.
This patch adds these checks.

Signed-off-by: Thomas Bartschies <thomas.bartschies@cvk.de>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 net/key/af_key.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/key/af_key.c b/net/key/af_key.c
index fd51db3be91c..fbb2c16693ad 100644
--- a/net/key/af_key.c
+++ b/net/key/af_key.c
@@ -2898,7 +2898,7 @@ static int count_ah_combs(const struct xfrm_tmpl *t)
 			break;
 		if (!aalg->pfkey_supported)
 			continue;
-		if (aalg_tmpl_set(t, aalg))
+		if (aalg_tmpl_set(t, aalg) && aalg->available)
 			sz += sizeof(struct sadb_comb);
 	}
 	return sz + sizeof(struct sadb_prop);
@@ -2916,7 +2916,7 @@ static int count_esp_combs(const struct xfrm_tmpl *t)
 		if (!ealg->pfkey_supported)
 			continue;
 
-		if (!(ealg_tmpl_set(t, ealg)))
+		if (!(ealg_tmpl_set(t, ealg) && ealg->available))
 			continue;
 
 		for (k = 1; ; k++) {
@@ -2927,7 +2927,7 @@ static int count_esp_combs(const struct xfrm_tmpl *t)
 			if (!aalg->pfkey_supported)
 				continue;
 
-			if (aalg_tmpl_set(t, aalg))
+			if (aalg_tmpl_set(t, aalg) && aalg->available)
 				sz += sizeof(struct sadb_comb);
 		}
 	}
-- 
2.35.1

Re: [PATCH AUTOSEL 5.17 08/12] net: af_key: check encryption module availability consistency

[Jakub Kicinski]

On Tue, 24 May 2022 11:59:22 -0400 Sasha Levin wrote:
> From: Thomas Bartschies <thomas.bartschies@cvk.de>
> 
> [ Upstream commit 015c44d7bff3f44d569716117becd570c179ca32 ]
> 
> Since the recent introduction supporting the SM3 and SM4 hash algos for IPsec, the kernel
> produces invalid pfkey acquire messages, when these encryption modules are disabled. This
> happens because the availability of the algos wasn't checked in all necessary functions.
> This patch adds these checks.
> 
> Signed-off-by: Thomas Bartschies <thomas.bartschies@cvk.de>
> Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
> Signed-off-by: Sasha Levin <sashal@kernel.org>

I don't see anyone else complaining yet so let me step up.

Please drop this, it's getting reverted.

Re: [PATCH AUTOSEL 5.17 08/12] net: af_key: check encryption module availability consistency

[Jakub Kicinski]

On Tue, 24 May 2022 11:09:08 -0700 Jakub Kicinski wrote:
> On Tue, 24 May 2022 11:59:22 -0400 Sasha Levin wrote:
> > From: Thomas Bartschies <thomas.bartschies@cvk.de>
> > 
> > [ Upstream commit 015c44d7bff3f44d569716117becd570c179ca32 ]
> > 
> > Since the recent introduction supporting the SM3 and SM4 hash algos for IPsec, the kernel
> > produces invalid pfkey acquire messages, when these encryption modules are disabled. This
> > happens because the availability of the algos wasn't checked in all necessary functions.
> > This patch adds these checks.
> > 
> > Signed-off-by: Thomas Bartschies <thomas.bartschies@cvk.de>
> > Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
> > Signed-off-by: Sasha Levin <sashal@kernel.org>  
> 
> I don't see anyone else complaining yet so let me step up.
> 
> Please drop this, it's getting reverted.

I take that back, sorry, it's a different patch.
I meant not to backport 4dc2a5a8f6754492180741facf2a8787f2c415d7.

[PATCH AUTOSEL 5.17 09/12] nfc: pn533: Fix buggy cleanup order

[Sasha Levin]

From: Lin Ma <linma@zju.edu.cn>

[ Upstream commit b8cedb7093b2d1394cae9b86494cba4b62d3a30a ]

When removing the pn533 device (i2c or USB), there is a logic error. The
original code first cancels the worker (flush_delayed_work) and then
destroys the workqueue (destroy_workqueue), leaving the timer the last
one to be deleted (del_timer). This result in a possible race condition
in a multi-core preempt-able kernel. That is, if the cleanup
(pn53x_common_clean) is concurrently run with the timer handler
(pn533_listen_mode_timer), the timer can queue the poll_work to the
already destroyed workqueue, causing use-after-free.

This patch reorder the cleanup: it uses the del_timer_sync to make sure
the handler is finished before the routine will destroy the workqueue.
Note that the timer cannot be activated by the worker again.

static void pn533_wq_poll(struct work_struct *work)
...
 rc = pn533_send_poll_frame(dev);
 if (rc)
   return;

 if (cur_mod->len == 0 && dev->poll_mod_count > 1)
   mod_timer(&dev->listen_timer, ...);

That is, the mod_timer can be called only when pn533_send_poll_frame()
returns no error, which is impossible because the device is detaching
and the lower driver should return ENODEV code.

Signed-off-by: Lin Ma <linma@zju.edu.cn>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/nfc/pn533/pn533.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/nfc/pn533/pn533.c b/drivers/nfc/pn533/pn533.c
index a491db46e3bd..d9f6367b9993 100644
--- a/drivers/nfc/pn533/pn533.c
+++ b/drivers/nfc/pn533/pn533.c
@@ -2787,13 +2787,14 @@ void pn53x_common_clean(struct pn533 *priv)
 {
 	struct pn533_cmd *cmd, *n;
 
+	/* delete the timer before cleanup the worker */
+	del_timer_sync(&priv->listen_timer);
+
 	flush_delayed_work(&priv->poll_work);
 	destroy_workqueue(priv->wq);
 
 	skb_queue_purge(&priv->resp_q);
 
-	del_timer(&priv->listen_timer);
-
 	list_for_each_entry_safe(cmd, n, &priv->cmd_queue, queue) {
 		list_del(&cmd->queue);
 		kfree(cmd);
-- 
2.35.1

[PATCH AUTOSEL 5.17 10/12] net: ftgmac100: Disable hardware checksum on AST2600

[Sasha Levin]

From: Joel Stanley <joel@jms.id.au>

[ Upstream commit 6fd45e79e8b93b8d22fb8fe22c32fbad7e9190bd ]

The AST2600 when using the i210 NIC over NC-SI has been observed to
produce incorrect checksum results with specific MTU values. This was
first observed when sending data across a long distance set of networks.

On a local network, the following test was performed using a 1MB file of
random data.

On the receiver run this script:

 #!/bin/bash
 while [ 1 ]; do
        # Zero the stats
        nstat -r  > /dev/null
        nc -l 9899 > test-file
        # Check for checksum errors
        TcpInCsumErrors=$(nstat | grep TcpInCsumErrors)
        if [ -z "$TcpInCsumErrors" ]; then
                echo No TcpInCsumErrors
        else
                echo TcpInCsumErrors = $TcpInCsumErrors
        fi
 done

On an AST2600 system:

 # nc <IP of  receiver host> 9899 < test-file

The test was repeated with various MTU values:

 # ip link set mtu 1410 dev eth0

The observed results:

 1500 - good
 1434 - bad
 1400 - good
 1410 - bad
 1420 - good

The test was repeated after disabling tx checksumming:

 # ethtool -K eth0 tx-checksumming off

And all MTU values tested resulted in transfers without error.

An issue with the driver cannot be ruled out, however there has been no
bug discovered so far.

David has done the work to take the original bug report of slow data
transfer between long distance connections and triaged it down to this
test case.

The vendor suspects this this is a hardware issue when using NC-SI. The
fixes line refers to the patch that introduced AST2600 support.

Reported-by: David Wilder <wilder@us.ibm.com>
Reviewed-by: Dylan Hung <dylan_hung@aspeedtech.com>
Signed-off-by: Joel Stanley <joel@jms.id.au>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/net/ethernet/faraday/ftgmac100.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c
index caf48023f8ea..5231818943c6 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -1928,6 +1928,11 @@ static int ftgmac100_probe(struct platform_device *pdev)
 	/* AST2400  doesn't have working HW checksum generation */
 	if (np && (of_device_is_compatible(np, "aspeed,ast2400-mac")))
 		netdev->hw_features &= ~NETIF_F_HW_CSUM;
+
+	/* AST2600 tx checksum with NCSI is broken */
+	if (priv->use_ncsi && of_device_is_compatible(np, "aspeed,ast2600-mac"))
+		netdev->hw_features &= ~NETIF_F_HW_CSUM;
+
 	if (np && of_get_property(np, "no-hw-checksum", NULL))
 		netdev->hw_features &= ~(NETIF_F_HW_CSUM | NETIF_F_RXCSUM);
 	netdev->features |= netdev->hw_features;
-- 
2.35.1

Re: [PATCH AUTOSEL 5.17 10/12] net: ftgmac100: Disable hardware checksum on AST2600

[Arnd Bergmann]

On Tue, May 24, 2022 at 5:59 PM Sasha Levin <sashal@kernel.org> wrote:
>
>  # ip link set mtu 1410 dev eth0
>
> The observed results:
>
>  1500 - good
>  1434 - bad
>  1400 - good
>  1410 - bad
>  1420 - good

Does it require multiples of four? Maybe it just skips the last bytes?

        Arnd

Re: [PATCH AUTOSEL 5.17 10/12] net: ftgmac100: Disable hardware checksum on AST2600

[Arnd Bergmann]

On Tue, May 24, 2022 at 8:47 PM Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Tue, May 24, 2022 at 5:59 PM Sasha Levin <sashal@kernel.org> wrote:
> >
> >  # ip link set mtu 1410 dev eth0
> >
> > The observed results:
> >
> >  1500 - good
> >  1434 - bad
> >  1400 - good
> >  1410 - bad
> >  1420 - good
>
> Does it require multiples of four? Maybe it just skips the last bytes?

Nevermind, I missed that this was a backport of a merged patch, rather
than a new one.

       Arnd

Re: [PATCH AUTOSEL 5.17 10/12] net: ftgmac100: Disable hardware checksum on AST2600

[Jakub Kicinski]

On Tue, 24 May 2022 20:47:51 +0200 Arnd Bergmann wrote:
> On Tue, May 24, 2022 at 8:47 PM Arnd Bergmann <arnd@arndb.de> wrote:
> >
> > On Tue, May 24, 2022 at 5:59 PM Sasha Levin <sashal@kernel.org> wrote:  
> > >
> > >  # ip link set mtu 1410 dev eth0
> > >
> > > The observed results:
> > >
> > >  1500 - good
> > >  1434 - bad
> > >  1400 - good
> > >  1410 - bad
> > >  1420 - good  
> >
> > Does it require multiples of four? Maybe it just skips the last bytes?  
> 
> Nevermind, I missed that this was a backport of a merged patch, rather
> than a new one.

Great minds think alike, tho ;)

https://lore.kernel.org/all/YowcZUX3lwAA6c5k@lunn.ch/