| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <1d5d8438-e1df-35cb-ed53-8a91ad8d8811@oyet.no>
Date: Tue, 9 May 2023 13:53:21 +0200
From: Jan Ingvoldstad <jan-lnfs@...t.no>
To: Chengen Du <chengen.du@...onical.com>,
trond.myklebust@...merspace.com
Cc: anna@...nel.org, linux-nfs@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH] NFS: Add mount option 'nofasc'
Hi,
The issues causing inconvenience could be easily resolved by
Canonical/Ubuntu without involving this upstream list, but they are of
concern for anyone trying to track non-stable Linux kernel releases as well.
May I be so bold as to suggest that the feature in question be reversed
to an opt-in instead of an opt-out?
As an option "fasc" for having these features enabled, with "nofasc" the
default.
By making it opt-out, as suggested, or keeping the current patches in
place, there is around a 50-100 fold resource requirement on the NFS
server, which makes the Linux versions using these patches (from 6.2-rc3
onwards) completely unsuitable for NFS clients in a multi-user
client-server environment.
It amazes me that this keeps going on without resolution for well over
two months now, and I request that this matter is bumped in priority.
Cheers,
Jan
Den 09.05.2023 04:31, skrev Chengen Du:
> Hi Trond,
>
> I sincerely apologize for interrupting you, as I understand that you
> may have other pressing matters to attend to.
> However, I was hoping to bring your attention to a pressing matter
> that a number of community users are currently experiencing.
> As can be seen in this link:
> https://bugs.launchpad.net/ubuntu/+source/linux/+bug/2015827,
> the issue at hand is causing a significant amount of inconvenience to
> many individuals.
>
> If it wouldn't be too much trouble, I would be immensely grateful if
> you could spare a moment to examine the patch and perhaps offer some
> guidance on how best to proceed.
> Your assistance in this matter would be greatly appreciated.
>
> Best regards,
> Chengen Du
>
> On Tue, Apr 25, 2023 at 9:41 AM Chengen Du <chengen.du@...onical.com> wrote:
>> Hi,
>>
>> May I ask if there are any concerns or opinions regarding the
>> introduction of the new mount option?
>> If there is a more suitable solution, we can discuss it, and I can
>> work on implementing it.
>>
>> Best regards,
>> Chengen Du
>>
>> On Wed, Apr 19, 2023 at 3:18 PM Daire Byrne <daire@...g.com> wrote:
>>> Just to say, I am personally for this or some other way to opt out of
>>> the increased ACCESS calls on select servers (e.g. ones with high
>>> latency or with lots of multi user logins).
>>>
>>> I see it as similar to "actimeo" and "nocto" options in allowing users
>>> to reduce "correctness" at their own risk if it helps with their
>>> workloads and they deem the risk worth it.
>>>
>>> I am currently reverting the original patches in our kernel for our
>>> nfs re-exporting workloads.
>>>
>>> Daire
>>>
>>>
>>> On Tue, 11 Apr 2023 at 04:03, Chengen Du <chengen.du@...onical.com> wrote:
>>>> This mount option is used to skip clearing the file access cache
>>>> upon login. Some users or applications switch to other privileged
>>>> users via commands such as 'su' to operate on NFS-mounted folders.
>>>> In such cases, the privileged user's login time will be renewed,
>>>> and NFS ACCESS operations will need to be re-sent, potentially
>>>> leading to performance degradation.
>>>> In some production environments where the access cache can be
>>>> trusted due to stable group membership, this option could be
>>>> particularly useful.
>>>>
>>>> Signed-off-by: Chengen Du <chengen.du@...onical.com>
>>>> ---
>>>> fs/nfs/dir.c | 21 ++++++++++++---------
>>>> fs/nfs/fs_context.c | 8 ++++++++
>>>> fs/nfs/super.c | 1 +
>>>> include/linux/nfs_fs_sb.h | 1 +
>>>> 4 files changed, 22 insertions(+), 9 deletions(-)
>>>>
>>>> diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
>>>> index 6fbcbb8d6587..9a6d86e2044e 100644
>>>> --- a/fs/nfs/dir.c
>>>> +++ b/fs/nfs/dir.c
>>>> @@ -2953,12 +2953,14 @@ static struct nfs_access_entry *nfs_access_search_rbtree(struct inode *inode, co
>>>> return NULL;
>>>> }
>>>>
>>>> -static u64 nfs_access_login_time(const struct task_struct *task,
>>>> - const struct cred *cred)
>>>> +static inline
>>>> +bool nfs_check_access_stale(const struct task_struct *task,
>>>> + const struct cred *cred,
>>>> + const struct nfs_access_entry *cache)
>>>> {
>>>> const struct task_struct *parent;
>>>> const struct cred *pcred;
>>>> - u64 ret;
>>>> + u64 login_time;
>>>>
>>>> rcu_read_lock();
>>>> for (;;) {
>>>> @@ -2968,15 +2970,15 @@ static u64 nfs_access_login_time(const struct task_struct *task,
>>>> break;
>>>> task = parent;
>>>> }
>>>> - ret = task->start_time;
>>>> + login_time = task->start_time;
>>>> rcu_read_unlock();
>>>> - return ret;
>>>> +
>>>> + return ((s64)(login_time - cache->timestamp) > 0);
>>>> }
>>>>
>>>> static int nfs_access_get_cached_locked(struct inode *inode, const struct cred *cred, u32 *mask, bool may_block)
>>>> {
>>>> struct nfs_inode *nfsi = NFS_I(inode);
>>>> - u64 login_time = nfs_access_login_time(current, cred);
>>>> struct nfs_access_entry *cache;
>>>> bool retry = true;
>>>> int err;
>>>> @@ -3005,7 +3007,8 @@ static int nfs_access_get_cached_locked(struct inode *inode, const struct cred *
>>>> retry = false;
>>>> }
>>>> err = -ENOENT;
>>>> - if ((s64)(login_time - cache->timestamp) > 0)
>>>> + if (!(NFS_SERVER(inode)->flags & NFS_MOUNT_NOFASC) &&
>>>> + nfs_check_access_stale(current, cred, cache))
>>>> goto out;
>>>> *mask = cache->mask;
>>>> list_move_tail(&cache->lru, &nfsi->access_cache_entry_lru);
>>>> @@ -3025,7 +3028,6 @@ static int nfs_access_get_cached_rcu(struct inode *inode, const struct cred *cre
>>>> * but do it without locking.
>>>> */
>>>> struct nfs_inode *nfsi = NFS_I(inode);
>>>> - u64 login_time = nfs_access_login_time(current, cred);
>>>> struct nfs_access_entry *cache;
>>>> int err = -ECHILD;
>>>> struct list_head *lh;
>>>> @@ -3040,7 +3042,8 @@ static int nfs_access_get_cached_rcu(struct inode *inode, const struct cred *cre
>>>> cache = NULL;
>>>> if (cache == NULL)
>>>> goto out;
>>>> - if ((s64)(login_time - cache->timestamp) > 0)
>>>> + if (!(NFS_SERVER(inode)->flags & NFS_MOUNT_NOFASC) &&
>>>> + nfs_check_access_stale(current, cred, cache))
>>>> goto out;
>>>> if (nfs_check_cache_invalid(inode, NFS_INO_INVALID_ACCESS))
>>>> goto out;
>>>> diff --git a/fs/nfs/fs_context.c b/fs/nfs/fs_context.c
>>>> index 9bcd53d5c7d4..5cd9b2882c79 100644
>>>> --- a/fs/nfs/fs_context.c
>>>> +++ b/fs/nfs/fs_context.c
>>>> @@ -88,6 +88,7 @@ enum nfs_param {
>>>> Opt_vers,
>>>> Opt_wsize,
>>>> Opt_write,
>>>> + Opt_fasc,
>>>> };
>>>>
>>>> enum {
>>>> @@ -194,6 +195,7 @@ static const struct fs_parameter_spec nfs_fs_parameters[] = {
>>>> fsparam_string("vers", Opt_vers),
>>>> fsparam_enum ("write", Opt_write, nfs_param_enums_write),
>>>> fsparam_u32 ("wsize", Opt_wsize),
>>>> + fsparam_flag_no("fasc", Opt_fasc),
>>>> {}
>>>> };
>>>>
>>>> @@ -861,6 +863,12 @@ static int nfs_fs_context_parse_param(struct fs_context *fc,
>>>> case Opt_sloppy:
>>>> ctx->sloppy = true;
>>>> break;
>>>> + case Opt_fasc:
>>>> + if (result.negated)
>>>> + ctx->flags |= NFS_MOUNT_NOFASC;
>>>> + else
>>>> + ctx->flags &= ~NFS_MOUNT_NOFASC;
>>>> + break;
>>>> }
>>>>
>>>> return 0;
>>>> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
>>>> index 05ae23657527..059513d403f8 100644
>>>> --- a/fs/nfs/super.c
>>>> +++ b/fs/nfs/super.c
>>>> @@ -444,6 +444,7 @@ static void nfs_show_mount_options(struct seq_file *m, struct nfs_server *nfss,
>>>> { NFS_MOUNT_NORDIRPLUS, ",nordirplus", "" },
>>>> { NFS_MOUNT_UNSHARED, ",nosharecache", "" },
>>>> { NFS_MOUNT_NORESVPORT, ",noresvport", "" },
>>>> + { NFS_MOUNT_NOFASC, ",nofasc", "" },
>>>> { 0, NULL, NULL }
>>>> };
>>>> const struct proc_nfs_info *nfs_infop;
>>>> diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
>>>> index ea2f7e6b1b0b..a39ae1bd2217 100644
>>>> --- a/include/linux/nfs_fs_sb.h
>>>> +++ b/include/linux/nfs_fs_sb.h
>>>> @@ -153,6 +153,7 @@ struct nfs_server {
>>>> #define NFS_MOUNT_WRITE_EAGER 0x01000000
>>>> #define NFS_MOUNT_WRITE_WAIT 0x02000000
>>>> #define NFS_MOUNT_TRUNK_DISCOVERY 0x04000000
>>>> +#define NFS_MOUNT_NOFASC 0x08000000
>>>>
>>>> unsigned int fattr_valid; /* Valid attributes */
>>>> unsigned int caps; /* server capabilities */
>>>> --
>>>> 2.37.2
>>>>
--
Cheers,
Jan
Powered by blists - more mailing lists