[PATCH net] netdev-genl: avoid empty messages in napi get

[PATCH net] netdev-genl: avoid empty messages in napi get

[Jakub Kicinski]

Empty netlink responses from do() are not correct (as opposed to
dump() where not dumping anything is perfectly fine).
We should return an error if the target object does not exist,
in this case if the netdev is down we "hide" the NAPI instances.

Fixes: 27f91aaf49b3 ("netdev-genl: Add netlink framework functions for napi")
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
CC: jdamato@fastly.com
CC: almasrymina@google.com
CC: sridhar.samudrala@intel.com
CC: amritha.nambiar@intel.com
---
 net/core/netdev-genl.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/net/core/netdev-genl.c b/net/core/netdev-genl.c
index b4becd4065d9..dfb2430a0fe3 100644
--- a/net/core/netdev-genl.c
+++ b/net/core/netdev-genl.c
@@ -238,6 +238,10 @@ int netdev_nl_napi_get_doit(struct sk_buff *skb, struct genl_info *info)
 	napi = napi_by_id(napi_id);
 	if (napi) {
 		err = netdev_nl_napi_fill_one(rsp, napi, info);
+		if (!rsp->len) {
+			err = -ENOENT;
+			goto err_free_msg;
+		}
 	} else {
 		NL_SET_BAD_ATTR(info->extack, info->attrs[NETDEV_A_NAPI_ID]);
 		err = -ENOENT;
-- 
2.47.1

Re: [PATCH net] netdev-genl: avoid empty messages in napi get

[Eric Dumazet]

On Wed, Dec 18, 2024 at 3:43 AM Jakub Kicinski <kuba@kernel.org> wrote:
>
> Empty netlink responses from do() are not correct (as opposed to
> dump() where not dumping anything is perfectly fine).
> We should return an error if the target object does not exist,
> in this case if the netdev is down we "hide" the NAPI instances.
>
> Fixes: 27f91aaf49b3 ("netdev-genl: Add netlink framework functions for napi")
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
> CC: jdamato@fastly.com
> CC: almasrymina@google.com
> CC: sridhar.samudrala@intel.com
> CC: amritha.nambiar@intel.com
> ---
>  net/core/netdev-genl.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/net/core/netdev-genl.c b/net/core/netdev-genl.c
> index b4becd4065d9..dfb2430a0fe3 100644
> --- a/net/core/netdev-genl.c
> +++ b/net/core/netdev-genl.c
> @@ -238,6 +238,10 @@ int netdev_nl_napi_get_doit(struct sk_buff *skb, struct genl_info *info)
>         napi = napi_by_id(napi_id);
>         if (napi) {
>                 err = netdev_nl_napi_fill_one(rsp, napi, info);
> +               if (!rsp->len) {
> +                       err = -ENOENT;
> +                       goto err_free_msg;

Well, rtnl and rcu are held at this point.

What about instead :

diff --git a/net/core/netdev-genl.c b/net/core/netdev-genl.c
index 9527dd46e4dc39a43e965b833df306a5cc44c94d..f86cfb0b33616722ec40874e8bc90cece57df869
100644
--- a/net/core/netdev-genl.c
+++ b/net/core/netdev-genl.c
@@ -246,6 +246,8 @@ int netdev_nl_napi_get_doit(struct sk_buff *skb,
struct genl_info *info)
        rcu_read_unlock();
        rtnl_unlock();

+       if (!err && !rsp->len)
+               err = -ENOENT;
        if (err)
                goto err_free_msg;