Re: [PATCH iproute] iplink: emit the error message if open_fds_add failed

[Stephen Hemminger <stephen@networkplumber.org>]

On Tue, 21 Jan 2025 17:16:42 +0300
Denis Kirjanov <kirjanov@gmail.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@gmail.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@6wind.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@orange.com>
    Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
    Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>