diff mbox series

[v2] generic: new test to verify selinux label of whiteout inode

Message ID 20220725061327.266746-1-zlang@kernel.org (mailing list archive)
State New, archived
Headers show
Series [v2] generic: new test to verify selinux label of whiteout inode | expand

Commit Message

Zorro Lang July 25, 2022, 6:13 a.m. UTC 
A but on XFS cause renameat2() with flags=RENAME_WHITEOUT doesn't
apply an selinux label. That's quite different with other fs (e.g.
ext4, tmpfs).

Signed-off-by: Zorro Lang <zlang@kernel.org>
---

Thanks the review points from Amir, this v2 did below changes:
1) Add "whiteout" group
2) Add commit ID from xfs-linux xfs-5.20-merge-2 (will change if need)
3) Rebase to latest fstests for-next branch

Thanks,
Zorro

 tests/generic/693     | 64 +++++++++++++++++++++++++++++++++++++++++++
 tests/generic/693.out |  2 ++
 2 files changed, 66 insertions(+)
 create mode 100755 tests/generic/693
 create mode 100644 tests/generic/693.out

Comments

Zorro Lang Aug. 5, 2022, 5:18 p.m. UTC | #1 
On Mon, Jul 25, 2022 at 02:13:27PM +0800, Zorro Lang wrote:
> A but on XFS cause renameat2() with flags=RENAME_WHITEOUT doesn't
> apply an selinux label. That's quite different with other fs (e.g.
> ext4, tmpfs).
> 
> Signed-off-by: Zorro Lang <zlang@kernel.org>
> ---

Ping, any review poings for this patch? The bug fix has been merged into
mainline kernel:
  70b589a37e1a ("xfs: add selinux labels to whiteout inodes")

Thanks,
Zorro

