[PATCH net-next v3] ethtool: tunnels: check the return value of nla_nest_start()

[PATCH net-next v3] ethtool: tunnels: check the return value of nla_nest_start()

[Li Zhong]

Check the return value of nla_nest_start(). When starting the entry
level nested attributes, if the tailroom of socket buffer is
insufficient to store the attribute header and payload, the return value
will be NULL. It will cause null pointer dereference when entry is used
in nla_nest_end().

Signed-off-by: Li Zhong <floridsleeves@gmail.com>
---
 net/ethtool/tunnels.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/ethtool/tunnels.c b/net/ethtool/tunnels.c
index efde33536687..67fb414ca859 100644
--- a/net/ethtool/tunnels.c
+++ b/net/ethtool/tunnels.c
@@ -136,6 +136,8 @@ ethnl_tunnel_info_fill_reply(const struct ethnl_req_info *req_base,
 			goto err_cancel_table;
 
 		entry = nla_nest_start(skb, ETHTOOL_A_TUNNEL_UDP_TABLE_ENTRY);
+		if (!entry)
+			goto err_cancel_entry;
 
 		if (nla_put_be16(skb, ETHTOOL_A_TUNNEL_UDP_ENTRY_PORT,
 				 htons(IANA_VXLAN_UDP_PORT)) ||
-- 
2.25.1

Re: [PATCH net-next v3] ethtool: tunnels: check the return value of nla_nest_start()

[Jakub Kicinski]

On Wed, 21 Sep 2022 11:17:16 -0700 Li Zhong wrote:
> It will cause null pointer dereference when entry is used
> in nla_nest_end().

No it will not, there is no way for the flow to get to nla_nest_end()
if the skb is full :/

I will fix the commit message myself and apply but I'd like you to not
send more "error checking" patches to networking unless you're sure
that there is indeed a bug.

Re: [PATCH net-next v3] ethtool: tunnels: check the return value of nla_nest_start()

[patchwork-bot+netdevbpf]

Hello:

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

On Wed, 21 Sep 2022 11:17:16 -0700 you wrote:
> Check the return value of nla_nest_start(). When starting the entry
> level nested attributes, if the tailroom of socket buffer is
> insufficient to store the attribute header and payload, the return value
> will be NULL. It will cause null pointer dereference when entry is used
> in nla_nest_end().
> 
> Signed-off-by: Li Zhong <floridsleeves@gmail.com>
> 
> [...]

Here is the summary with links:
  - [net-next,v3] ethtool: tunnels: check the return value of nla_nest_start()
    https://git.kernel.org/netdev/net-next/c/05cd823863fd

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

Re: [PATCH net-next v3] ethtool: tunnels: check the return value of nla_nest_start()

[Li Zhong]

On Thu, Sep 22, 2022 at 7:28 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Wed, 21 Sep 2022 11:17:16 -0700 Li Zhong wrote:
> > It will cause null pointer dereference when entry is used
> > in nla_nest_end().
>
> No it will not, there is no way for the flow to get to nla_nest_end()
> if the skb is full :/
>
> I will fix the commit message myself and apply but I'd like you to not
> send more "error checking" patches to networking unless you're sure
> that there is indeed a bug.

Thanks for your reply. We'll try to increase the precision of the tool.