netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next 0/3] Small fixes for MCTP
@ 2022-02-25  5:39 Matt Johnston
  2022-02-25  5:39 ` [PATCH net-next 1/3] mctp: Avoid warning if unregister notifies twice Matt Johnston
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Matt Johnston @ 2022-02-25  5:39 UTC (permalink / raw)
  To: netdev; +Cc: David S. Miller, Jakub Kicinski, Jeremy Kerr

Hi,

This series has 3 fixes for MCTP.

Cheers,
Matt

Matt Johnston (3):
  mctp: Avoid warning if unregister notifies twice
  mctp i2c: Fix potential use-after-free
  mctp i2c: Fix hard head TX bounds length check

 drivers/net/mctp/mctp-i2c.c | 7 ++++---
 net/mctp/device.c           | 8 ++++----
 2 files changed, 8 insertions(+), 7 deletions(-)

-- 
2.32.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH net-next 1/3] mctp: Avoid warning if unregister notifies twice
  2022-02-25  5:39 [PATCH net-next 0/3] Small fixes for MCTP Matt Johnston
@ 2022-02-25  5:39 ` Matt Johnston
  2022-02-25  5:39 ` [PATCH net-next 2/3] mctp i2c: Fix potential use-after-free Matt Johnston
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Matt Johnston @ 2022-02-25  5:39 UTC (permalink / raw)
  To: netdev; +Cc: David S. Miller, Jakub Kicinski, Jeremy Kerr

Previously if an unregister notify handler ran twice (waiting for
netdev to be released) it would print a warning in mctp_unregister()
every subsequent time the unregister notify occured.

Instead we only need to worry about the case where a mctp_ptr is
set on an unknown device type.

Signed-off-by: Matt Johnston <matt@codeconstruct.com.au>
---
 net/mctp/device.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/mctp/device.c b/net/mctp/device.c
index da13444c632b..f49be882e98e 100644
--- a/net/mctp/device.c
+++ b/net/mctp/device.c
@@ -428,10 +428,10 @@ static void mctp_unregister(struct net_device *dev)
 	struct mctp_dev *mdev;
 
 	mdev = mctp_dev_get_rtnl(dev);
-	if (mctp_known(dev) != (bool)mdev) {
+	if (mdev && !mctp_known(dev)) {
 		// Sanity check, should match what was set in mctp_register
-		netdev_warn(dev, "%s: mdev pointer %d but type (%d) match is %d",
-			    __func__, (bool)mdev, mctp_known(dev), dev->type);
+		netdev_warn(dev, "%s: BUG mctp_ptr set for unknown type %d",
+			    __func__, dev->type);
 		return;
 	}
 	if (!mdev)
@@ -455,7 +455,7 @@ static int mctp_register(struct net_device *dev)
 
 	if (mdev) {
 		if (!mctp_known(dev))
-			netdev_warn(dev, "%s: mctp_dev set for unknown type %d",
+			netdev_warn(dev, "%s: BUG mctp_ptr set for unknown type %d",
 				    __func__, dev->type);
 		return 0;
 	}
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH net-next 2/3] mctp i2c: Fix potential use-after-free
  2022-02-25  5:39 [PATCH net-next 0/3] Small fixes for MCTP Matt Johnston
  2022-02-25  5:39 ` [PATCH net-next 1/3] mctp: Avoid warning if unregister notifies twice Matt Johnston
