2011-04-18 19:57:34

by Anna Schumaker

[permalink] [raw]
Subject: [PATCH 1/2] Handle NFS4ERR_WRONGSEC outside of nfs4_handle_exception()

From: Bryan Schumaker <[email protected]>

I only want to try other secflavors during an initial mount if
NFS4ERR_WRONGSEC is returned. nfs4_handle_exception() could
potentially map other errors to EPERM, so we should handle this
error specially for correctness.

Signed-off-by: Bryan Schumaker <[email protected]>

---
fs/nfs/nfs4proc.c | 17 ++++++++++++-----
1 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index b8e1ac6..152e0eb 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -2186,9 +2186,14 @@ static int nfs4_lookup_root(struct nfs_server *server, struct nfs_fh *fhandle,
struct nfs4_exception exception = { };
int err;
do {
- err = nfs4_handle_exception(server,
- _nfs4_lookup_root(server, fhandle, info),
- &exception);
+ err = _nfs4_lookup_root(server, fhandle, info);
+ switch (err) {
+ case 0:
+ case -NFS4ERR_WRONGSEC:
+ break;
+ default:
+ err = nfs4_handle_exception(server, err, &exception);
+ }
} while (exception.retry);
return err;
}
@@ -2221,10 +2226,12 @@ static int nfs4_find_root_sec(struct nfs_server *server, struct nfs_fh *fhandle,

for (i = 0; i < len; i++) {
status = nfs4_lookup_root_sec(server, fhandle, info, flav_array[i]);
- if (status == -EPERM || status == -EACCES)
+ if (status == -NFS4ERR_WRONGSEC || status == -EACCES)
continue;
break;
}
+ if (status == -EACCES)
+ status = -EPERM;
return status;
}

