[dpdk-dev] [PATCH] net/mlx5: fix existing file remove

[dpdk-dev] [PATCH] net/mlx5: fix existing file remove

[Xueming Li]

Remove file detection before removing existing file.

Fixes: f8b9a3bad467 ("net/mlx5: install a socket to exchange a file descriptor")

Signed-off-by: Xueming Li <xuemingl@mellanox.com>
---
 drivers/net/mlx5/mlx5_socket.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_socket.c b/drivers/net/mlx5/mlx5_socket.c
index 61c1a4a..fb80fb3 100644
--- a/drivers/net/mlx5/mlx5_socket.c
+++ b/drivers/net/mlx5/mlx5_socket.c
@@ -32,7 +32,6 @@
 	};
 	int ret;
 	int flags;
-	struct stat file_stat;
 
 	/*
 	 * Initialise the socket to communicate with the secondary
@@ -52,9 +51,7 @@
 		goto out;
 	snprintf(sun.sun_path, sizeof(sun.sun_path), "/var/tmp/%s_%d",
 		 MLX5_DRIVER_NAME, priv->primary_socket);
-	ret = stat(sun.sun_path, &file_stat);
-	if (!ret)
-		claim_zero(remove(sun.sun_path));
+	remove(sun.sun_path);
 	ret = bind(priv->primary_socket, (const struct sockaddr *)&sun,
 		   sizeof(sun));
 	if (ret < 0) {
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH] net/mlx5: fix existing file remove

[Nélio Laranjeiro]

On Fri, Mar 16, 2018 at 06:27:48PM +0800, Xueming Li wrote:
> Remove file detection before removing existing file.
> 

The explanation can induce some misunderstandings, the fact is there is
no guarantee that the file won't be removed by an external
user/application in between the stat() and remove() syscalls.
As it cannot be done atomically there is no need to need to make such
verification, remove() will fail if the file does not exists anymore.

> Fixes: f8b9a3bad467 ("net/mlx5: install a socket to exchange a file descriptor")
> 
> Signed-off-by: Xueming Li <xuemingl@mellanox.com>

Unless the commit log which can explain why this move is made,

Acked-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>

> ---
>  drivers/net/mlx5/mlx5_socket.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/drivers/net/mlx5/mlx5_socket.c b/drivers/net/mlx5/mlx5_socket.c
> index 61c1a4a..fb80fb3 100644
> --- a/drivers/net/mlx5/mlx5_socket.c
> +++ b/drivers/net/mlx5/mlx5_socket.c
> @@ -32,7 +32,6 @@
>  	};
>  	int ret;
>  	int flags;
> -	struct stat file_stat;
>  
>  	/*
>  	 * Initialise the socket to communicate with the secondary
> @@ -52,9 +51,7 @@
>  		goto out;
>  	snprintf(sun.sun_path, sizeof(sun.sun_path), "/var/tmp/%s_%d",
>  		 MLX5_DRIVER_NAME, priv->primary_socket);
> -	ret = stat(sun.sun_path, &file_stat);
> -	if (!ret)
> -		claim_zero(remove(sun.sun_path));
> +	remove(sun.sun_path);
>  	ret = bind(priv->primary_socket, (const struct sockaddr *)&sun,
>  		   sizeof(sun));
>  	if (ret < 0) {
> -- 
> 1.8.3.1
> 

-- 
Nélio Laranjeiro
6WIND

[dpdk-dev] [PATCH v1] net/mlx5: fix existing file removal

[Xueming Li]

There is no guarantee that the file won't be removed by external
user/application between the stat() and remove() syscalls, remove() will
fail if the file no longer exists.

Fixes: f8b9a3bad467 ("net/mlx5: install a socket to exchange a file descriptor")

Signed-off-by: Xueming Li <xuemingl@mellanox.com>
---
 drivers/net/mlx5/mlx5_socket.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_socket.c b/drivers/net/mlx5/mlx5_socket.c
index 61c1a4a..fb80fb3 100644
--- a/drivers/net/mlx5/mlx5_socket.c
+++ b/drivers/net/mlx5/mlx5_socket.c
@@ -32,7 +32,6 @@
 	};
 	int ret;
 	int flags;
-	struct stat file_stat;
 
 	/*
 	 * Initialise the socket to communicate with the secondary
@@ -52,9 +51,7 @@
 		goto out;
 	snprintf(sun.sun_path, sizeof(sun.sun_path), "/var/tmp/%s_%d",
 		 MLX5_DRIVER_NAME, priv->primary_socket);
-	ret = stat(sun.sun_path, &file_stat);
-	if (!ret)
-		claim_zero(remove(sun.sun_path));
+	remove(sun.sun_path);
 	ret = bind(priv->primary_socket, (const struct sockaddr *)&sun,
 		   sizeof(sun));
 	if (ret < 0) {
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH v1] net/mlx5: fix existing file removal

[Nélio Laranjeiro]

On Fri, Mar 16, 2018 at 11:22:27PM +0800, Xueming Li wrote:
> There is no guarantee that the file won't be removed by external
> user/application between the stat() and remove() syscalls, remove() will
> fail if the file no longer exists.
> 
> Fixes: f8b9a3bad467 ("net/mlx5: install a socket to exchange a file descriptor")
> 
> Signed-off-by: Xueming Li <xuemingl@mellanox.com>

Acked-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>

> ---
>  drivers/net/mlx5/mlx5_socket.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/drivers/net/mlx5/mlx5_socket.c b/drivers/net/mlx5/mlx5_socket.c
> index 61c1a4a..fb80fb3 100644
> --- a/drivers/net/mlx5/mlx5_socket.c
> +++ b/drivers/net/mlx5/mlx5_socket.c
> @@ -32,7 +32,6 @@
>  	};
>  	int ret;
>  	int flags;
> -	struct stat file_stat;
>  
>  	/*
>  	 * Initialise the socket to communicate with the secondary
> @@ -52,9 +51,7 @@
>  		goto out;
>  	snprintf(sun.sun_path, sizeof(sun.sun_path), "/var/tmp/%s_%d",
>  		 MLX5_DRIVER_NAME, priv->primary_socket);
> -	ret = stat(sun.sun_path, &file_stat);
> -	if (!ret)
> -		claim_zero(remove(sun.sun_path));
> +	remove(sun.sun_path);
>  	ret = bind(priv->primary_socket, (const struct sockaddr *)&sun,
>  		   sizeof(sun));
>  	if (ret < 0) {
> -- 
> 1.8.3.1
> 

-- 
Nélio Laranjeiro
6WIND

Re: [dpdk-dev] [PATCH v1] net/mlx5: fix existing file removal

[Shahaf Shuler]

Friday, March 16, 2018 5:38 PM, Nélio Laranjeiro:
> On Fri, Mar 16, 2018 at 11:22:27PM +0800, Xueming Li wrote:
> > There is no guarantee that the file won't be removed by external
> > user/application between the stat() and remove() syscalls, remove()
> > will fail if the file no longer exists.
> >
> > Fixes: f8b9a3bad467 ("net/mlx5: install a socket to exchange a file
> > descriptor")
> >
> > Signed-off-by: Xueming Li <xuemingl@mellanox.com>
> 
> Acked-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>

Applied to next-net-mlx, thanks.