[LU-8656] IS_NOSEC check in vvp_io_write_start always returns false

• Status: Resolved
• Project: Lustre
• Fix Version/s: Lustre 2.10.0
• Type: Bug
• Priority: Minor
• Reporter: Patrick Farrell (Inactive)
• Assignee: Patrick Farrell (Inactive)
• Resolution: Fixed
• Severity: 3

Description

In vvp_io_write_start, we check "IS_NOSEC" to see whether or not to take the inode mutex. (This is to protect the automatic setuid bit removal when writing to a file.)

This was added in patch http://review.whamcloud.com/19840 "LU-8025 llite: make vvp_io_write_start lockless for newer kernels".

Unfortunately, it looks like that check is not working correctly, and is always returning false, causing us to take the inode mutex. This may have some dependence on kernel version, as different versions vary in whether they provide IS_NOSEC.

Issue Links

• is related to: LU-8025 ll_direct_io code introduced in LU-62...
• is related to: LU-8162 Get i_mutex lock when changing SUID bits

Comments

[Gerrit Updater]

Patrick Farrell ([e-mail]) uploaded a new patch: http://review.whamcloud.com/22853
Subject: LU-8656 test: test IS_NOSEC and i_mutex usage
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: 21f2167063086ae7651722991cb2a1fd49467ad0

[James A Simmons]

Which kernel versions is this broken on? The backwards macros are taken straight from the upstream kernel.

[Patrick Farrell (Inactive)]

Confirmed the IS_NOSEC check isn't working right on CentOS 6 (which does not provide its own version of the check) and on SLES 12, which does.

Andreas and I discussed this at LAD, I will share details of what we figured out as I work on fixing this.

[Patrick Farrell (Inactive)]

James,

I don't know exactly. That's part of why I contributed the test, so it can run on the Intel infrastructure and hopefully on a few different kernel versions.

[Patrick Farrell (Inactive)]

James,

Can you point me at where in the upstream kernel? I see this:

static inline void inode_has_no_xattr(struct inode *inode)
{
        if (!is_sxid(inode->i_mode) && (inode->i_sb->s_flags & MS_NOSEC))
                inode->i_flags |= S_NOSEC;
}

But the second part of that is important too, perhaps?

[Patrick Farrell (Inactive)]

One weird note - I don't see why the check is wrong on CentOS 6, where IS_NOSEC is not provided by the kernel. In that case, it looks like our test should work. But it doesn't seem to, at least if my test framework test is correct. (I ran it outside of the framework, but still.)

[James A Simmons]

The commit that introduced the IS_NOSEC work is 69b4573296469fd3f70cf7044693074980517067 for the linux kernel.

[Andreas Dilger]

Patrick or James, it would be useful if you could provide a stack trace of where/how file_remove_perms() gets called, because I just can't find it in the code.

[Patrick Farrell (Inactive)]

Andreas -

File_remove_privs.

[Gerrit Updater]

Oleg Drokin ([e-mail]) merged in patch https://review.whamcloud.com/22853/
Subject: LU-8656 vvp: Add S_NOSEC support
Project: fs/lustre-release
Branch: master
Current Patch Set:
Commit: 8bc4b26453fb9d3296475ab3b9c9f3a9bf905afc

[Peter Jones]

Landed for 2.10