examples/vhost_blk: fix the TOCTOU

Fix the time of check time of use warning in example code

Coverity issue: 350589 158663

Fixes c19beb3f38cd ("examples/vhost_blk: introduce vhost storage sample")

Cc [email protected]

Signed-off-by Jin Yu <[email protected]>

---
 examples/vhost_blk/vhost_blk.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

examples/vhost_blk/vhost_blk.c +0 -0

index 3182a488b..bcb4ebb0b 100644

-----Original Message-----
From: Bruce Richardson <[email protected]>
Sent: Tuesday, November 26, 2019 6:26 PM
To: Yu, Jin <[email protected]>
Cc: Maxime Coquelin <[email protected]>; Bie, Tiwei <[email protected]>; Wang, Zhihong <[email protected]>; [email protected]; [email protected]
Subject: Re: [dpdk-dev] [PATCH] examples/vhost_blk: fix the TOCTOU

Quoted 18 lines · Jin Yu

Fix the time of check time of use warning in example code

Coverity issue: 350589 158663

Fixes c19beb3f38cd ("examples/vhost_blk: introduce vhost storage

sample")
Cc: [email protected]

Signed-off-by Jin Yu <[email protected]>

---
 examples/vhost_blk/vhost_blk.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

(quoted diff) +0 -0

b/examples/vhost_blk/vhost_blk.c index 3182a488b..bcb4ebb0b 100644

examples/vhost_blk/vhost_blk.c +0 -0

@@ -993,11 +993,7 @@ vhost_blk_ctrlr_construct(const char *ctrlr_name)
 	}

examples/vhost_blk/vhost_blk.c vhost_blk_ctrlr_construct(const char *ctrlr_name) · line 998

Quoted 10 lines

examples/vhost_blk/vhost_blk.c +1 -5

 	snprintf(dev_pathname, sizeof(dev_pathname), "%s/%s", path,
 ctrlr_name);
 
-	if (access(dev_pathname, F_OK) != -1) {
-		if (unlink(dev_pathname) != 0)
-			rte_exit(EXIT_FAILURE, "Cannot remove %s.\n",
-				 dev_pathname);
-	}
+	unlink(dev_pathname);
 

The original code did an exit if the delete failed, do you intend there to be a behaviour change here? You can probably get the same behaviour if you check the errno on an unlink failure, e.g. ENOENT means file doesn't exist.

If not having the app exit on unlink failure is reasonable behaviour then ignore this comment.

Regards,
/Bruce

Quoted 43 lines

-----Original Message-----
From: Bruce Richardson <[email protected]>
Sent: Tuesday, November 26, 2019 6:26 PM
To: Yu, Jin <[email protected]>
Cc: Maxime Coquelin <[email protected]>; Bie, Tiwei <[email protected]>; Wang, Zhihong <[email protected]>; [email protected]; [email protected]
Subject: Re: [dpdk-dev] [PATCH] examples/vhost_blk: fix the TOCTOU

Quoted 18 lines · Jin Yu

Fix the time of check time of use warning in example code

Coverity issue: 350589 158663

Fixes c19beb3f38cd ("examples/vhost_blk: introduce vhost storage

sample")
Cc: [email protected]

Signed-off-by Jin Yu <[email protected]>

---
 examples/vhost_blk/vhost_blk.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

(quoted diff) +0 -0

b/examples/vhost_blk/vhost_blk.c index 3182a488b..bcb4ebb0b 100644

examples/vhost_blk/vhost_blk.c +0 -0

@@ -993,11 +993,7 @@ vhost_blk_ctrlr_construct(const char *ctrlr_name)
 	}

examples/vhost_blk/vhost_blk.c vhost_blk_ctrlr_construct(const char *ctrlr_name) · line 998

Quoted 10 lines

examples/vhost_blk/vhost_blk.c +1 -5

 	snprintf(dev_pathname, sizeof(dev_pathname), "%s/%s", path,
 ctrlr_name);
 
-	if (access(dev_pathname, F_OK) != -1) {
-		if (unlink(dev_pathname) != 0)
-			rte_exit(EXIT_FAILURE, "Cannot remove %s.\n",
-				 dev_pathname);
-	}
+	unlink(dev_pathname);
 

The original code did an exit if the delete failed, do you intend there to be a behaviour change here? You can probably get the same behaviour if you check the errno on an unlink failure, e.g. ENOENT means file doesn't exist.

Quoted 2 lines

If not having the app exit on unlink failure is reasonable behaviour then ignore this comment.

I considered it. I think it's ok to ignore the errno of unlink failure. This code just want to remove the file. There are two situations. The first one is that file doesn't exist the unlink fails and it's ok to ignore. The second one is that unlink fails to remove file but the next bind() would fail too so I think it's ok to ignore too.

Quoted 1 line

Quoted 43 lines

-----Original Message-----
From: Bruce Richardson <[email protected]>
Sent: Tuesday, November 26, 2019 6:26 PM
To: Yu, Jin <[email protected]>
Cc: Maxime Coquelin <[email protected]>; Bie, Tiwei <[email protected]>; Wang, Zhihong <[email protected]>; [email protected]; [email protected]
Subject: Re: [dpdk-dev] [PATCH] examples/vhost_blk: fix the TOCTOU

Quoted 18 lines · Jin Yu

Fix the time of check time of use warning in example code

Coverity issue: 350589 158663

Fixes c19beb3f38cd ("examples/vhost_blk: introduce vhost storage

sample")
Cc: [email protected]

Signed-off-by Jin Yu <[email protected]>

---
 examples/vhost_blk/vhost_blk.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

(quoted diff) +0 -0

b/examples/vhost_blk/vhost_blk.c index 3182a488b..bcb4ebb0b 100644

examples/vhost_blk/vhost_blk.c +0 -0

@@ -993,11 +993,7 @@ vhost_blk_ctrlr_construct(const char *ctrlr_name)
 	}

examples/vhost_blk/vhost_blk.c vhost_blk_ctrlr_construct(const char *ctrlr_name) · line 998

Quoted 10 lines

examples/vhost_blk/vhost_blk.c +1 -5

 	snprintf(dev_pathname, sizeof(dev_pathname), "%s/%s", path,
 ctrlr_name);
 
-	if (access(dev_pathname, F_OK) != -1) {
-		if (unlink(dev_pathname) != 0)
-			rte_exit(EXIT_FAILURE, "Cannot remove %s.\n",
-				 dev_pathname);
-	}
+	unlink(dev_pathname);
 

The original code did an exit if the delete failed, do you intend there to be a behaviour change here? You can probably get the same behaviour if you check the errno on an unlink failure, e.g. ENOENT means file doesn't exist.

Quoted 2 lines

If not having the app exit on unlink failure is reasonable behaviour then ignore this comment.

I considered it. I think it's ok to ignore the errno of unlink failure. This code just want to remove the file. There are two situations. The first one is that file doesn't exist the unlink fails and it's ok to ignore. The second one is that unlink fails to remove file but the next bind() would fail too so I think it's ok to ignore too.

Quoted 2 lines

Regards,
/Bruce

Fix the time of check time of use warning in example code. Ignore the errno of unlink failure. There are two situations. The first one is that file doesn't exist the unlink fails and it's ok to ignore. The second one is that unlink fails to remove file but the next bind() would fail too.

Coverity issue: 350589 158663

Fixes c19beb3f38cd ("examples/vhost_blk: introduce vhost storage sample")

Cc [email protected]

Signed-off-by Jin Yu <[email protected]>

Reviewed-by Maxime Coquelin <[email protected]>

---
V2 - complement the commit message.

---
 examples/vhost_blk/vhost_blk.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)