Fix file descriptor leak on pool import.

While here, return the error we get from fgets() instead of pretending we succeeded. Descriptor leak can be easily reproduced by doing: # zpool import tank # sysctl kern.openfiles # zpool export tank; zpool import tank # sysctl kern.openfiles We were leaking four file descriptors on every import. Signed-off-by: Pawel Jakub Dawidek <[email protected]>
openzfs · Dec 4, 2023 · fd97ceb · fd97ceb
1 parent 014265f
commit fd97ceb
Show file tree

Hide file tree

Showing 2 changed files with 55 additions and 24 deletions.
diff --git a/include/sys/zfs_file.h b/include/sys/zfs_file.h
@@ -29,8 +29,13 @@ typedef struct zfs_file {
 	int f_fd;
 	int f_dump_fd;
 } zfs_file_t;
-#elif defined(__linux__) || defined(__FreeBSD__)
+#elif defined(__linux__)
 typedef struct file zfs_file_t;
+#elif defined(__FreeBSD__)
+typedef struct zfs_file {
+	int zf_fd;
+	struct file *zf_fp;
+} zfs_file_t;
 #else
 #error "unknown OS"
 #endif

diff --git a/module/os/freebsd/zfs/zfs_file_os.c b/module/os/freebsd/zfs/zfs_file_os.c
@@ -47,9 +47,11 @@
 #include <sys/stat.h>
 
 int
-zfs_file_open(const char *path, int flags, int mode, zfs_file_t **fpp)
+zfs_file_open(const char *path, int flags, int mode, zfs_file_t **zfpp)
 {
 	struct thread *td;
+	struct file *fp;
+	zfs_file_t *zfp;
 	int rc, fd;
 
 	td = curthread;
@@ -61,20 +63,31 @@ zfs_file_open(const char *path, int flags, int mode, zfs_file_t **fpp)
 		return (SET_ERROR(rc));
 	fd = td->td_retval[0];
 	td->td_retval[0] = 0;
-	if (fget(curthread, fd, &cap_no_rights, fpp))
+	rc = fget(curthread, fd, &cap_no_rights, &fp);
+	if (rc) {
 		kern_close(td, fd);
+		return (SET_ERROR(rc));
+	}
+	zfp = kmem_alloc(sizeof (*zfp), KM_SLEEP);
+	zfp->zf_fd = fd;
+	zfp->zf_fp = fp;
+	*zfpp = zfp;
 	return (0);
 }
 
 void
-zfs_file_close(zfs_file_t *fp)
+zfs_file_close(zfs_file_t *zfp)
 {
-	fo_close(fp, curthread);
+	struct thread *td = curthread;
+
+	fdrop(zfp->zf_fp, td);
+	kern_close(td, zfp->zf_fd);
+	kmem_free(zfp, sizeof (*zfp));
 }
 
 static int
