diff mbox series

[1/8] libbtrfsutil: fix accidentally closing fd passed to subvolume iterator

Message ID 85517de7964d1e062789d024a803bcb7ad415b1c.1718995160.git.osandov@fb.com (mailing list archive)
State New, archived
Headers show
Series btrfs-progs: add subvol list options for sane path behavior | expand

Commit Message

Omar Sandoval June 21, 2024, 6:53 p.m. UTC 
From: Omar Sandoval <osandov@fb.com>

For an unprivileged subvolume iterator, append_to_search_stack() closes
cur_fd. On the first call to btrfs_util_subvolume_iterator_next(),
cur_fd is equal to the fd that was passed to
btrfs_util_create_subvolume_iterator_fd(). We're not supposed to close
that. We didn't notice it because it's more common to use it through
btrfs_util_create_subvolume_iterator(), which opens its own fd that
should be closed, and because the fd number is often reused internally
by the subvolume iterator.

pop_search_stack() already has a check to avoid closing the passed fd;
add the same check to append_to_search_stack(). Also add a regression
test.

Signed-off-by: Omar Sandoval <osandov@fb.com>
---
 libbtrfsutil/python/tests/test_subvolume.py | 18 ++++++++++++++++++
 libbtrfsutil/subvolume.c                    |  3 ++-
 2 files changed, 20 insertions(+), 1 deletion(-)
diff mbox series

Patch

diff --git a/libbtrfsutil/python/tests/test_subvolume.py b/libbtrfsutil/python/tests/test_subvolume.py
index 690ff107..1e7df00e 100644
--- a/libbtrfsutil/python/tests/test_subvolume.py
+++ b/libbtrfsutil/python/tests/test_subvolume.py
@@ -561,3 +561,21 @@  class TestSubvolume(BtrfsTestCase):
                 self._test_subvolume_iterator_race()
         finally:
             os.chdir(pwd)
+
+    def test_subvolume_iterator_fd_unprivileged(self):
+        pwd = os.getcwd()
+        try:
+            os.chdir(self.mountpoint)
+            btrfsutil.create_subvolume('subvol')
+            with drop_privs():
+                fd = os.open('.', os.O_RDONLY | os.O_DIRECTORY)
+                try:
+                    with btrfsutil.SubvolumeIterator(fd) as it:
+                        for _ in it:
+                            break
+                finally:
+                    # A bug in SubvolumeIterator previously made it close the
+                    # passed in fd, so this would fail with EBADF.
+                    os.close(fd)
+        finally:
+            os.chdir(pwd)
diff --git a/libbtrfsutil/subvolume.c b/libbtrfsutil/subvolume.c
index 6b2f2671..1f0f2d2b 100644
--- a/libbtrfsutil/subvolume.c
+++ b/libbtrfsutil/subvolume.c
@@ -930,7 +930,8 @@  static enum btrfs_util_error append_to_search_stack(struct btrfs_util_subvolume_
 				return err;
 			}
 
-			close(iter->cur_fd);
+			if (iter->cur_fd != iter->fd)
+				close(iter->cur_fd);
 			iter->cur_fd = fd;
 		}
 	}