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

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

[Viacheslav Ovsiienko]

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);
 	}
-- 
2.18.1

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

[fengchengwen]

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);
>  	}
> 

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

[Burakov, Anatoly]

On 6/28/2023 1:19 PM, 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>
> ---

Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>

-- 
Thanks,
Anatoly

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

[David Marchand]

On Mon, Jul 3, 2023 at 12:31 PM Burakov, Anatoly
<anatoly.burakov@intel.com> wrote:
> On 6/28/2023 1:19 PM, 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")

Fixed the Fixes: tag (12 digits for hash).

> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>
Acked-by: Chengwen Feng <fengchengwen@huawei.com>
> Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>

Applied, thanks.


-- 
David Marchand