> 
> Thanks the review points from Amir, this v2 did below changes:
> 1) Add "whiteout" group
> 2) Add commit ID from xfs-linux xfs-5.20-merge-2 (will change if need)
> 3) Rebase to latest fstests for-next branch
> 
> Thanks,
> Zorro
> 
>  tests/generic/693     | 64 +++++++++++++++++++++++++++++++++++++++++++
>  tests/generic/693.out |  2 ++
>  2 files changed, 66 insertions(+)
>  create mode 100755 tests/generic/693
>  create mode 100644 tests/generic/693.out
> 
> diff --git a/tests/generic/693 b/tests/generic/693
> new file mode 100755
> index 00000000..adf191c4
> --- /dev/null
> +++ b/tests/generic/693
> @@ -0,0 +1,64 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2022 Red Hat, Copyright.  All Rights Reserved.
> +#
> +# FS QA Test No. 693
> +#
> +# Verify selinux label can be kept after RENAME_WHITEOUT. This is
> +# a regression test for:
> +#   70b589a37e1a ("xfs: add selinux labels to whiteout inodes")
> +#
> +. ./common/preamble
> +_begin_fstest auto quick rename attr whiteout
> +
> +# Import common functions.
> +. ./common/attr
> +. ./common/renameat2
> +
> +# real QA test starts here
> +_supported_fs generic
> +_require_scratch
> +_require_attrs
> +_require_renameat2 whiteout
> +
> +_fixed_by_kernel_commit 70b589a37e1a \
> +	xfs: add selinux labels to whiteout inodes
> +
> +get_selinux_label()
> +{
> +	local label
> +
> +	label=`_getfattr --absolute-names -n security.selinux $@ | sed -n 's/security.selinux=\"\(.*\)\"/\1/p'`
> +	if [ ${PIPESTATUS[0]} -ne 0 -o -z "$label" ];then
> +		_fail "Fail to get selinux label: $label"
> +	fi
> +	echo $label
> +}
> +
> +_scratch_mkfs >> $seqres.full 2>&1
> +# SELINUX_MOUNT_OPTIONS will be set in common/config if selinux is enabled
> +if [ -z "$SELINUX_MOUNT_OPTIONS" ]; then
> +	_notrun "Require selinux to be enabled"
> +fi
> +# This test need to verify selinux labels in objects, so unset this selinux
> +# mount option
> +export SELINUX_MOUNT_OPTIONS=""
> +_scratch_mount
> +
> +touch $SCRATCH_MNT/f1
> +echo "Before RENAME_WHITEOUT" >> $seqres.full
> +ls -lZ $SCRATCH_MNT >> $seqres.full 2>&1
> +# Expect f1 and f2 have same label after RENAME_WHITEOUT
> +$here/src/renameat2 -w $SCRATCH_MNT/f1 $SCRATCH_MNT/f2
> +echo "After RENAME_WHITEOUT" >> $seqres.full
> +ls -lZ $SCRATCH_MNT >> $seqres.full 2>&1
> +label1=`get_selinux_label $SCRATCH_MNT/f1`
> +label2=`get_selinux_label $SCRATCH_MNT/f2`
> +if [ "$label1" != "$label2" ];then
> +	echo "$label1 != $label2"
> +fi
> +
> +echo "Silence is golden"
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/generic/693.out b/tests/generic/693.out
> new file mode 100644
> index 00000000..01884ea5
> --- /dev/null
> +++ b/tests/generic/693.out
> @@ -0,0 +1,2 @@
> +QA output created by 693
> +Silence is golden
> -- 
> 2.31.1
>
Darrick J. Wong Aug. 23, 2022, 2:43 p.m. UTC | #2 
On Mon, Jul 25, 2022 at 02:13:27PM +0800, Zorro Lang wrote:
> A but on XFS cause renameat2() with flags=RENAME_WHITEOUT doesn't
> apply an selinux label. That's quite different with other fs (e.g.
> ext4, tmpfs).
> 
> Signed-off-by: Zorro Lang <zlang@kernel.org>
> ---
> 
> Thanks the review points from Amir, this v2 did below changes:
> 1) Add "whiteout" group
> 2) Add commit ID from xfs-linux xfs-5.20-merge-2 (will change if need)
> 3) Rebase to latest fstests for-next branch
> 
> Thanks,
> Zorro
> 
>  tests/generic/693     | 64 +++++++++++++++++++++++++++++++++++++++++++
>  tests/generic/693.out |  2 ++
>  2 files changed, 66 insertions(+)
>  create mode 100755 tests/generic/693
>  create mode 100644 tests/generic/693.out
> 
> diff --git a/tests/generic/693 b/tests/generic/693
> new file mode 100755
> index 00000000..adf191c4
> --- /dev/null
> +++ b/tests/generic/693
> @@ -0,0 +1,64 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2022 Red Hat, Copyright.  All Rights Reserved.
> +#
> +# FS QA Test No. 693
> +#
> +# Verify selinux label can be kept after RENAME_WHITEOUT. This is
> +# a regression test for:
> +#   70b589a37e1a ("xfs: add selinux labels to whiteout inodes")
> +#
> +. ./common/preamble
> +_begin_fstest auto quick rename attr whiteout
> +
> +# Import common functions.
> +. ./common/attr
> +. ./common/renameat2
> +
> +# real QA test starts here
> +_supported_fs generic
> +_require_scratch
> +_require_attrs
> +_require_renameat2 whiteout
> +
> +_fixed_by_kernel_commit 70b589a37e1a \
> +	xfs: add selinux labels to whiteout inodes
> +
> +get_selinux_label()
> +{
> +	local label
> +
> +	label=`_getfattr --absolute-names -n security.selinux $@ | sed -n 's/security.selinux=\"\(.*\)\"/\1/p'`
> +	if [ ${PIPESTATUS[0]} -ne 0 -o -z "$label" ];then
> +		_fail "Fail to get selinux label: $label"
> +	fi
> +	echo $label
> +}
> +
> +_scratch_mkfs >> $seqres.full 2>&1
> +# SELINUX_MOUNT_OPTIONS will be set in common/config if selinux is enabled
> +if [ -z "$SELINUX_MOUNT_OPTIONS" ]; then
> +	_notrun "Require selinux to be enabled"
> +fi
> +# This test need to verify selinux labels in objects, so unset this selinux
> +# mount option
> +export SELINUX_MOUNT_OPTIONS=""
> +_scratch_mount
> +
> +touch $SCRATCH_MNT/f1
> +echo "Before RENAME_WHITEOUT" >> $seqres.full
> +ls -lZ $SCRATCH_MNT >> $seqres.full 2>&1
> +# Expect f1 and f2 have same label after RENAME_WHITEOUT
> +$here/src/renameat2 -w $SCRATCH_MNT/f1 $SCRATCH_MNT/f2
> +echo "After RENAME_WHITEOUT" >> $seqres.full
> +ls -lZ $SCRATCH_MNT >> $seqres.full 2>&1
> +label1=`get_selinux_label $SCRATCH_MNT/f1`
> +label2=`get_selinux_label $SCRATCH_MNT/f2`

