[PATCH net 0/3] amt: fix several bugs

[PATCH net 0/3] amt: fix several bugs

[Taehee Yoo]

This patchset fixes several bugs in amt module

First patch fixes typo.

Second patch fixes wrong return value of amt_update_handler().
A relay finds a tunnel if it receives an update message from the gateway.
If it can't find a tunnel, amt_update_handler() should return an error,
not success. But it always returns success.

Third patch fixes a possible memory leak in amt_rcv().
A skb would not be freed if an amt interface doesn't have a socket.

Taehee Yoo (3):
  amt: fix typo in amt
  amt: fix return value of amt_update_handler()
  amt: fix possible memory leak in amt_rcv()

 drivers/net/amt.c | 6 +++---
 include/net/amt.h | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

-- 
2.17.1

[PATCH net 1/3] amt: fix typo in amt

[Taehee Yoo]

AMT_MSG_TEARDOWM is defined,
But it should be AMT_MSG_TEARDOWN.

Fixes: b9022b53adad ("amt: add control plane of amt interface")
Signed-off-by: Taehee Yoo <ap420073@gmail.com>
---
 drivers/net/amt.c | 2 +-
 include/net/amt.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/amt.c b/drivers/net/amt.c
index de4ea518c793..f41668ddd94a 100644
--- a/drivers/net/amt.c
+++ b/drivers/net/amt.c
@@ -57,7 +57,7 @@ static char *type_str[] = {
 	"AMT_MSG_MEMBERSHIP_QUERY",
 	"AMT_MSG_MEMBERSHIP_UPDATE",
 	"AMT_MSG_MULTICAST_DATA",
-	"AMT_MSG_TEARDOWM",
+	"AMT_MSG_TEARDOWN",
 };
 
 static char *action_str[] = {
diff --git a/include/net/amt.h b/include/net/amt.h
index 7a4db8b903ee..0e40c3d64fcf 100644
--- a/include/net/amt.h
+++ b/include/net/amt.h
@@ -15,7 +15,7 @@ enum amt_msg_type {
 	AMT_MSG_MEMBERSHIP_QUERY,
 	AMT_MSG_MEMBERSHIP_UPDATE,
 	AMT_MSG_MULTICAST_DATA,
-	AMT_MSG_TEARDOWM,
+	AMT_MSG_TEARDOWN,
 	__AMT_MSG_MAX,
 };
 
-- 
2.17.1

[PATCH net 2/3] amt: fix return value of amt_update_handler()

[Taehee Yoo]

If a relay receives an update message, it lookup a tunnel.
and if there is no tunnel for that message, it should be treated
as an error, not a success.
But amt_update_handler() returns false, which means success.

Fixes: cbc21dc1cfe9 ("amt: add data plane of amt interface")
Signed-off-by: Taehee Yoo <ap420073@gmail.com>
---
 drivers/net/amt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/amt.c b/drivers/net/amt.c
index f41668ddd94a..635de07b2e40 100644
--- a/drivers/net/amt.c
+++ b/drivers/net/amt.c
@@ -2423,7 +2423,7 @@ static bool amt_update_handler(struct amt_dev *amt, struct sk_buff *skb)
 		}
 	}
 
-	return false;
+	return true;
 
 report:
 	iph = ip_hdr(skb);
-- 
2.17.1

[PATCH net 3/3] amt: fix possible memory leak in amt_rcv()

[Taehee Yoo]

If an amt receives packets and it finds socket.
If it can't find a socket, it should free a received skb.
But it doesn't.
So, a memory leak would possibly occur.

Fixes: cbc21dc1cfe9 ("amt: add data plane of amt interface")
Signed-off-by: Taehee Yoo <ap420073@gmail.com>
---
 drivers/net/amt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/amt.c b/drivers/net/amt.c
index 635de07b2e40..ebee5f07a208 100644
--- a/drivers/net/amt.c
+++ b/drivers/net/amt.c
@@ -2679,7 +2679,7 @@ static int amt_rcv(struct sock *sk, struct sk_buff *skb)
 	amt = rcu_dereference_sk_user_data(sk);
 	if (!amt) {
 		err = true;
-		goto out;
+		goto drop;
 	}
 
 	skb->dev = amt->dev;
-- 
2.17.1

Re: [PATCH net 0/3] amt: fix several bugs

[Jakub Kicinski]

On Mon, 23 May 2022 16:17:05 +0000 Taehee Yoo wrote:
> This patchset fixes several bugs in amt module
> 
> First patch fixes typo.
> 
> Second patch fixes wrong return value of amt_update_handler().
> A relay finds a tunnel if it receives an update message from the gateway.
> If it can't find a tunnel, amt_update_handler() should return an error,
> not success. But it always returns success.
> 
> Third patch fixes a possible memory leak in amt_rcv().
> A skb would not be freed if an amt interface doesn't have a socket.

Please double check you're not missing pskb_may_pull() calls.
E.g. in amt_update_handler()? There's more.

Re: [PATCH net 0/3] amt: fix several bugs

[patchwork-bot+netdevbpf]

Hello:

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

On Mon, 23 May 2022 16:17:05 +0000 you wrote:
> This patchset fixes several bugs in amt module
> 
> First patch fixes typo.
> 
> Second patch fixes wrong return value of amt_update_handler().
> A relay finds a tunnel if it receives an update message from the gateway.
> If it can't find a tunnel, amt_update_handler() should return an error,
> not success. But it always returns success.
> 
> [...]

Here is the summary with links:
  - [net,1/3] amt: fix typo in amt
    https://git.kernel.org/netdev/net/c/4934609dda03
  - [net,2/3] amt: fix return value of amt_update_handler()
    https://git.kernel.org/netdev/net/c/ac1dbf55981b
  - [net,3/3] amt: fix possible memory leak in amt_rcv()
    https://git.kernel.org/netdev/net/c/1a1a0e80e005

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

Re: [PATCH net 0/3] amt: fix several bugs

[Taehee Yoo]

On 5/26/22 13:47, Jakub Kicinski wrote:

Hi Jakub,
Thanks a lot for your review!

 > On Mon, 23 May 2022 16:17:05 +0000 Taehee Yoo wrote:
 >> This patchset fixes several bugs in amt module
 >>
 >> First patch fixes typo.
 >>
 >> Second patch fixes wrong return value of amt_update_handler().
 >> A relay finds a tunnel if it receives an update message from the 
gateway.
 >> If it can't find a tunnel, amt_update_handler() should return an error,
 >> not success. But it always returns success.
 >>
 >> Third patch fixes a possible memory leak in amt_rcv().
 >> A skb would not be freed if an amt interface doesn't have a socket.
 >
 > Please double check you're not missing pskb_may_pull() calls.
 > E.g. in amt_update_handler()? There's more.

As you pointed, I found that I missed pskb_may_pull() calls in 
amt_multicast_data_handler() and amt_update_handler().
So, I will do some more checks and then send a patch.

Thank you so much!
Taehee Yoo