[PATCH 0/2] Overlayfs tests for file handle bugs

[PATCH 0/2] Overlayfs tests for file handle bugs

[Amir Goldstein]

Eryu,

Recently, two independent bugs have been reported by Lubos Dolezel
and Dan Carpenter.

The first bug was there since overlayfs nfs_export support.  The second
bug is a regression introduced in v5.5 that can crash the kernel.

The fix patches have just been posted, so the purpose of this posting
it to test the fixes pre merge.

Thanks,
Amir.

Amir Goldstein (2):
  open_by_handle: add option -z to query file handle size
  overlay: regression test for two file handle bugs

 src/open_by_handle.c  | 27 +++++++++++++--
 tests/overlay/073     | 80 +++++++++++++++++++++++++++++++++++++++++++
 tests/overlay/073.out |  2 ++
 tests/overlay/group   |  1 +
 4 files changed, 107 insertions(+), 3 deletions(-)
 create mode 100755 tests/overlay/073
 create mode 100644 tests/overlay/073.out

-- 
2.17.1

[PATCH 1/2] open_by_handle: add option -z to query file handle size

[Amir Goldstein]

Instead of using MAX_HANDLE_SZ, query the filesystem buffer size
and use that buffer size to get the file handle.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 src/open_by_handle.c | 27 ++++++++++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

diff --git a/src/open_by_handle.c b/src/open_by_handle.c
index 4fdfacd7..0f74ed08 100644
--- a/src/open_by_handle.c
+++ b/src/open_by_handle.c
@@ -114,6 +114,7 @@ void usage(void)
 	fprintf(stderr, "open_by_handle -i <handles_file> <test_dir> [N] - read test files handles from file and try to open by handle\n");
 	fprintf(stderr, "open_by_handle -o <handles_file> <test_dir> [N] - get file handles of test files and write handles to file\n");
 	fprintf(stderr, "open_by_handle -s <test_dir> [N] - wait in sleep loop after opening files by handle to keep them open\n");
+	fprintf(stderr, "open_by_handle -z <test_dir> [N] - query filesystem required buffer size\n");
 	exit(EXIT_FAILURE);
 }
 
@@ -136,11 +137,12 @@ int main(int argc, char **argv)
 	int	create = 0, delete = 0, nlink = 1, move = 0;
 	int	rd = 0, wr = 0, wrafter = 0, parent = 0;
 	int	keepopen = 0, drop_caches = 1, sleep_loop = 0;
+	int	bufsz = MAX_HANDLE_SZ;
 
 	if (argc < 2)
 		usage();
 
