I reviewed the patch, and tested the upstream 4.7.0-rc3 kernel without
then with the patch. I was able to recreate the problem without the
patch, and the patch fixed the problem.
ACK
-->Andy
On Thu, Jun 16, 2016 at 12:33 PM, Benjamin Coddington
<bcodding@xxxxxxxxxx> wrote:
> Hey Scott, after reviewing this work, I recreated the problem at BAT today.
> This patch looks good, and fixes the problem for me. Thanks!
>
> Reviewed-by: Benjamin Coddington <bcodding@xxxxxxxxxx>
>
> Ben
>
>
> On 7 Jun 2016, at 15:14, Scott Mayhew wrote:
>
>> A generic_cred can be used to look up a unx_cred or a gss_cred, so it's
>> not really safe to use the the generic_cred->acred->ac_flags to store
>> the NO_CRKEY_TIMEOUT flag. A lookup for a unx_cred triggered while the
>> KEY_EXPIRE_SOON flag is already set will cause both NO_CRKEY_TIMEOUT and
>> KEY_EXPIRE_SOON to be set in the ac_flags, leaving the user associated
>> with the auth_cred to be in a state where they're perpetually doing 4K
>> NFS_FILE_SYNC writes.
>>
>> This can be reproduced as follows:
>>
>> 1. Mount two NFS filesystems, one with sec=krb5 and one with sec=sys.
>> They do not need to be the same export, nor do they even need to be from
>> the same NFS server. Also, v3 is fine.
>> $ sudo mount -o v3,sec=krb5 server1:/export /mnt/krb5
>> $ sudo mount -o v3,sec=sys server2:/export /mnt/sys
>>
>> 2. As the normal user, before accessing the kerberized mount, kinit with
>> a short lifetime (but not so short that renewing the ticket would leave
>> you within the 4-minute window again by the time the original ticket
>> expires), e.g.
>> $ kinit -l 10m -r 60m
>>
>> 3. Do some I/O to the kerberized mount and verify that the writes are
>> wsize, UNSTABLE:
>> $ dd if=/dev/zero of=/mnt/krb5/file bs=1M count=1
>>
>> 4. Wait until you're within 4 minutes of key expiry, then do some more
>> I/O to the kerberized mount to ensure that RPC_CRED_KEY_EXPIRE_SOON gets
>> set. Verify that the writes are 4K, FILE_SYNC:
>> $ dd if=/dev/zero of=/mnt/krb5/file bs=1M count=1
>>
>> 5. Now do some I/O to the sec=sys mount. This will cause
>> RPC_CRED_NO_CRKEY_TIMEOUT to be set:
>> $ dd if=/dev/zero of=/mnt/sys/file bs=1M count=1
>>
>> 6. Writes for that user will now be permanently 4K, FILE_SYNC for that
>> user, regardless of which mount is being written to, until you reboot
>> the client. Renewing the kerberos ticket (assuming it hasn't already
>> expired) will have no effect. Grabbing a new kerberos ticket at this
>> point will have no effect either.
>>
>> Move the flag to the auth->au_flags field (which is currently unused)
>> and rename it slightly to reflect that it's no longer associated with
>> the auth_cred->ac_flags. Add the rpc_auth to the arg list of
>> rpcauth_cred_key_to_expire and check the au_flags there too. Finally,
>> add the inode to the arg list of nfs_ctx_key_to_expire so we can
>> determine the rpc_auth to pass to rpcauth_cred_key_to_expire.
>>
>> Signed-off-by: Scott Mayhew <smayhew@xxxxxxxxxx>
>> ---
>> fs/nfs/file.c | 4 ++--
>> fs/nfs/internal.h | 2 +-
>> fs/nfs/write.c | 6 ++++--
>> include/linux/sunrpc/auth.h | 6 ++++--
>> net/sunrpc/auth.c | 4 +++-
>> net/sunrpc/auth_generic.c | 9 +--------
>> net/sunrpc/auth_gss/auth_gss.c | 1 +
>> net/sunrpc/auth_null.c | 1 +
>> net/sunrpc/auth_unix.c | 1 +
>> 9 files changed, 18 insertions(+), 16 deletions(-)
>>
>> diff --git a/fs/nfs/file.c b/fs/nfs/file.c
>> index 717a8d6..6bcd891 100644
>> --- a/fs/nfs/file.c
>> +++ b/fs/nfs/file.c
>> @@ -432,7 +432,7 @@ static int nfs_write_end(struct file *file, struct
>> address_space *mapping,
>> return status;
>> NFS_I(mapping->host)->write_io += copied;
>>
>> - if (nfs_ctx_key_to_expire(ctx)) {
>> + if (nfs_ctx_key_to_expire(ctx, mapping->host)) {
>> status = nfs_wb_all(mapping->host);
>> if (status < 0)
>> return status;
>> @@ -645,7 +645,7 @@ static int nfs_need_check_write(struct file *filp,
>> struct inode *inode)
>>
>> ctx = nfs_file_open_context(filp);
>> if (test_bit(NFS_CONTEXT_ERROR_WRITE, &ctx->flags) ||
>> - nfs_ctx_key_to_expire(ctx))
>> + nfs_ctx_key_to_expire(ctx, inode))
>> return 1;
>> return 0;
>> }
>> diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
>> index 5154fa6..ca87787 100644
>> --- a/fs/nfs/internal.h
>> +++ b/fs/nfs/internal.h
>> @@ -496,7 +496,7 @@ void nfs_init_cinfo(struct nfs_commit_info *cinfo,
>> struct inode *inode,
>> struct nfs_direct_req *dreq);
>> int nfs_key_timeout_notify(struct file *filp, struct inode *inode);
>> -bool nfs_ctx_key_to_expire(struct nfs_open_context *ctx);
>> +bool nfs_ctx_key_to_expire(struct nfs_open_context *ctx, struct inode
>> *inode);
>> void nfs_pageio_stop_mirroring(struct nfs_pageio_descriptor *pgio);
>>
>> #ifdef CONFIG_MIGRATION
>> diff --git a/fs/nfs/write.c b/fs/nfs/write.c
>> index e1c74d3..0b949a0 100644
>> --- a/fs/nfs/write.c
>> +++ b/fs/nfs/write.c
>> @@ -1195,9 +1195,11 @@ nfs_key_timeout_notify(struct file *filp, struct
>> inode *inode)
>> /*
>> * Test if the open context credential key is marked to expire soon.
>> */
>> -bool nfs_ctx_key_to_expire(struct nfs_open_context *ctx)
>> +bool nfs_ctx_key_to_expire(struct nfs_open_context *ctx, struct inode
>> *inode)
>> {
>> - return rpcauth_cred_key_to_expire(ctx->cred);
>> + struct rpc_auth *auth = NFS_SERVER(inode)->client->cl_auth;
>> +
>> + return rpcauth_cred_key_to_expire(auth, ctx->cred);
>> }
>>
>> /*
>> diff --git a/include/linux/sunrpc/auth.h b/include/linux/sunrpc/auth.h
>> index 8997915..f890a29 100644
>> --- a/include/linux/sunrpc/auth.h
>> +++ b/include/linux/sunrpc/auth.h
>> @@ -37,7 +37,6 @@ struct rpcsec_gss_info;
>>
>> /* auth_cred ac_flags bits */
>> enum {
>> - RPC_CRED_NO_CRKEY_TIMEOUT = 0, /* underlying cred has no key
>> timeout */
>> RPC_CRED_KEY_EXPIRE_SOON = 1, /* underlying cred key will expire
>> soon */
>> RPC_CRED_NOTIFY_TIMEOUT = 2, /* nofity generic cred when
>> underlying
>> key will expire soon */
>> @@ -82,6 +81,9 @@ struct rpc_cred {
>>
>> #define RPCAUTH_CRED_MAGIC 0x0f4aa4f0
>>
>> +/* rpc_auth au_flags */
>> +#define RPCAUTH_AUTH_NO_CRKEY_TIMEOUT 0x0001 /* underlying cred has no
>> key timeout */
>> +
>> /*
>> * Client authentication handle
>> */
>> @@ -196,7 +198,7 @@ void
>> rpcauth_destroy_credcache(struct rpc_auth *);
>> void rpcauth_clear_credcache(struct rpc_cred_cache *);
>> int rpcauth_key_timeout_notify(struct rpc_auth *,
>> struct rpc_cred *);
>> -bool rpcauth_cred_key_to_expire(struct rpc_cred *);
>> +bool rpcauth_cred_key_to_expire(struct rpc_auth *,
>> struct rpc_cred *);
>> char * rpcauth_stringify_acceptor(struct rpc_cred *);
>>
>> static inline
>> diff --git a/net/sunrpc/auth.c b/net/sunrpc/auth.c
>> index 040ff62..696eb39 100644
>> --- a/net/sunrpc/auth.c
>> +++ b/net/sunrpc/auth.c
>> @@ -359,8 +359,10 @@ rpcauth_key_timeout_notify(struct rpc_auth *auth,
>> struct rpc_cred *cred)
>> EXPORT_SYMBOL_GPL(rpcauth_key_timeout_notify);
>>
>> bool
>> -rpcauth_cred_key_to_expire(struct rpc_cred *cred)
>> +rpcauth_cred_key_to_expire(struct rpc_auth *auth, struct rpc_cred *cred)
>> {
>> + if (auth->au_flags & RPCAUTH_AUTH_NO_CRKEY_TIMEOUT)
>> + return false;
>> if (!cred->cr_ops->crkey_to_expire)
>> return false;
>> return cred->cr_ops->crkey_to_expire(cred);
>> diff --git a/net/sunrpc/auth_generic.c b/net/sunrpc/auth_generic.c
>> index 54dd3fd..1682195 100644
>> --- a/net/sunrpc/auth_generic.c
>> +++ b/net/sunrpc/auth_generic.c
>> @@ -224,7 +224,7 @@ generic_key_timeout(struct rpc_auth *auth, struct
>> rpc_cred *cred)
>>
>>
>> /* Fast track for non crkey_timeout (no key) underlying
>> credentials */
>> - if (test_bit(RPC_CRED_NO_CRKEY_TIMEOUT, &acred->ac_flags))
>> + if (auth->au_flags & RPCAUTH_AUTH_NO_CRKEY_TIMEOUT)
>> return 0;
>>
>> /* Fast track for the normal case */
>> @@ -236,12 +236,6 @@ generic_key_timeout(struct rpc_auth *auth, struct
>> rpc_cred *cred)
>> if (IS_ERR(tcred))
>> return -EACCES;
>>
>> - if (!tcred->cr_ops->crkey_timeout) {
>> - set_bit(RPC_CRED_NO_CRKEY_TIMEOUT, &acred->ac_flags);
>> - ret = 0;
>> - goto out_put;
>> - }
>> -
>> /* Test for the almost error case */
>> ret = tcred->cr_ops->crkey_timeout(tcred);
>> if (ret != 0) {
>> @@ -257,7 +251,6 @@ generic_key_timeout(struct rpc_auth *auth, struct
>> rpc_cred *cred)
>> set_bit(RPC_CRED_NOTIFY_TIMEOUT, &acred->ac_flags);
>> }
>>
>> -out_put:
>> put_rpccred(tcred);
>> return ret;
>> }
>> diff --git a/net/sunrpc/auth_gss/auth_gss.c
>> b/net/sunrpc/auth_gss/auth_gss.c
>> index e64ae93..813a3cd 100644
>> --- a/net/sunrpc/auth_gss/auth_gss.c
>> +++ b/net/sunrpc/auth_gss/auth_gss.c
>> @@ -1015,6 +1015,7 @@ gss_create_new(struct rpc_auth_create_args *args,
>> struct rpc_clnt *clnt)
>> auth = &gss_auth->rpc_auth;
>> auth->au_cslack = GSS_CRED_SLACK >> 2;
>> auth->au_rslack = GSS_VERF_SLACK >> 2;
>> + auth->au_flags = 0;
>> auth->au_ops = &authgss_ops;
>> auth->au_flavor = flavor;
>> atomic_set(&auth->au_count, 1);
>> diff --git a/net/sunrpc/auth_null.c b/net/sunrpc/auth_null.c
>> index 8d9eb4d..4d17376 100644
>> --- a/net/sunrpc/auth_null.c
>> +++ b/net/sunrpc/auth_null.c
>> @@ -115,6 +115,7 @@ static
>> struct rpc_auth null_auth = {
>> .au_cslack = NUL_CALLSLACK,
>> .au_rslack = NUL_REPLYSLACK,
>> + .au_flags = RPCAUTH_AUTH_NO_CRKEY_TIMEOUT,
>> .au_ops = &authnull_ops,
>> .au_flavor = RPC_AUTH_NULL,
>> .au_count = ATOMIC_INIT(0),
>> diff --git a/net/sunrpc/auth_unix.c b/net/sunrpc/auth_unix.c
>> index 9f65452..a99278c 100644
>> --- a/net/sunrpc/auth_unix.c
>> +++ b/net/sunrpc/auth_unix.c
>> @@ -228,6 +228,7 @@ static
>> struct rpc_auth unix_auth = {
>> .au_cslack = UNX_CALLSLACK,
>> .au_rslack = NUL_REPLYSLACK,
>> + .au_flags = RPCAUTH_AUTH_NO_CRKEY_TIMEOUT,
>> .au_ops = &authunix_ops,
>> .au_flavor = RPC_AUTH_UNIX,
>> .au_count = ATOMIC_INIT(0),
>> --
>> 2.4.11
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>> the body of a message to majordomo@xxxxxxxxxxxxxxx
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