@@ -2235,7 +2242,7 @@ static int nfs4_proc_get_root(struct nfs_server *server, struct nfs_fh *fhandle,
struct nfs_fsinfo *info)
{
int status = nfs4_lookup_root(server, fhandle, info);
- if ((status == -EPERM) && !(server->flags & NFS_MOUNT_SECFLAVOUR))
+ if ((status == -NFS4ERR_WRONGSEC) && !(server->flags & NFS_MOUNT_SECFLAVOUR))
status = nfs4_find_root_sec(server, fhandle, info);
if (status == 0)
status = nfs4_server_capabilities(server, fhandle);
--
1.7.4.4



2011-04-18 19:57:35

by Anna Schumaker

[permalink] [raw]
Subject: [PATCH 2/2] Don't update sequence number if rpc_task is not sent

From: Bryan Schumaker <[email protected]>

If we fail to contact the gss upcall program, then no message will
be sent to the server. The client still updated the sequence number,
however, and this lead to NFS4ERR_SEQ_MISMATCH for the next several
RPC calls.

Signed-off-by: Bryan Schumaker <[email protected]>

---
fs/nfs/nfs4proc.c | 4 ++--
include/linux/sunrpc/sched.h | 2 ++
net/sunrpc/xprt.c | 1 +
3 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 152e0eb..869caf5 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -444,8 +444,8 @@ static int nfs41_sequence_done(struct rpc_task *task, struct nfs4_sequence_res *
if (res->sr_status == 1)
res->sr_status = NFS_OK;

- /* -ERESTARTSYS can result in skipping nfs41_sequence_setup */
- if (!res->sr_slot)
+ /* don't increment the sequence number if the task wasn't sent */
+ if (!RPC_WAS_SENT(task))
goto out;

/* Check the SEQUENCE operation status */
diff --git a/include/linux/sunrpc/sched.h b/include/linux/sunrpc/sched.h
index d81db80..3b94f80 100644
--- a/include/linux/sunrpc/sched.h
+++ b/include/linux/sunrpc/sched.h
@@ -127,6 +127,7 @@ struct rpc_task_setup {
#define RPC_TASK_KILLED 0x0100 /* task was killed */
#define RPC_TASK_SOFT 0x0200 /* Use soft timeouts */
#define RPC_TASK_SOFTCONN 0x0400 /* Fail if can't connect */
+#define RPC_TASK_SENT 0x0800 /* message was sent */

#define RPC_IS_ASYNC(t) ((t)->tk_flags & RPC_TASK_ASYNC)
#define RPC_IS_SWAPPER(t) ((t)->tk_flags & RPC_TASK_SWAPPER)
@@ -134,6 +135,7 @@ struct rpc_task_setup {
#define RPC_ASSASSINATED(t) ((t)->tk_flags & RPC_TASK_KILLED)
#define RPC_IS_SOFT(t) ((t)->tk_flags & RPC_TASK_SOFT)
#define RPC_IS_SOFTCONN(t) ((t)->tk_flags & RPC_TASK_SOFTCONN)
+#define RPC_WAS_SENT(t) ((t)->tk_flags & RPC_TASK_SENT)

#define RPC_TASK_RUNNING 0
#define RPC_TASK_QUEUED 1
diff --git a/net/sunrpc/xprt.c b/net/sunrpc/xprt.c
index 9494c37..ce5eb68 100644
--- a/net/sunrpc/xprt.c
+++ b/net/sunrpc/xprt.c
@@ -906,6 +906,7 @@ void xprt_transmit(struct rpc_task *task)
}

dprintk("RPC: %5u xmit complete\n", task->tk_pid);
+ task->tk_flags |= RPC_TASK_SENT;
spin_lock_bh(&xprt->transport_lock);

xprt->ops->set_retrans_timeout(task);
--
1.7.4.4


2011-04-18 20:00:18

by Myklebust, Trond

[permalink] [raw]
Subject: Re: [PATCH 1/2] Handle NFS4ERR_WRONGSEC outside of nfs4_handle_exception()

On Mon, 2011-04-18 at 15:57 -0400, [email protected] wrote:
> From: Bryan Schumaker <[email protected]>
>
> I only want to try other secflavors during an initial mount if
> NFS4ERR_WRONGSEC is returned. nfs4_handle_exception() could
> potentially map other errors to EPERM, so we should handle this
> error specially for correctness.
>
> Signed-off-by: Bryan Schumaker <[email protected]>
>
> ---
> fs/nfs/nfs4proc.c | 17 ++++++++++++-----
> 1 files changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index b8e1ac6..152e0eb 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -2186,9 +2186,14 @@ static int nfs4_lookup_root(struct nfs_server *server, struct nfs_fh *fhandle,
> struct nfs4_exception exception = { };
> int err;
> do {
> - err = nfs4_handle_exception(server,
> - _nfs4_lookup_root(server, fhandle, info),
> - &exception);
> + err = _nfs4_lookup_root(server, fhandle, info);
> + switch (err) {
> + case 0:
> + case -NFS4ERR_WRONGSEC:
> + break;
> + default:
> + err = nfs4_handle_exception(server, err, &exception);
> + }
> } while (exception.retry);
> return err;
> }
> @@ -2221,10 +2226,12 @@ static int nfs4_find_root_sec(struct nfs_server *server, struct nfs_fh *fhandle,
>
> for (i = 0; i < len; i++) {
> status = nfs4_lookup_root_sec(server, fhandle, info, flav_array[i]);
> - if (status == -EPERM || status == -EACCES)
> + if (status == -NFS4ERR_WRONGSEC || status == -EACCES)
> continue;
> break;
> }
> + if (status == -EACCES)
> + status = -EPERM;

Won't this leak -NFS4ERR_WRONGSEC errors to the user? I think you need
to handle both errors above by mapping them into EPERM.

> return status;
> }
>
> @@ -2235,7 +2242,7 @@ static int nfs4_proc_get_root(struct nfs_server *server, struct nfs_fh *fhandle,
> struct nfs_fsinfo *info)
> {
> int status = nfs4_lookup_root(server, fhandle, info);
> - if ((status == -EPERM) && !(server->flags & NFS_MOUNT_SECFLAVOUR))
> + if ((status == -NFS4ERR_WRONGSEC) && !(server->flags & NFS_MOUNT_SECFLAVOUR))
> status = nfs4_find_root_sec(server, fhandle, info);
> if (status == 0)
> status = nfs4_server_capabilities(server, fhandle);

--
Trond Myklebust
Linux NFS client maintainer

NetApp
[email protected]
http://www.netapp.com