From a225507009a3bbb7bdf1a701efc0d2e11e334594 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Mon, 14 Dec 2020 15:47:20 -0800 Subject: [PATCH 1/1] generic: test for creating duplicate filenames in encrypted dir Test for a race condition where a duplicate filename could be created in an encrypted directory while the directory's encryption key was being added concurrently. generic/595 was already failing on ubifs due to this bug, but only by accident. This new test detects the bug on both ext4 and ubifs. I wasn't able to get it to detect the bug on f2fs. Signed-off-by: Eric Biggers --- tests/generic/621 | 166 ++++++++++++++++++++++++++++++++++++++++++ tests/generic/621.out | 21 ++++++ tests/generic/group | 1 + 3 files changed, 188 insertions(+) create mode 100755 tests/generic/621 create mode 100644 tests/generic/621.out diff --git a/tests/generic/621 b/tests/generic/621 new file mode 100755 index 00000000..989bd7ce --- /dev/null +++ b/tests/generic/621 @@ -0,0 +1,166 @@ +#! /bin/bash +# SPDX-License-Identifier: GPL-2.0 +# Copyright 2020 Google LLC +# +# FS QA Test No. 621 +# +# Test for a race condition where a duplicate filename could be created in an +# encrypted directory while the directory's encryption key was being added +# concurrently. This is a regression test for the following kernel commits: +# +# 968dd6d0c6d6 ("fscrypt: fix race allowing rename() and link() of ciphertext dentries") +# 75d18cd1868c ("ext4: prevent creating duplicate encrypted filenames") +# bfc2b7e85189 ("f2fs: prevent creating duplicate encrypted filenames") +# 76786a0f0834 ("ubifs: prevent creating duplicate encrypted filenames") +# +# The first commit fixed the bug for the rename() and link() syscalls. +# The others fixed the bug for the other syscalls that create new filenames. +# +# Note, the bug wasn't actually reproducible on f2fs. +# +# The race condition worked as follows: +# 1. Initial state: an encrypted directory "dir" contains a file "foo", +# but the directory's key hasn't been added yet so 'ls dir' shows an +# encoded no-key name rather than "foo". +# 2. The key is added concurrently with mkdir("dir/foo") or another syscall +# that creates a new filename and should fail if it already exists. +# a. The syscall looks up "dir/foo", creating a negative no-key dentry +# for "foo" since the directory's key hasn't been added yet. +# b. The directory's key is added. +# c. The syscall does the actual fs-level operation to create the +# filename. With the bug, the filesystem failed to detect that the +# dentry was created without the key, potentially causing the +# operation to unexpectedly succeed and add a duplicate filename. +# +# To test this, we try to reproduce the above race. Afterwards we check for +# duplicate filenames, plus a few other things. +# +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() +{ + touch $tmp.done + wait + rm -f $tmp.* +} + +. ./common/rc +. ./common/filter +. ./common/encrypt +. ./common/renameat2 + +rm -f $seqres.full + +_supported_fs generic +_require_scratch_encryption -v 2 +_require_renameat2 noreplace + +_scratch_mkfs_encrypted &>> $seqres.full +_scratch_mount + +runtime=$((5 * TIME_FACTOR)) +dir=$SCRATCH_MNT/dir + +echo -e "\n# Creating encrypted directory containing files" +mkdir $dir +_add_enckey $SCRATCH_MNT "$TEST_RAW_KEY" +_set_encpolicy $dir $TEST_KEY_IDENTIFIER +for i in {1..100}; do + touch $dir/$i +done + +# This is the filename which we'll try to duplicate. +inode=$(stat -c %i $dir/100) + +# ext4 checks for duplicate dentries when inserting one, which can hide the bug +# this test is testing for. However, ext4 stops checking for duplicates once it +# finds space for the new dentry. Therefore, we can circumvent ext4's duplicate +# checking by creating space at the beginning of the directory block. +rm $dir/1 + +echo -e "\n# Starting duplicate filename creator process" +( + # Repeatedly try to create the filename $dir/100 (which already exists) + # using syscalls that should fail if the file already exists: mkdir(), + # mknod(), symlink(), link(), and renameat2(RENAME_NOREPLACE). This + # hopefully detects any one of them having the bug. TODO: we should + # also try open(O_EXCL|O_CREAT), but it needs a command-line tool. + while [ ! -e $tmp.done ]; do + if mkdir $dir/100 &> /dev/null; then + touch $tmp.mkdir_succeeded + fi + if mknod $dir/100 c 5 5 &> /dev/null; then + touch $tmp.mknod_succeeded + fi + if ln -s target $dir/100 &> /dev/null; then + touch $tmp.symlink_succeeded + fi + if ln $dir/50 $dir/100 &> /dev/null; then + touch $tmp.link_succeeded + fi + if $here/src/renameat2 -n $dir/50 $dir/100 &> /dev/null; then + touch $tmp.rename_noreplace_succeeded + fi + done +) & + +echo -e "\n# Starting add/remove enckey process" +( + # Repeatedly add and remove the encryption key for $dir. The actual + # race this test is trying to reproduce occurs when adding the key. + while [ ! -e $tmp.done ]; do + _add_enckey $SCRATCH_MNT "$TEST_RAW_KEY" > /dev/null + _rm_enckey $SCRATCH_MNT $TEST_KEY_IDENTIFIER > /dev/null + done +) & + +echo -e "\n# Running for a few seconds..." +sleep $runtime +echo -e "\n# Stopping subprocesses" +touch $tmp.done +wait + +_add_enckey $SCRATCH_MNT "$TEST_RAW_KEY" > /dev/null + +# Check for failure in several different ways, since different ways work on +# different filesystems. E.g. ext4 shows duplicate filenames but ubifs doesn't. + +echo -e "\n# Checking for duplicate filenames via readdir" +ls $dir | grep 100 + +echo -e "\n# Checking for unexpected change in inode number" +new_inode=$(stat -c %i $dir/100) +if [ $new_inode != $inode ]; then + echo "Dentry changed inode number $inode => $new_inode!" +fi + +echo -e "\n# Checking for operations that unexpectedly succeeded on an existing filename" +for op in "mkdir" "mknod" "symlink" "link" "rename_noreplace"; do + if [ -e $tmp.${op}_succeeded ]; then + echo "$op operation(s) on existing filename unexpectedly succeeded!" + fi +done + +# Also check that the fsck program can't find any duplicate filenames. +# For ext4, override _check_scratch_fs() so that we can specify -D (optimize +# directories); otherwise e2fsck doesn't check for duplicate filenames. +echo -e "\n# Checking for duplicate filenames via fsck" +_scratch_unmount +if [ "$FSTYP" = ext4 ]; then + if ! e2fsck -f -y -D $SCRATCH_DEV &>> $seqres.full; then + _log_err "filesystem on $SCRATCH_DEV is inconsistent" + fi +else + _check_scratch_fs +fi + +# success, all done +status=0 +exit diff --git a/tests/generic/621.out b/tests/generic/621.out new file mode 100644 index 00000000..f43f7ff8 --- /dev/null +++ b/tests/generic/621.out @@ -0,0 +1,21 @@ +QA output created by 621 + +# Creating encrypted directory containing files +Added encryption key with identifier 69b2f6edeee720cce0577937eb8a6751 + +# Starting duplicate filename creator process + +# Starting add/remove enckey process + +# Running for a few seconds... + +# Stopping subprocesses + +# Checking for duplicate filenames via readdir +100 + +# Checking for unexpected change in inode number + +# Checking for operations that unexpectedly succeeded on an existing filename + +# Checking for duplicate filenames via fsck diff --git a/tests/generic/group b/tests/generic/group index d8758d7f..fec35d8e 100644 --- a/tests/generic/group +++ b/tests/generic/group @@ -623,3 +623,4 @@ 618 auto quick attr 619 auto rw enospc 620 auto mount quick +621 auto quick encrypt -- 2.30.2