The operations of this test look ok to me, but the piece I do not know
is the higher level context of whether or not it's appropriate for
whiteout inodes to have selinux labels, or if the selinux developers
even care.  Perhaps they should be cc'd?  (And maybe I should've made
Eric do that for the kernel patch...sigh...)

--D

> +if [ "$label1" != "$label2" ];then
> +	echo "$label1 != $label2"
> +fi
> +
> +echo "Silence is golden"
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/generic/693.out b/tests/generic/693.out
> new file mode 100644
> index 00000000..01884ea5
> --- /dev/null
> +++ b/tests/generic/693.out
> @@ -0,0 +1,2 @@
> +QA output created by 693
> +Silence is golden
> -- 
> 2.31.1
>
Zorro Lang Aug. 26, 2022, 10:40 a.m. UTC | #3 
On Tue, Aug 23, 2022 at 07:43:54AM -0700, Darrick J. Wong wrote:
> On Mon, Jul 25, 2022 at 02:13:27PM +0800, Zorro Lang wrote:
> > A but on XFS cause renameat2() with flags=RENAME_WHITEOUT doesn't
> > apply an selinux label. That's quite different with other fs (e.g.
> > ext4, tmpfs).
> > 
> > Signed-off-by: Zorro Lang <zlang@kernel.org>
> > ---
> > 
> > Thanks the review points from Amir, this v2 did below changes:
> > 1) Add "whiteout" group
> > 2) Add commit ID from xfs-linux xfs-5.20-merge-2 (will change if need)
> > 3) Rebase to latest fstests for-next branch
> > 
> > Thanks,
> > Zorro
> > 
> >  tests/generic/693     | 64 +++++++++++++++++++++++++++++++++++++++++++
> >  tests/generic/693.out |  2 ++
> >  2 files changed, 66 insertions(+)
> >  create mode 100755 tests/generic/693
> >  create mode 100644 tests/generic/693.out
> > 
> > diff --git a/tests/generic/693 b/tests/generic/693
> > new file mode 100755
> > index 00000000..adf191c4
> > --- /dev/null
> > +++ b/tests/generic/693
> > @@ -0,0 +1,64 @@
> > +#! /bin/bash
> > +# SPDX-License-Identifier: GPL-2.0
> > +# Copyright (c) 2022 Red Hat, Copyright.  All Rights Reserved.
> > +#
> > +# FS QA Test No. 693
> > +#
> > +# Verify selinux label can be kept after RENAME_WHITEOUT. This is
> > +# a regression test for:
> > +#   70b589a37e1a ("xfs: add selinux labels to whiteout inodes")
> > +#
> > +. ./common/preamble
> > +_begin_fstest auto quick rename attr whiteout
> > +
> > +# Import common functions.
> > +. ./common/attr
> > +. ./common/renameat2
> > +
> > +# real QA test starts here
> > +_supported_fs generic
> > +_require_scratch
> > +_require_attrs
> > +_require_renameat2 whiteout
> > +
> > +_fixed_by_kernel_commit 70b589a37e1a \
> > +	xfs: add selinux labels to whiteout inodes
> > +
> > +get_selinux_label()
> > +{
> > +	local label
> > +
> > +	label=`_getfattr --absolute-names -n security.selinux $@ | sed -n 's/security.selinux=\"\(.*\)\"/\1/p'`
> > +	if [ ${PIPESTATUS[0]} -ne 0 -o -z "$label" ];then
> > +		_fail "Fail to get selinux label: $label"
> > +	fi
> > +	echo $label
> > +}
> > +
> > +_scratch_mkfs >> $seqres.full 2>&1
> > +# SELINUX_MOUNT_OPTIONS will be set in common/config if selinux is enabled
> > +if [ -z "$SELINUX_MOUNT_OPTIONS" ]; then
> > +	_notrun "Require selinux to be enabled"
> > +fi
> > +# This test need to verify selinux labels in objects, so unset this selinux
> > +# mount option
> > +export SELINUX_MOUNT_OPTIONS=""
> > +_scratch_mount
> > +
> > +touch $SCRATCH_MNT/f1
> > +echo "Before RENAME_WHITEOUT" >> $seqres.full
> > +ls -lZ $SCRATCH_MNT >> $seqres.full 2>&1
> > +# Expect f1 and f2 have same label after RENAME_WHITEOUT
> > +$here/src/renameat2 -w $SCRATCH_MNT/f1 $SCRATCH_MNT/f2
> > +echo "After RENAME_WHITEOUT" >> $seqres.full
> > +ls -lZ $SCRATCH_MNT >> $seqres.full 2>&1
> > +label1=`get_selinux_label $SCRATCH_MNT/f1`
> > +label2=`get_selinux_label $SCRATCH_MNT/f2`
> 
> The operations of this test look ok to me, but the piece I do not know
> is the higher level context of whether or not it's appropriate for
> whiteout inodes to have selinux labels, or if the selinux developers
> even care.  Perhaps they should be cc'd?  (And maybe I should've made
> Eric do that for the kernel patch...sigh...)

