netdev - Re: [PATCH iproute] iplink: emit the error message if open_fds

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: <20250126085802.5af23e35@hermes.local>
Date: Sun, 26 Jan 2025 08:58:02 -0800
From: Stephen Hemminger <stephen@...workplumber.org>
To: Denis Kirjanov <kirjanov@...il.com>
Cc: dsahern@...nel.org, netdev@...r.kernel.org
Subject: Re: [PATCH iproute] iplink: emit the error message if open_fds_add
 failed

On Tue, 21 Jan 2025 17:16:42 +0300
Denis Kirjanov <kirjanov@...il.com> wrote:

> open_fds_add may fail since it adds an open fd
> to the fixed array. Print the error message in
> the such case.
> 
> Signed-off-by: Denis Kirjanov <kirjanov@...il.com>
> ---
>  ip/iplink.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/ip/iplink.c b/ip/iplink.c
> index 59e8caf4..1396da23 100644
> --- a/ip/iplink.c
> +++ b/ip/iplink.c
> @@ -666,7 +666,9 @@ int iplink_parse(int argc, char **argv, struct iplink_req *req, char **type)
>  			if (netns < 0)
>  				invarg("Invalid \"netns\" value\n", *argv);
>  
> -			open_fds_add(netns);
> +			if (open_fds_add(netns))
> +				invarg("No descriptors left\n", *argv);
> +
>  			addattr_l(&req->n, sizeof(*req), IFLA_NET_NS_FD,
>  				  &netns, 4);
>  			move_netns = true;

Not an acceptable fix.
It is not an invalid argument, that would just be confusing.
How to reproduce this?

The original patch that added this made arbitrary choice of 5 fd's.
If this can be reproduced, that should be increased or switch to a dynamic list.

commit 57daf8ff8c6c357a5a083657e5b03d2883cbc4f9
Author: Nicolas Dichtel <nicolas.dichtel@...nd.com>
Date:   Wed Sep 18 18:49:41 2024 +0200

    iplink: fix fd leak when playing with netns
    
    The command 'ip link set foo netns mynetns' opens a file descriptor to fill
    the netlink attribute IFLA_NET_NS_FD. This file descriptor is never closed.
    When batch mode is used, the number of file descriptor may grow greatly and
    reach the maximum file descriptor number that can be opened.
    
    This fd can be closed only after the netlink answer. Moreover, a second
    fd could be opened because some (struct link_util)->parse_opt() handlers
    call iplink_parse().
    
    Let's add a helper to manage these fds:
     - open_fds_add() stores a fd, up to 5 (arbitrary choice, it seems enough);
     - open_fds_close() closes all stored fds.
    
    Fixes: 0dc34c7713bb ("iproute2: Add processless network namespace support")
    Reported-by: Alexandre Ferrieux <alexandre.ferrieux@...nge.com>
    Signed-off-by: Nicolas Dichtel <nicolas.dichtel@...nd.com>
    Signed-off-by: Stephen Hemminger <stephen@...workplumber.org>