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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CANn89i+yvyPMU1SE=p3Mm1S=UexsXSa4gzH3heUg17sa+iFK9w@mail.gmail.com>
Date: Wed, 18 Dec 2024 11:44:21 +0100
From: Eric Dumazet <edumazet@...gle.com>
To: Jakub Kicinski <kuba@...nel.org>
Cc: davem@...emloft.net, netdev@...r.kernel.org, pabeni@...hat.com, 
	jdamato@...tly.com, almasrymina@...gle.com, sridhar.samudrala@...el.com, 
	amritha.nambiar@...el.com
Subject: Re: [PATCH net] netdev-genl: avoid empty messages in napi get

On Wed, Dec 18, 2024 at 3:43 AM Jakub Kicinski <kuba@...nel.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@...nel.org>
> ---
> CC: jdamato@...tly.com
> CC: almasrymina@...gle.com
> CC: sridhar.samudrala@...el.com
> CC: amritha.nambiar@...el.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;