Whamcloud - gitweb
LU-8656 vvp: Add S_NOSEC support 53/22853/15
authorPatrick Farrell <paf@cray.com>
Sat, 3 Jun 2017 15:20:10 +0000 (11:20 -0400)
committerOleg Drokin <oleg.drokin@intel.com>
Sat, 10 Jun 2017 02:49:31 +0000 (02:49 +0000)
commit8bc4b26453fb9d3296475ab3b9c9f3a9bf905afc
tree250f26bb91c0529ab1c07b3b9fbbf6c42ab2ff61
parent2aabe690e90306628b779916908afd701f2396de
LU-8656 vvp: Add S_NOSEC support

We must use the i_mutex to protect permission changes,
which means we need to take it when we write to a file with
the setuid or setgid bit set (as this removes those bits).

LU-8025 attempted to use IS_NOSEC to check for this case,
but did not actually add support for the S_NOSEC flag to
Lustre.

S_NOSEC was added in upstream kernel commit:
69b4573296469fd3f70cf7044693074980517067
But a key change requiring parallel filesystems to opt in
with a superblock flag was added in:
9e1f1de02c2275d7172e18dc4e7c2065777611bf

This patch adds the required support.

Specifically, Lustre should set S_NOSEC correctly when an
inode is created (ll_iget), but only for new inodes.
Setting it for existing inodes requires taking the i_mutex,
creating an unacceptable metadata performance impact in the
lookup code.

Existing inodes get S_NOSEC set either by a setattr call
(see below), or by the first writer to write to the node,
in file_remove_privs/file_remove_suid.  So it's OK not to
set S_NOSEC on all inodes in ll_iget.

Also, Lustre must clear S_NOSEC when it gets a metadata
update because another node could have changed permissions.
i_flags is already cleared in ll_update_inode, but we would
prefer to have S_NOSEC set whenever possible, so we want to
re-do the check after the update.

This requires holding the i_mutex to avoid a check/set race
with permissions changes.

We cannot easily take the i_mutex in ll_update_inode (it is
called from too many places, some of which already hold the
i_mutex).

It is acceptable to sometimes not have S_NOSEC set (since
occasionally taking the i_mutex when not needed is OK), so
we look at getting it set most of the time.

It looks like the primary concern is ll_md_setattr.  The
caller (ll_setattr_raw) takes the i_mutex before returning,
so we do the relevant call there.

This opens a window during which S_NOSEC is not set, but
again, this merely creates a situation where we take the
i_mutex unnecessarily, and should be rare in practice.

Testing with multiple writers shows that we only very
rarely attempt to take the i_mutex, so the performance
impact is minimal.

This commit also adds a test to verify the S_NOSEC/i_mutex
behavior.

Signed-off-by: Patrick Farrell <paf@cray.com>
Change-Id: If481366283ceaeb247e50cc4d536fb10e9c329ee
Reviewed-on: https://review.whamcloud.com/22853
Tested-by: Jenkins
Reviewed-by: James Simmons <uja.ornl@yahoo.com>
Tested-by: Maloo <hpdd-maloo@intel.com>
Reviewed-by: Andreas Dilger <andreas.dilger@intel.com>
lustre/include/lustre_compat.h
lustre/include/obd_support.h
lustre/llite/llite_lib.c
lustre/llite/namei.c
lustre/llite/vvp_io.c
lustre/tests/sanity.sh