-zfs_file_write_impl(zfs_file_t *fp, const void *buf, size_t count, loff_t *offp,
-    ssize_t *resid)
+zfs_file_write_impl(struct file *fp, const void *buf, size_t count,
+    loff_t *offp, ssize_t *resid)
 {
 	ssize_t rc;
 	struct uio auio;
@@ -110,8 +123,9 @@ zfs_file_write_impl(zfs_file_t *fp, const void *buf, size_t count, loff_t *offp,
 }
 
 int
-zfs_file_write(zfs_file_t *fp, const void *buf, size_t count, ssize_t *resid)
+zfs_file_write(zfs_file_t *zfp, const void *buf, size_t count, ssize_t *resid)
 {
+	struct file *fp = zfp->zf_fp;
 	loff_t off = fp->f_offset;
 	ssize_t rc;
 
@@ -123,14 +137,14 @@ zfs_file_write(zfs_file_t *fp, const void *buf, size_t count, ssize_t *resid)
 }
 
 int
-zfs_file_pwrite(zfs_file_t *fp, const void *buf, size_t count, loff_t off,
+zfs_file_pwrite(zfs_file_t *zfp, const void *buf, size_t count, loff_t off,
     ssize_t *resid)
 {
-	return (zfs_file_write_impl(fp, buf, count, &off, resid));
+	return (zfs_file_write_impl(zfp->zf_fp, buf, count, &off, resid));
 }
 
 static int
-zfs_file_read_impl(zfs_file_t *fp, void *buf, size_t count, loff_t *offp,
+zfs_file_read_impl(struct file *fp, void *buf, size_t count, loff_t *offp,
     ssize_t *resid)
 {
 	ssize_t rc;
@@ -162,8 +176,9 @@ zfs_file_read_impl(zfs_file_t *fp, void *buf, size_t count, loff_t *offp,
 }
 
 int
-zfs_file_read(zfs_file_t *fp, void *buf, size_t count, ssize_t *resid)
+zfs_file_read(zfs_file_t *zfp, void *buf, size_t count, ssize_t *resid)
 {
+	struct file *fp = zfp->zf_fp;
 	loff_t off = fp->f_offset;
 	ssize_t rc;
 
@@ -174,17 +189,18 @@ zfs_file_read(zfs_file_t *fp, void *buf, size_t count, ssize_t *resid)
 }
 
 int
-zfs_file_pread(zfs_file_t *fp, void *buf, size_t count, loff_t off,
+zfs_file_pread(zfs_file_t *zfp, void *buf, size_t count, loff_t off,
     ssize_t *resid)
 {
-	return (zfs_file_read_impl(fp, buf, count, &off, resid));
+	return (zfs_file_read_impl(zfp->zf_fp, buf, count, &off, resid));
 }
 
 int
-zfs_file_seek(zfs_file_t *fp, loff_t *offp, int whence)
+zfs_file_seek(zfs_file_t *zfp, loff_t *offp, int whence)
 {
-	int rc;
+	struct file *fp = zfp->zf_fp;
 	struct thread *td;
+	int rc;
 
 	td = curthread;
 	if ((fp->f_ops->fo_flags & DFLAG_SEEKABLE) == 0)
@@ -196,8 +212,9 @@ zfs_file_seek(zfs_file_t *fp, loff_t *offp, int whence)
 }
 
 int
-zfs_file_getattr(zfs_file_t *fp, zfs_file_attr_t *zfattr)
+zfs_file_getattr(zfs_file_t *zfp, zfs_file_attr_t *zfattr)
 {
+	struct file *fp = zfp->zf_fp;
 	struct thread *td;
 	struct stat sb;
 	int rc;
@@ -238,8 +255,10 @@ zfs_vop_fsync(vnode_t *vp)
 }
 
 int
-zfs_file_fsync(zfs_file_t *fp, int flags)
+zfs_file_fsync(zfs_file_t *zfp, int flags)
 {
+	struct file *fp = zfp->zf_fp;
+
 	if (fp->f_type != DTYPE_VNODE)
 		return (EINVAL);
 
@@ -249,29 +268,36 @@ zfs_file_fsync(zfs_file_t *fp, int flags)
 zfs_file_t *
 zfs_file_get(int fd)
 {
+	zfs_file_t *zfp;
 	struct file *fp;
 
 	if (fget(curthread, fd, &cap_no_rights, &fp))
 		return (NULL);
 
-	return (fp);
+	zfp = kmem_alloc(sizeof (*zfp), KM_SLEEP);
+	zfp->zf_fd = fd;
+	zfp->zf_fp = fp;
+
+	return (zfp);
 }
 
 void
-zfs_file_put(zfs_file_t *fp)
+zfs_file_put(zfs_file_t *zfp)
 {
-	fdrop(fp, curthread);
+	fdrop(zfp->zf_fp, curthread);
+	kmem_free(zfp, sizeof (*zfp));
 }
 
 loff_t
-zfs_file_off(zfs_file_t *fp)
+zfs_file_off(zfs_file_t *zfp)
 {
-	return (fp->f_offset);
+	return (zfp->zf_fp->f_offset);
 }
 
 void *
-zfs_file_private(zfs_file_t *fp)
+zfs_file_private(zfs_file_t *zfp)
 {
+	struct file *fp = zfp->zf_fp;
 	file_t *tmpfp;
 	void *data;
 	int error;