| 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: <b45404d4-cbf1-4f42-ab74-5868ef7fe895@oracle.com>
Date: Sat, 14 Dec 2024 11:34:32 -0500
From: Chuck Lever <chuck.lever@...cle.com>
To: Jeff Layton <jlayton@...nel.org>, Neil Brown <neilb@...e.de>,
Olga Kornievskaia <okorniev@...hat.com>, Dai Ngo <Dai.Ngo@...cle.com>,
Tom Talpey <tom@...pey.com>, Jonathan Corbet <corbet@....net>,
Trond Myklebust <trondmy@...nel.org>, Anna Schumaker <anna@...nel.org>
Cc: linux-nfs@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-doc@...r.kernel.org
Subject: Re: [PATCH v5 09/10] nfsd: handle delegated timestamps in SETATTR
On 12/14/24 9:55 AM, Jeff Layton wrote:
> On Fri, 2024-12-13 at 09:18 -0500, Chuck Lever wrote:
>> On 12/13/24 9:14 AM, Jeff Layton wrote:
>>> On Thu, 2024-12-12 at 16:06 -0500, Chuck Lever wrote:
>>>> On 12/9/24 4:14 PM, Jeff Layton wrote:
>>>>> Allow SETATTR to handle delegated timestamps. This patch assumes that
>>>>> only the delegation holder has the ability to set the timestamps in this
>>>>> way, so we allow this only if the SETATTR stateid refers to a
>>>>> *_ATTRS_DELEG delegation.
>>>>>
>>>>> Signed-off-by: Jeff Layton <jlayton@...nel.org>
>>>>> ---
>>>>> fs/nfsd/nfs4proc.c | 31 ++++++++++++++++++++++++++++---
>>>>> fs/nfsd/nfs4state.c | 2 +-
>>>>> fs/nfsd/nfs4xdr.c | 20 ++++++++++++++++++++
>>>>> fs/nfsd/nfsd.h | 5 ++++-
>>>>> 4 files changed, 53 insertions(+), 5 deletions(-)
>>>>>
>>>>> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
>>>>> index f8a10f90bc7a4b288c20d2733c85f331cc0a8dba..fea171ffed623818c61886b786339b0b73f1053d 100644
>>>>> --- a/fs/nfsd/nfs4proc.c
>>>>> +++ b/fs/nfsd/nfs4proc.c
>>>>> @@ -1135,18 +1135,43 @@ nfsd4_setattr(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>>>>> .na_iattr = &setattr->sa_iattr,
>>>>> .na_seclabel = &setattr->sa_label,
>>>>> };
>>>>> + bool save_no_wcc, deleg_attrs;
>>>>> + struct nfs4_stid *st = NULL;
>>>>> struct inode *inode;
>>>>> __be32 status = nfs_ok;
>>>>> - bool save_no_wcc;
>>>>> int err;
>>>>>
>>>>> - if (setattr->sa_iattr.ia_valid & ATTR_SIZE) {
>>>>> + deleg_attrs = setattr->sa_bmval[2] & (FATTR4_WORD2_TIME_DELEG_ACCESS |
>>>>> + FATTR4_WORD2_TIME_DELEG_MODIFY);
>>>>> +
>>>>> + if (deleg_attrs || (setattr->sa_iattr.ia_valid & ATTR_SIZE)) {
>>>>> + int flags = WR_STATE;
>>>>> +
>>>>> + if (setattr->sa_bmval[2] & FATTR4_WORD2_TIME_DELEG_ACCESS)
>>>>> + flags |= RD_STATE;
>>>>> +
>>>>> status = nfs4_preprocess_stateid_op(rqstp, cstate,
>>>>> &cstate->current_fh, &setattr->sa_stateid,
>>>>> - WR_STATE, NULL, NULL);
>>>>> + flags, NULL, &st);
>>>>> if (status)
>>>>> return status;
>>>>> }
>>>>> +
>>>>> + if (deleg_attrs) {
>>>>> + status = nfserr_bad_stateid;
>>>>> + if (st->sc_type & SC_TYPE_DELEG) {
>>>>> + struct nfs4_delegation *dp = delegstateid(st);
>>>>> +
>>>>> + /* Only for *_ATTRS_DELEG flavors */
>>>>> + if (deleg_attrs_deleg(dp->dl_type))
>>>>> + status = nfs_ok;
>>>>> + }
>>>>> + }
>>>>> + if (st)
>>>>> + nfs4_put_stid(st);
>>>>> + if (status)
>>>>> + return status;
>>>>> +
>>>>> err = fh_want_write(&cstate->current_fh);
>>>>> if (err)
>>>>> return nfserrno(err);
>>>>> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
>>>>> index c882eeba7830b0249ccd74654f81e63b12a30f14..a76e35f86021c5657e31e4fddf08cb5781f01e32 100644
>>>>> --- a/fs/nfsd/nfs4state.c
>>>>> +++ b/fs/nfsd/nfs4state.c
>>>>> @@ -5486,7 +5486,7 @@ nfsd4_process_open1(struct nfsd4_compound_state *cstate,
>>>>> static inline __be32
>>>>> nfs4_check_delegmode(struct nfs4_delegation *dp, int flags)
>>>>> {
>>>>> - if ((flags & WR_STATE) && deleg_is_read(dp->dl_type))
>>>>> + if (!(flags & RD_STATE) && deleg_is_read(dp->dl_type))
>>>>> return nfserr_openmode;
>>>>> else
>>>>> return nfs_ok;
>>>>> diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
>>>>> index 0561c99b5def2eccf679bf3ea0e5b1a57d5d8374..ce93a31ac5cec75b0f944d288e796e7a73641572 100644
>>>>> --- a/fs/nfsd/nfs4xdr.c
>>>>> +++ b/fs/nfsd/nfs4xdr.c
>>>>> @@ -521,6 +521,26 @@ nfsd4_decode_fattr4(struct nfsd4_compoundargs *argp, u32 *bmval, u32 bmlen,
>>>>> *umask = mask & S_IRWXUGO;
>>>>> iattr->ia_valid |= ATTR_MODE;
>>>>> }
>>>>> + if (bmval[2] & FATTR4_WORD2_TIME_DELEG_ACCESS) {
>>>>> + fattr4_time_deleg_access access;
>>>>> +
>>>>> + if (!xdrgen_decode_fattr4_time_deleg_access(argp->xdr, &access))
>>>>> + return nfserr_bad_xdr;
>>>>> + iattr->ia_atime.tv_sec = access.seconds;
>>>>> + iattr->ia_atime.tv_nsec = access.nseconds;
>>>>> + iattr->ia_valid |= ATTR_ATIME | ATTR_ATIME_SET | ATTR_DELEG;
>>>>> + }
>>>>> + if (bmval[2] & FATTR4_WORD2_TIME_DELEG_MODIFY) {
>>>>> + fattr4_time_deleg_modify modify;
>>>>> +
>>>>> + if (!xdrgen_decode_fattr4_time_deleg_modify(argp->xdr, &modify))
>>>>> + return nfserr_bad_xdr;
>>>>> + iattr->ia_mtime.tv_sec = modify.seconds;
>>>>> + iattr->ia_mtime.tv_nsec = modify.nseconds;
>>>>> + iattr->ia_ctime.tv_sec = modify.seconds;
>>>>> + iattr->ia_ctime.tv_nsec = modify.seconds;
>>>>> + iattr->ia_valid |= ATTR_CTIME | ATTR_MTIME | ATTR_MTIME_SET | ATTR_DELEG;
>>>>> + }
>>>>>
>>>>> /* request sanity: did attrlist4 contain the expected number of words? */
>>>>> if (attrlist4_count != xdr_stream_pos(argp->xdr) - starting_pos)
>>>>> diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
>>>>> index 004415651295891b3440f52a4c986e3a668a48cb..f007699aa397fe39042d80ccd568db4654d19dd5 100644
>>>>> --- a/fs/nfsd/nfsd.h
>>>>> +++ b/fs/nfsd/nfsd.h
>>>>> @@ -531,7 +531,10 @@ static inline bool nfsd_attrs_supported(u32 minorversion, const u32 *bmval)
>>>>> #endif
>>>>> #define NFSD_WRITEABLE_ATTRS_WORD2 \
>>>>> (FATTR4_WORD2_MODE_UMASK \
>>>>> - | MAYBE_FATTR4_WORD2_SECURITY_LABEL)
>>>>> + | MAYBE_FATTR4_WORD2_SECURITY_LABEL \
>>>>> + | FATTR4_WORD2_TIME_DELEG_ACCESS \
>>>>> + | FATTR4_WORD2_TIME_DELEG_MODIFY \
>>>>> + )
>>>>>
>>>>> #define NFSD_SUPPATTR_EXCLCREAT_WORD0 \
>>>>> NFSD_WRITEABLE_ATTRS_WORD0
>>>>>
>>>>
>>>> Hi Jeff-
>>>>
>>>> After this patch is applied, I see failures of the git regression suite
>>>> on NFSv4.2 mounts.
>>>>
>>>> Test Summary Report
>>>> -------------------
>>>> ./t3412-rebase-root.sh (Wstat: 256 (exited
>>>> 1) Tests: 25 Failed: 5)
>>>> Failed tests: 6, 19, 21-22, 24
>>>> Non-zero exit status: 1
>>>> ./t3400-rebase.sh (Wstat: 256 (exited
>>>> 1) Tests: 38 Failed: 1)
>>>> Failed test: 31
>>>> Non-zero exit status: 1
>>>> ./t3406-rebase-message.sh (Wstat: 256 (exited
>>>> 1) Tests: 32 Failed: 2)
>>>> Failed tests: 15, 20
>>>> Non-zero exit status: 1
>>>> ./t3428-rebase-signoff.sh (Wstat: 256 (exited
>>>> 1) Tests: 7 Failed: 2)
>>>> Failed tests: 6-7
>>>> Non-zero exit status: 1
>>>> ./t3418-rebase-continue.sh (Wstat: 256 (exited
>>>> 1) Tests: 29 Failed: 1)
>>>> Failed test: 7
>>>> Non-zero exit status: 1
>>>> ./t3415-rebase-autosquash.sh (Wstat: 256 (exited
>>>> 1) Tests: 27 Failed: 2)
>>>> Failed tests: 3-4
>>>> Non-zero exit status: 1
>>>> ./t3404-rebase-interactive.sh (Wstat: 256 (exited
>>>> 1) Tests: 131 Failed: 15)
>>>> Failed tests: 32, 34-43, 45, 121-123
>>>> Non-zero exit status: 1
>>>> ./t1013-read-tree-submodule.sh (Wstat: 256 (exited
>>>> 1) Tests: 68 Failed: 1)
>>>> Failed test: 34
>>>> Non-zero exit status: 1
>>>> ./t2013-checkout-submodule.sh (Wstat: 256 (exited
>>>> 1) Tests: 74 Failed: 4)
>>>> Failed tests: 26-27, 30-31
>>>> Non-zero exit status: 1
>>>> ./t5500-fetch-pack.sh (Wstat: 256 (exited
>>>> 1) Tests: 375 Failed: 1)
>>>> Failed test: 28
>>>> Non-zero exit status: 1
>>>> ./t5572-pull-submodule.sh (Wstat: 256 (exited
>>>> 1) Tests: 67 Failed: 2)
>>>> Failed tests: 5, 7
>>>> Non-zero exit status: 1
>>>> Files=1007, Tests=30810, 1417 wallclock secs (11.18 usr 10.17 sys +
>>>> 1037.05 cusr 6529.12 csys = 7587.52 CPU)
>>>> Result: FAIL
>>>>
>>>> The NFS client and NFS server under test are running the same v6.13-rc2
>>>> kernel from my git.kernel.org nfsd-testing branch.
>>>>
>>>>
>>>
>>> I'm not seeing these failures. I ran the gitr suite under kdevops with
>>> your nfsd-testing branch (6.13.0-rc2-ge9a809c5714e):
>>>
>>> All tests successful.
>>> Files=1007, Tests=30695, 10767 wallclock secs (13.87 usr 16.86 sys + 1160.76 cusr 17870.80 csys = 19062.29 CPU)
>>> Result: PASS
>>>
>>> ...and looking at the results of those specific tests, they did run and
>>> they did pass.
>>>
>>> I'm rerunning the tests now. It's possible the underlying fs matters.
>>> Mine is exporting xfs. Yours?
>>
>> Mine is btrfs, and the NFS version is v4.2 on TCP.
>>
>>
>
> Nope, I still can't reproduce this with btrfs either. I'm also using
> v4.2 on TCP. I assume you're running this under kdevops, so we should
> have a relatively similar environment.
I'm running the "stress" setting, which starts twice as many threads
as there are CPUs (so, 16, I think?). 32 nfsd threads.
> Are you also testing the same commit?
The first failing test run is on 6.13.0-rc2-00016-gb45eda1daa7d
https://git.kernel.org/pub/scm/linux/kernel/git/cel/linux.git/commit/?h=nfsd-testing&id=b45eda1daa7d79a2bf0426d27d4b359b8bb71d33
I'll take a closer look.
--
Chuck Lever
Powered by blists - more mailing lists