sfd: Don't forget to test sfd_map when closing old fds

Submitted by Cyrill Gorcunov on May 16, 2018, 8:29 p.m.

Details

Message ID 20180516202949.6035-1-gorcunov@gmail.com
State Accepted
Series "sfd: Don't forget to test sfd_map when closing old fds"
Commit e5856d4a7b1eb3625ac660c04290ccc2bf433a45
Headers show

Commit Message

Cyrill Gorcunov May 16, 2018, 8:29 p.m.
is_any_service_fd didn't check for service file descriptor
being marked in sfd_map in result we may end up leaving
some garbage file obtained from the program executing
criu.

[root@sqvm0910 criu-stable.git]# ll /proc/354877/fd
lr-x------ 1 root root 64 May 16 19:03 114 -> /
lr-x------ 1 root root 64 May 16 19:03 115 -> /
l--------- 1 root root 64 May 16 19:03 117 -> /18311 (deleted)
lr-x------ 1 root root 64 May 16 19:03 119 -> /vzt/del/vzctl-rm-me.X1JH4vI (deleted)

We are to test if the file descriptor was indeed installed
explicitly.

https://jira.sw.ru/browse/PSBM-83892

Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---
 criu/util.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

Patch hide | download patch | download mbox

diff --git a/criu/util.c b/criu/util.c
index be892bbbfe84..45d87dd72d0b 100644
--- a/criu/util.c
+++ b/criu/util.c
@@ -636,8 +636,16 @@  int clone_service_fd(struct pstree_item *me)
 
 bool is_any_service_fd(int fd)
 {
-	return fd > __get_service_fd(SERVICE_FD_MAX, service_fd_id) &&
-		fd < __get_service_fd(SERVICE_FD_MIN, service_fd_id);
+	int sfd_min_fd = __get_service_fd(SERVICE_FD_MAX, service_fd_id);
+	int sfd_max_fd = __get_service_fd(SERVICE_FD_MIN, service_fd_id);
+
+	if (fd > sfd_min_fd && fd < sfd_max_fd) {
+		int type = SERVICE_FD_MAX - (fd - sfd_min_fd);
+		if (type > SERVICE_FD_MIN && type < SERVICE_FD_MAX)
+			return !!test_bit(type, sfd_map);
+	}
+
+	return false;
 }
 
 bool is_service_fd(int fd, enum sfd_type type)

Comments

Andrei Vagin May 22, 2018, 8:34 p.m.
On Wed, May 16, 2018 at 11:29:49PM +0300, Cyrill Gorcunov wrote:
> is_any_service_fd didn't check for service file descriptor
> being marked in sfd_map in result we may end up leaving
> some garbage file obtained from the program executing
> criu.
> 
> [root@sqvm0910 criu-stable.git]# ll /proc/354877/fd
> lr-x------ 1 root root 64 May 16 19:03 114 -> /
> lr-x------ 1 root root 64 May 16 19:03 115 -> /
> l--------- 1 root root 64 May 16 19:03 117 -> /18311 (deleted)
> lr-x------ 1 root root 64 May 16 19:03 119 -> /vzt/del/vzctl-rm-me.X1JH4vI (deleted)
> 
> We are to test if the file descriptor was indeed installed
> explicitly.
> 
> https://jira.sw.ru/browse/PSBM-83892

pls don't use a non-public links in commit messages

> 
> Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
> ---
>  criu/util.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/criu/util.c b/criu/util.c
> index be892bbbfe84..45d87dd72d0b 100644
> --- a/criu/util.c
> +++ b/criu/util.c
> @@ -636,8 +636,16 @@ int clone_service_fd(struct pstree_item *me)
>  
>  bool is_any_service_fd(int fd)
>  {
> -	return fd > __get_service_fd(SERVICE_FD_MAX, service_fd_id) &&
> -		fd < __get_service_fd(SERVICE_FD_MIN, service_fd_id);
> +	int sfd_min_fd = __get_service_fd(SERVICE_FD_MAX, service_fd_id);
> +	int sfd_max_fd = __get_service_fd(SERVICE_FD_MIN, service_fd_id);
> +
> +	if (fd > sfd_min_fd && fd < sfd_max_fd) {

^^^
> +		int type = SERVICE_FD_MAX - (fd - sfd_min_fd);
> +		if (type > SERVICE_FD_MIN && type < SERVICE_FD_MAX)

This expression and the previous one are equal, aren't they?

> +			return !!test_bit(type, sfd_map);
> +	}
> +
> +	return false;
>  }
>  
>  bool is_service_fd(int fd, enum sfd_type type)
> -- 
> 2.14.3
>