@ 2022-02-25  5:39 ` Matt Johnston
  2022-02-25  5:39 ` [PATCH net-next 3/3] mctp i2c: Fix hard head TX bounds length check Matt Johnston
  2022-02-26  6:30 ` [PATCH net-next 0/3] Small fixes for MCTP patchwork-bot+netdevbpf
  3 siblings, 0 replies; 5+ messages in thread
From: Matt Johnston @ 2022-02-25  5:39 UTC (permalink / raw)
  To: netdev; +Cc: David S. Miller, Jakub Kicinski, Jeremy Kerr, Dan Carpenter

The skb is handed off to netif_rx() which may free it.
Found by Smatch.

Reported-By: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Matt Johnston <matt@codeconstruct.com.au>
---
 drivers/net/mctp/mctp-i2c.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/mctp/mctp-i2c.c b/drivers/net/mctp/mctp-i2c.c
index 365c3dfd4034..470682c88d7e 100644
--- a/drivers/net/mctp/mctp-i2c.c
+++ b/drivers/net/mctp/mctp-i2c.c
@@ -338,7 +338,7 @@ static int mctp_i2c_recv(struct mctp_i2c_dev *midev)
 
 	if (status == NET_RX_SUCCESS) {
 		ndev->stats.rx_packets++;
-		ndev->stats.rx_bytes += skb->len;
+		ndev->stats.rx_bytes += recvlen;
 	} else {
 		ndev->stats.rx_dropped++;
 	}
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH net-next 3/3] mctp i2c: Fix hard head TX bounds length check
  2022-02-25  5:39 [PATCH net-next 0/3] Small fixes for MCTP Matt Johnston
  2022-02-25  5:39 ` [PATCH net-next 1/3] mctp: Avoid warning if unregister notifies twice Matt Johnston
  2022-02-25  5:39 ` [PATCH net-next 2/3] mctp i2c: Fix potential use-after-free Matt Johnston
@ 2022-02-25  5:39 ` Matt Johnston
  2022-02-26  6:30 ` [PATCH net-next 0/3] Small fixes for MCTP patchwork-bot+netdevbpf
  3 siblings, 0 replies; 5+ messages in thread
From: Matt Johnston @ 2022-02-25  5:39 UTC (permalink / raw)
  To: netdev; +Cc: David S. Miller, Jakub Kicinski, Jeremy Kerr

We should be testing the length before fitting into the u8 byte_count.
This is just a sanity check, the MCTP stack should have limited to MTU
which is checked, and we check consistency later in mctp_i2c_xmit().

Found by Smatch
mctp_i2c_header_create() warn: impossible condition
    '(hdr->byte_count > 255) => (0-255 > 255)'

Signed-off-by: Matt Johnston <matt@codeconstruct.com.au>
---
 drivers/net/mctp/mctp-i2c.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/mctp/mctp-i2c.c b/drivers/net/mctp/mctp-i2c.c
index 470682c88d7e..baf7afac7857 100644
--- a/drivers/net/mctp/mctp-i2c.c
+++ b/drivers/net/mctp/mctp-i2c.c
@@ -537,6 +537,9 @@ static int mctp_i2c_header_create(struct sk_buff *skb, struct net_device *dev,
 	struct mctp_hdr *mhdr;
 	u8 lldst, llsrc;
 
+	if (len > MCTP_I2C_MAXMTU)
+		return -EMSGSIZE;
+
 	lldst = *((u8 *)daddr);
 	llsrc = *((u8 *)saddr);
 
@@ -547,8 +550,6 @@ static int mctp_i2c_header_create(struct sk_buff *skb, struct net_device *dev,
 	hdr->dest_slave = (lldst << 1) & 0xff;
 	hdr->command = MCTP_I2C_COMMANDCODE;
 	hdr->byte_count = len + 1;
-	if (hdr->byte_count > MCTP_I2C_MAXBLOCK)
-		return -EMSGSIZE;
 	hdr->source_slave = ((llsrc << 1) & 0xff) | 0x01;
 	mhdr->ver = 0x01;
 
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH net-next 0/3] Small fixes for MCTP
  2022-02-25  5:39 [PATCH net-next 0/3] Small fixes for MCTP Matt Johnston
                   ` (2 preceding siblings ...)
  2022-02-25  5:39 ` [PATCH net-next 3/3] mctp i2c: Fix hard head TX bounds length check Matt Johnston
@ 2022-02-26  6:30 ` patchwork-bot+netdevbpf
  3 siblings, 0 replies; 5+ messages in thread
From: patchwork-bot+netdevbpf @ 2022-02-26  6:30 UTC (permalink / raw)
  To: Matt Johnston; +Cc: netdev, davem, kuba, jk

Hello:

This series was applied to netdev/net-next.git (master)
by Jakub Kicinski <kuba@kernel.org>:

On Fri, 25 Feb 2022 13:39:35 +0800 you wrote:
> Hi,
> 
> This series has 3 fixes for MCTP.
> 
> Cheers,
> Matt
> 
> [...]

Here is the summary with links:
  - [net-next,1/3] mctp: Avoid warning if unregister notifies twice
    https://git.kernel.org/netdev/net-next/c/f62457df5cc4
  - [net-next,2/3] mctp i2c: Fix potential use-after-free
    https://git.kernel.org/netdev/net-next/c/06bf1ce69d55
  - [net-next,3/3] mctp i2c: Fix hard head TX bounds length check
    https://git.kernel.org/netdev/net-next/c/33f5d1a9d970

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2022-02-26  6:30 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-25  5:39 [PATCH net-next 0/3] Small fixes for MCTP Matt Johnston
2022-02-25  5:39 ` [PATCH net-next 1/3] mctp: Avoid warning if unregister notifies twice Matt Johnston
2022-02-25  5:39 ` [PATCH net-next 2/3] mctp i2c: Fix potential use-after-free Matt Johnston
2022-02-25  5:39 ` [PATCH net-next 3/3] mctp i2c: Fix hard head TX bounds length check Matt Johnston
2022-02-26  6:30 ` [PATCH net-next 0/3] Small fixes for MCTP patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).