[master] e3e2ef33a vslc: Plug mmap file descriptor leak

Dridi Boukelmoune dridi.boukelmoune at gmail.com
Tue Nov 22 10:17:05 UTC 2022


commit e3e2ef33a0455a3b8b7ede38d1bcef73f18ffd61
Author: Dridi Boukelmoune <dridi.boukelmoune at gmail.com>
Date:   Thu Nov 10 14:58:27 2022 +0100

    vslc: Plug mmap file descriptor leak
    
    To be consistent with how the file cursor behaves, the close_fd field is
    duplicated in the mmap cursor. If a VUT replaces stdin's file descriptor
    with a regular file's fd using dup2(2), we don't want to close it just
    because we managed to mmap(2) it.
    
    For some reason we don't use the closefd() macro in the VSL cursor code,
    potentially to avoid its underlying assertion in libvarnishapi.
    
    On the other hand we do use it in other places:
    
        $ git grep -l closefd -- lib/libvarnishapi/
        lib/libvarnishapi/daemon.c
        lib/libvarnishapi/vsm.c
    
    So maybe in a subsequent change `(void)close(fd)` statements could turn
    into `closefd(&fd)` in vsl_cursor.c to harden those code paths as well.

diff --git a/lib/libvarnishapi/vsl_cursor.c b/lib/libvarnishapi/vsl_cursor.c
index 3d356ad1c..8cf259394 100644
--- a/lib/libvarnishapi/vsl_cursor.c
+++ b/lib/libvarnishapi/vsl_cursor.c
@@ -409,6 +409,7 @@ struct vslc_mmap {
 	unsigned			magic;
 #define VSLC_MMAP_MAGIC			0x7de15f61
 	int				fd;
+	int				close_fd;
 	char				*b;
 	char				*e;
 	struct VSL_cursor		cursor;
@@ -424,6 +425,8 @@ vslc_mmap_delete(const struct VSL_cursor *cursor)
 	CAST_OBJ_NOTNULL(c, cursor->priv_data, VSLC_MMAP_MAGIC);
 	assert(&c->cursor == cursor);
 	AZ(munmap(c->b, c->e - c->b));
+	if (c->close_fd)
+		(void)close(c->fd);
 	FREE_OBJ(c);
 }
 
@@ -480,7 +483,7 @@ static const struct vslc_tbl vslc_mmap_tbl = {
 };
 
 static struct VSL_cursor *
-vsl_cursor_mmap(struct VSL_data *vsl, int fd)
+vsl_cursor_mmap(struct VSL_data *vsl, int fd, int close_fd)
 {
 	struct vslc_mmap *c;
 	struct stat st[1];
@@ -500,7 +503,8 @@ vsl_cursor_mmap(struct VSL_data *vsl, int fd)
 	ALLOC_OBJ(c, VSLC_MMAP_MAGIC);
 	if (c == NULL) {
 		(void)munmap(p, st->st_size);
-		(void)close(fd);
+		if (close_fd)
+			(void)close(fd);
 		vsl_diag(vsl, "Out of memory");
 		return (NULL);
 	}
@@ -508,6 +512,7 @@ vsl_cursor_mmap(struct VSL_data *vsl, int fd)
 	c->cursor.priv_data = c;
 
 	c->fd = fd;
+	c->close_fd = close_fd;
 	c->b = p;
 	c->e = c->b + st->st_size;
 	c->next.ptr = TRUST_ME(c->b + sizeof VSL_FILE_ID);
@@ -557,7 +562,7 @@ VSL_CursorFile(struct VSL_data *vsl, const char *name, unsigned options)
 		return (NULL);
 	}
 
-	mc = vsl_cursor_mmap(vsl, fd);
+	mc = vsl_cursor_mmap(vsl, fd, close_fd);
 	if (mc == NULL)
 		return (NULL);
 	if (mc != MAP_FAILED)


More information about the varnish-commit mailing list