Hi Eric,

May you help to review this patch :)

Thanks,
Zorro

> 
> --D
> 
> > +if [ "$label1" != "$label2" ];then
> > +	echo "$label1 != $label2"
> > +fi
> > +
> > +echo "Silence is golden"
> > +# success, all done
> > +status=0
> > +exit
> > diff --git a/tests/generic/693.out b/tests/generic/693.out
> > new file mode 100644
> > index 00000000..01884ea5
> > --- /dev/null
> > +++ b/tests/generic/693.out
> > @@ -0,0 +1,2 @@
> > +QA output created by 693
> > +Silence is golden
> > -- 
> > 2.31.1
> > 
>
diff mbox series

Patch

diff --git a/tests/generic/693 b/tests/generic/693
new file mode 100755
index 00000000..adf191c4
--- /dev/null
+++ b/tests/generic/693
@@ -0,0 +1,64 @@ 
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2022 Red Hat, Copyright.  All Rights Reserved.
+#
+# FS QA Test No. 693
+#
+# Verify selinux label can be kept after RENAME_WHITEOUT. This is
+# a regression test for:
+#   70b589a37e1a ("xfs: add selinux labels to whiteout inodes")
+#
+. ./common/preamble
+_begin_fstest auto quick rename attr whiteout
+
+# Import common functions.
+. ./common/attr
+. ./common/renameat2
+
+# real QA test starts here
+_supported_fs generic
+_require_scratch
+_require_attrs
+_require_renameat2 whiteout
+
+_fixed_by_kernel_commit 70b589a37e1a \
+	xfs: add selinux labels to whiteout inodes
+
+get_selinux_label()
+{
+	local label
+
+	label=`_getfattr --absolute-names -n security.selinux $@ | sed -n 's/security.selinux=\"\(.*\)\"/\1/p'`
+	if [ ${PIPESTATUS[0]} -ne 0 -o -z "$label" ];then
+		_fail "Fail to get selinux label: $label"
+	fi
+	echo $label
+}
+
+_scratch_mkfs >> $seqres.full 2>&1
+# SELINUX_MOUNT_OPTIONS will be set in common/config if selinux is enabled
+if [ -z "$SELINUX_MOUNT_OPTIONS" ]; then
+	_notrun "Require selinux to be enabled"
+fi
+# This test need to verify selinux labels in objects, so unset this selinux
+# mount option
+export SELINUX_MOUNT_OPTIONS=""
+_scratch_mount
+
+touch $SCRATCH_MNT/f1
+echo "Before RENAME_WHITEOUT" >> $seqres.full
+ls -lZ $SCRATCH_MNT >> $seqres.full 2>&1
+# Expect f1 and f2 have same label after RENAME_WHITEOUT
+$here/src/renameat2 -w $SCRATCH_MNT/f1 $SCRATCH_MNT/f2
+echo "After RENAME_WHITEOUT" >> $seqres.full
+ls -lZ $SCRATCH_MNT >> $seqres.full 2>&1
+label1=`get_selinux_label $SCRATCH_MNT/f1`
+label2=`get_selinux_label $SCRATCH_MNT/f2`
+if [ "$label1" != "$label2" ];then
+	echo "$label1 != $label2"
+fi
+
+echo "Silence is golden"
+# success, all done
+status=0
+exit
diff --git a/tests/generic/693.out b/tests/generic/693.out
new file mode 100644
index 00000000..01884ea5
--- /dev/null
+++ b/tests/generic/693.out
@@ -0,0 +1,2 @@ 
+QA output created by 693
+Silence is golden