diff mbox

[dpdk-dev] vhost: add note about sockets in server mode

Message ID 1519634341-12305-1-git-send-email-i.maximets@samsung.com (mailing list archive)
State Accepted, archived
Delegated to: Maxime Coquelin
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Ilya Maximets Feb. 26, 2018, 8:39 a.m. UTC 
  From time to time, someone sends patches about unlinking existing
sockets when registering a vhost user in server mode.

A recent example:
	http://dpdk.org/ml/archives/dev/2018-February/090025.html

This problem has been discussed many times, and it was made clear that
the library should not unlink files given by the application in order
to avoid possible security problems, such as removing random files
used by other programs.

One of the first discussions:
	http://dpdk.org/ml/archives/dev/2015-December/030326.html

To avoid such patches in the future, it was decided to add a comment
that explains what is happening and tries to describe the reasoning.

Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
---

I'm open for suggestions. Wording/grammar fixes are also welcome.

 lib/librte_vhost/socket.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

Comments

Maxime Coquelin March 6, 2018, 9:06 a.m. UTC | #1 
Hi Ilya,

On 02/26/2018 09:39 AM, Ilya Maximets wrote:
>  From time to time, someone sends patches about unlinking existing
> sockets when registering a vhost user in server mode.
> 
> A recent example:
> 	http://dpdk.org/ml/archives/dev/2018-February/090025.html
> 
> This problem has been discussed many times, and it was made clear that
> the library should not unlink files given by the application in order
> to avoid possible security problems, such as removing random files
> used by other programs.
> 
> One of the first discussions:
> 	http://dpdk.org/ml/archives/dev/2015-December/030326.html
> 
> To avoid such patches in the future, it was decided to add a comment
> that explains what is happening and tries to describe the reasoning.
> 
> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
> ---
> 
> I'm open for suggestions. Wording/grammar fixes are also welcome.
> 
>   lib/librte_vhost/socket.c | 10 ++++++++++
>   1 file changed, 10 insertions(+)
> 
> diff --git a/lib/librte_vhost/socket.c b/lib/librte_vhost/socket.c
> index 83befdc..e8584f3 100644
> --- a/lib/librte_vhost/socket.c
> +++ b/lib/librte_vhost/socket.c
> @@ -318,6 +318,16 @@ vhost_user_start_server(struct vhost_user_socket *vsocket)
>   	int fd = vsocket->socket_fd;
>   	const char *path = vsocket->path;
>   
> +	/*
> +	 * bind () may fail if the socket file with the same name already
> +	 * exists. But the library obviously should not delete the file
> +	 * provided by the user, since we can not be sure that it is not
> +	 * being used by other applications. Moreover, many applications form
> +	 * socket names based on user input, which is prone to errors.
> +	 *
> +	 * The user must ensure that the socket does not exist before
> +	 * registering the vhost driver in server mode.
> +	 */
>   	ret = bind(fd, (struct sockaddr *)&vsocket->un, sizeof(vsocket->un));
>   	if (ret < 0) {
>   		RTE_LOG(ERR, VHOST_CONFIG,
> 

Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Thanks!
Maxime
diff mbox

Patch

diff --git a/lib/librte_vhost/socket.c b/lib/librte_vhost/socket.c
index 83befdc..e8584f3 100644
--- a/lib/librte_vhost/socket.c
+++ b/lib/librte_vhost/socket.c
@@ -318,6 +318,16 @@  vhost_user_start_server(struct vhost_user_socket *vsocket)
 	int fd = vsocket->socket_fd;
 	const char *path = vsocket->path;
 
+	/*
+	 * bind () may fail if the socket file with the same name already
+	 * exists. But the library obviously should not delete the file
+	 * provided by the user, since we can not be sure that it is not
+	 * being used by other applications. Moreover, many applications form
+	 * socket names based on user input, which is prone to errors.
+	 *
+	 * The user must ensure that the socket does not exist before
+	 * registering the vhost driver in server mode.
+	 */
 	ret = bind(fd, (struct sockaddr *)&vsocket->un, sizeof(vsocket->un));
 	if (ret < 0) {
 		RTE_LOG(ERR, VHOST_CONFIG,