Re: [PATCH] eal: fix file descriptor leakage with unhandled messages

[fengchengwen <fengchengwen@huawei.com>]

Acked-by: Chengwen Feng <fengchengwen@huawei.com>

On 2023/6/28 20:19, Viacheslav Ovsiienko wrote:
> The sendmsg()/recvmsg() API is used to establish communication between
> the DPDK processes. The API supposes inter-process file descriptors
> sending and conversion, the recipient sees the resulting descriptors
> in the received message - the operating systems creates ones in the
> right context.
> 
> The message receiving is performed by EAL in the dedicated thread
> and it might happen the message is received by EAL and not handled
> by addressed PMD or application due to some reasons (timeouts, race
> condition, etc). EAL just dropped unhandled messages causing the
> file descriptor leakage if these ones were presented in the message.
> 
> The patch closes the descriptors (if any) in unhandled messages.
> 
> Fixes: 783b6e54971 ("eal: add synchronous multi-process communication")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>
> ---
>  lib/eal/common/eal_common_proc.c | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/eal/common/eal_common_proc.c b/lib/eal/common/eal_common_proc.c
> index 1fc1d6c53b..9676dd73c5 100644
> --- a/lib/eal/common/eal_common_proc.c
> +++ b/lib/eal/common/eal_common_proc.c
> @@ -321,6 +321,15 @@ read_msg(int fd, struct mp_msg_internal *m, struct sockaddr_un *s)
>  	return msglen;
>  }
>  
> +static void
> +cleanup_msg_fds(const struct rte_mp_msg *msg)
> +{
> +	int i;
> +
> +	for (i = 0; i < msg->num_fds; i++)
> +		close(msg->fds[i]);
> +}
> +
>  static void
>  process_msg(struct mp_msg_internal *m, struct sockaddr_un *s)
>  {
> @@ -349,8 +358,10 @@ process_msg(struct mp_msg_internal *m, struct sockaddr_un *s)
>  			else if (pending_req->type == REQUEST_TYPE_ASYNC)
>  				req = async_reply_handle_thread_unsafe(
>  						pending_req);
> -		} else
> +		} else {
>  			RTE_LOG(ERR, EAL, "Drop mp reply: %s\n", msg->name);
> +			cleanup_msg_fds(msg);
> +		}
>  		pthread_mutex_unlock(&pending_requests.lock);
>  
>  		if (req != NULL)
> @@ -380,6 +391,7 @@ process_msg(struct mp_msg_internal *m, struct sockaddr_un *s)
>  			RTE_LOG(ERR, EAL, "Cannot find action: %s\n",
>  				msg->name);
>  		}
> +		cleanup_msg_fds(msg);
>  	} else if (action(msg, s->sun_path) < 0) {
>  		RTE_LOG(ERR, EAL, "Fail to handle message: %s\n", msg->name);
>  	}
>