-	while ((c = getopt(argc, argv, "cludmrwapknhi:o:s")) != -1) {
+	while ((c = getopt(argc, argv, "cludmrwapknhi:o:sz")) != -1) {
 		switch (c) {
 		case 'c':
 			create = 1;
@@ -199,6 +201,9 @@ int main(int argc, char **argv)
 		case 's':
 			sleep_loop = 1;
 			break;
+		case 'z':
+			bufsz = 0;
+			break;
 		default:
 			fprintf(stderr, "illegal option '%s'\n", argv[optind]);
 		case 'h':
@@ -300,8 +305,16 @@ int main(int argc, char **argv)
 				return EXIT_FAILURE;
 			}
 		} else {
-			handle[i].fh.handle_bytes = MAX_HANDLE_SZ;
+			handle[i].fh.handle_bytes = bufsz;
 			ret = name_to_handle_at(AT_FDCWD, fname, &handle[i].fh, &mount_id, 0);
+			if (bufsz < handle[i].fh.handle_bytes) {
+				/* Query the filesystem required bufsz and the file handle */
+				if (ret != -1 || errno != EOVERFLOW) {
+					fprintf(stderr, "Unexpected result from name_to_handle_at(%s)\n", fname);
+					return EXIT_FAILURE;
+				}
+				ret = name_to_handle_at(AT_FDCWD, fname, &handle[i].fh, &mount_id, 0);
+			}
 			if (ret < 0) {
 				strcat(fname, ": name_to_handle");
 				perror(fname);
@@ -334,8 +347,16 @@ int main(int argc, char **argv)
 				return EXIT_FAILURE;
 			}
 		} else {
-			dir_handle.fh.handle_bytes = MAX_HANDLE_SZ;
+			dir_handle.fh.handle_bytes = bufsz;
 			ret = name_to_handle_at(AT_FDCWD, test_dir, &dir_handle.fh, &mount_id, 0);
+			if (bufsz < dir_handle.fh.handle_bytes) {
+				/* Query the filesystem required bufsz and the file handle */
+				if (ret != -1 || errno != EOVERFLOW) {
+					fprintf(stderr, "Unexpected result from name_to_handle_at(%s)\n", dname);
+					return EXIT_FAILURE;
+				}
+				ret = name_to_handle_at(AT_FDCWD, test_dir, &dir_handle.fh, &mount_id, 0);
+			}
 			if (ret < 0) {
 				strcat(dname, ": name_to_handle");
 				perror(dname);
-- 
2.17.1

[PATCH 2/2] overlay: regression test for two file handle bugs

[Amir Goldstein]

Test two overlayfs file handle bugs:

 1. Failure to query file handle size
    Fixed by kernel commit:
        ovl: return required buffer size for file handles

 2. Kernel OOPS on open by hand crafted malformed file handle
    Fixed by kernel commit:
        ovl: potential crash in ovl_fid_to_fh()

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 tests/overlay/073     | 80 +++++++++++++++++++++++++++++++++++++++++++
 tests/overlay/073.out |  2 ++
 tests/overlay/group   |  1 +
 3 files changed, 83 insertions(+)
 create mode 100755 tests/overlay/073
 create mode 100644 tests/overlay/073.out

diff --git a/tests/overlay/073 b/tests/overlay/073
new file mode 100755
index 00000000..72233fae
--- /dev/null
+++ b/tests/overlay/073
@@ -0,0 +1,80 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (C) 2020 CTERA Networks. All Rights Reserved.
+#
+# FS QA Test No. 073
+#
+# Test two overlayfs file handle bugs:
+# 1. Failure to query file handle size
+#    Fixed by kernel commit:
+#        ovl: return required buffer size for file handles
+#
+# 2. Kernel OOPS on open by hand crafted malformed file handle
+#    Fixed by kernel commit:
+#        ovl: potential crash in ovl_fid_to_fh()
+#
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1	# failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	cd /
+	#rm -f $tmp.*
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+
+# real QA test starts here
+
+_supported_fs overlay
+_supported_os Linux
+_require_scratch
+_require_test_program "open_by_handle"
+# We need to require all features together, because nfs_export cannot
+# be enabled when index is disabled
+_require_scratch_overlay_features index nfs_export
+
+rm -f $seqres.full
+
+_scratch_mkfs
+_scratch_mount -o "index=on,nfs_export=on"
+
+testdir=$SCRATCH_MNT/testdir
+
+# Create directory with test file
+$here/src/open_by_handle -cp $testdir
+
+# Test query file handle size on dir and file
+$here/src/open_by_handle -pz $testdir
+
+# Export file handle into tmp file
+$here/src/open_by_handle -o $tmp.file_handle $testdir
+
+# Verify open by exported file handle
+$here/src/open_by_handle -i $tmp.file_handle $testdir
+
+# Mangle the exported file handle:
+# handle_bytes = 1
+# handle_type = OVL_FILEID_V0 (0xfb)
+# File handle is encoded in host order
+# The command below crafts this header for little endian.
+# On different big endian architectures the file handle will still
+# be malformed just not with the specific values to trigger the bug
+$XFS_IO_PROG -c "pwrite -S 0 0 8" -c "pwrite -S 1 0 1" -c "pwrite -S 0xfb 4 1" $tmp.file_handle >> $seqres.full
+
+# Verify failure to open by mangled file handle
+# This will trigger NULL pointer dereference on affected kernels
+$here/src/open_by_handle -i $tmp.file_handle $testdir 2>> $seqres.full && \
+	_fail "open by mangaled file handle is expected to fail"
+
+echo "Silence is golden"
+status=0
+exit
diff --git a/tests/overlay/073.out b/tests/overlay/073.out
new file mode 100644
index 00000000..d107704c
--- /dev/null
+++ b/tests/overlay/073.out
@@ -0,0 +1,2 @@
+QA output created by 073
+Silence is golden
diff --git a/tests/overlay/group b/tests/overlay/group
index 82876d09..5625a46d 100644
--- a/tests/overlay/group
+++ b/tests/overlay/group
@@ -75,3 +75,4 @@
 070 auto quick copyup redirect nested
 071 auto quick copyup redirect nested nonsamefs
 072 auto quick copyup hardlink
+073 auto quick exportfs dangerous
-- 
2.17.1