[Nfs-ganesha-devel] XID missing in error path for RPC AUTH failure.

2017-12-12 Thread Pradeep
Hello,

When using krb5 exports, I noticed that TIRPC does not send XID in response
- see xdr_reply_encode() for MSG_DENIED case. Looks like Linux clients
can't decode the message and go in to an infinite loop retrying the same
NFS operation. I tried adding XID back (like it is done for normal case)
and it seems to have fixed the problem. Is this the right thing to do?

diff --git a/src/rpc_dplx_msg.c b/src/rpc_dplx_msg.c
index 01e5a5c..a585e8a 100644
--- a/src/rpc_dplx_msg.c
+++ b/src/rpc_dplx_msg.c
@@ -194,9 +194,12 @@ xdr_reply_encode(XDR *xdrs, struct rpc_msg *dmsg)
__warnx(TIRPC_DEBUG_FLAG_RPC_MSG,
"%s:%u DENIED AUTH",
__func__, __LINE__);
-   buf = XDR_INLINE(xdrs, 2 * BYTES_PER_XDR_UNIT);
+   buf = XDR_INLINE(xdrs, 5 * BYTES_PER_XDR_UNIT);

if (buf != NULL) {
+   IXDR_PUT_INT32(buf, dmsg->rm_xid);
+   IXDR_PUT_ENUM(buf, dmsg->rm_direction);
+   IXDR_PUT_ENUM(buf, dmsg->rm_reply.rp_stat);
IXDR_PUT_ENUM(buf, rr->rj_stat);
IXDR_PUT_ENUM(buf, rr->rj_why);
} else if (!xdr_putenum(xdrs, rr->rj_stat)) {
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Nfs-ganesha-devel mailing list
Nfs-ganesha-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs-ganesha-devel


Re: [Nfs-ganesha-devel] XID missing in error path for RPC AUTH failure.

2017-12-12 Thread Matt Benjamin
That sounds right, I'm uncertain whether this has regressed in the
text, or maybe in the likelihood of inlining in the new dispatch
model.  Bill?

Matt

On Wed, Dec 13, 2017 at 9:38 AM, Pradeep  wrote:
> Hello,
>
> When using krb5 exports, I noticed that TIRPC does not send XID in response
> - see xdr_reply_encode() for MSG_DENIED case. Looks like Linux clients can't
> decode the message and go in to an infinite loop retrying the same NFS
> operation. I tried adding XID back (like it is done for normal case) and it
> seems to have fixed the problem. Is this the right thing to do?
>
> diff --git a/src/rpc_dplx_msg.c b/src/rpc_dplx_msg.c
> index 01e5a5c..a585e8a 100644
> --- a/src/rpc_dplx_msg.c
> +++ b/src/rpc_dplx_msg.c
> @@ -194,9 +194,12 @@ xdr_reply_encode(XDR *xdrs, struct rpc_msg *dmsg)
> __warnx(TIRPC_DEBUG_FLAG_RPC_MSG,
> "%s:%u DENIED AUTH",
> __func__, __LINE__);
> -   buf = XDR_INLINE(xdrs, 2 * BYTES_PER_XDR_UNIT);
> +   buf = XDR_INLINE(xdrs, 5 * BYTES_PER_XDR_UNIT);
>
> if (buf != NULL) {
> +   IXDR_PUT_INT32(buf, dmsg->rm_xid);
> +   IXDR_PUT_ENUM(buf, dmsg->rm_direction);
> +   IXDR_PUT_ENUM(buf, dmsg->rm_reply.rp_stat);
> IXDR_PUT_ENUM(buf, rr->rj_stat);
> IXDR_PUT_ENUM(buf, rr->rj_why);
> } else if (!xdr_putenum(xdrs, rr->rj_stat)) {
>
> --
> Check out the vibrant tech community on one of the world's most
> engaging tech sites, Slashdot.org! http://sdm.link/slashdot
> ___
> Nfs-ganesha-devel mailing list
> Nfs-ganesha-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/nfs-ganesha-devel
>



-- 

Matt Benjamin
Red Hat, Inc.
315 West Huron Street, Suite 140A
Ann Arbor, Michigan 48103

http://www.redhat.com/en/technologies/storage

tel.  734-821-5101
fax.  734-769-8938
cel.  734-216-5309

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Nfs-ganesha-devel mailing list
Nfs-ganesha-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs-ganesha-devel


Re: [Nfs-ganesha-devel] XID missing in error path for RPC AUTH failure.

2017-12-13 Thread Pradeep
On Tue, Dec 12, 2017 at 10:22 PM, Matt Benjamin  wrote:

> That sounds right, I'm uncertain whether this has regressed in the
> text, or maybe in the likelihood of inlining in the new dispatch
> model.  Bill?
>
>
​It doesn't look like this code has changed recently. ​Is it possible that
some other function was used to encode XID that got removed recently?

Also I see that XID is missing in other error cases as well. Perhaps it is
better to move this up to handle all 'MSG_DENIED' cases.



> Matt
>
> On Wed, Dec 13, 2017 at 9:38 AM, Pradeep  wrote:
> > Hello,
> >
> > When using krb5 exports, I noticed that TIRPC does not send XID in
> response
> > - see xdr_reply_encode() for MSG_DENIED case. Looks like Linux clients
> can't
> > decode the message and go in to an infinite loop retrying the same NFS
> > operation. I tried adding XID back (like it is done for normal case) and
> it
> > seems to have fixed the problem. Is this the right thing to do?
> >
> > diff --git a/src/rpc_dplx_msg.c b/src/rpc_dplx_msg.c
> > index 01e5a5c..a585e8a 100644
> > --- a/src/rpc_dplx_msg.c
> > +++ b/src/rpc_dplx_msg.c
> > @@ -194,9 +194,12 @@ xdr_reply_encode(XDR *xdrs, struct rpc_msg *dmsg)
> > __warnx(TIRPC_DEBUG_FLAG_RPC_MSG,
> > "%s:%u DENIED AUTH",
> > __func__, __LINE__);
> > -   buf = XDR_INLINE(xdrs, 2 * BYTES_PER_XDR_UNIT);
> > +   buf = XDR_INLINE(xdrs, 5 * BYTES_PER_XDR_UNIT);
> >
> > if (buf != NULL) {
> > +   IXDR_PUT_INT32(buf, dmsg->rm_xid);
> > +   IXDR_PUT_ENUM(buf, dmsg->rm_direction);
> > +   IXDR_PUT_ENUM(buf,
> dmsg->rm_reply.rp_stat);
> > IXDR_PUT_ENUM(buf, rr->rj_stat);
> > IXDR_PUT_ENUM(buf, rr->rj_why);
> > } else if (!xdr_putenum(xdrs, rr->rj_stat)) {
> >
> > 
> --
> > Check out the vibrant tech community on one of the world's most
> > engaging tech sites, Slashdot.org! http://sdm.link/slashdot
> > ___
> > Nfs-ganesha-devel mailing list
> > Nfs-ganesha-devel@lists.sourceforge.net
> > https://lists.sourceforge.net/lists/listinfo/nfs-ganesha-devel
> >
>
>
>
> --
>
> Matt Benjamin
> Red Hat, Inc.
> 315 West Huron Street, Suite 140A
> Ann Arbor, Michigan 48103
>
> http://www.redhat.com/en/technologies/storage
>
> tel.  734-821-5101
> fax.  734-769-8938
> cel.  734-216-5309
>
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Nfs-ganesha-devel mailing list
Nfs-ganesha-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs-ganesha-devel


Re: [Nfs-ganesha-devel] XID missing in error path for RPC AUTH failure.

2017-12-14 Thread William Allen Simpson

This is May 2015 code, based upon 2012 code.  Obviously, we haven't been
testing error responses ;)

Not quite.  That would need to be duplicated for each of the error
conditions.  Instead, it should be a bit higher in the function.

Still, I'll keep it duplicated from the ACCEPTED code path, for
trivial efficiency.

On 12/13/17 1:22 AM, Matt Benjamin wrote:

That sounds right, I'm uncertain whether this has regressed in the
text, or maybe in the likelihood of inlining in the new dispatch
model.  Bill?

Matt

On Wed, Dec 13, 2017 at 9:38 AM, Pradeep  wrote:

Hello,

When using krb5 exports, I noticed that TIRPC does not send XID in response
- see xdr_reply_encode() for MSG_DENIED case. Looks like Linux clients can't
decode the message and go in to an infinite loop retrying the same NFS
operation. I tried adding XID back (like it is done for normal case) and it
seems to have fixed the problem. Is this the right thing to do?

diff --git a/src/rpc_dplx_msg.c b/src/rpc_dplx_msg.c
index 01e5a5c..a585e8a 100644
--- a/src/rpc_dplx_msg.c
+++ b/src/rpc_dplx_msg.c
@@ -194,9 +194,12 @@ xdr_reply_encode(XDR *xdrs, struct rpc_msg *dmsg)
 __warnx(TIRPC_DEBUG_FLAG_RPC_MSG,
 "%s:%u DENIED AUTH",
 __func__, __LINE__);
-   buf = XDR_INLINE(xdrs, 2 * BYTES_PER_XDR_UNIT);
+   buf = XDR_INLINE(xdrs, 5 * BYTES_PER_XDR_UNIT);

 if (buf != NULL) {
+   IXDR_PUT_INT32(buf, dmsg->rm_xid);
+   IXDR_PUT_ENUM(buf, dmsg->rm_direction);
+   IXDR_PUT_ENUM(buf, dmsg->rm_reply.rp_stat);
 IXDR_PUT_ENUM(buf, rr->rj_stat);
 IXDR_PUT_ENUM(buf, rr->rj_why);
 } else if (!xdr_putenum(xdrs, rr->rj_stat)) {

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Nfs-ganesha-devel mailing list
Nfs-ganesha-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs-ganesha-devel








--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Nfs-ganesha-devel mailing list
Nfs-ganesha-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs-ganesha-devel


Re: [Nfs-ganesha-devel] XID missing in error path for RPC AUTH failure.

2017-12-15 Thread William Allen Simpson

On 12/14/17 1:13 PM, William Allen Simpson wrote:

This is May 2015 code, based upon 2012 code.  Obviously, we haven't
been testing error responses ;)


I wanted to add a personal thank you for such an excellent bug report.
The patch went in the next branch yesterday, and should show up in the
Ganesha dev.22 branch today.  You are credited in the log message.

There are (at least) 2 different code paths for handling similar reply
message output, and this one wasn't correct.  Back in 2005, I'd spent a
fair amount of time trying to debug it (the reason for all those logging
messages), and had found missing breaks and returns.

But I wasn't looking at the formatting for error messages that weren't
being tested, and missed the obvious   Next year, we should probably
try to find all the code paths and consolidate.

What versions do you need backported?


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Nfs-ganesha-devel mailing list
Nfs-ganesha-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs-ganesha-devel


Re: [Nfs-ganesha-devel] XID missing in error path for RPC AUTH failure.

2017-12-15 Thread Pradeep
Thanks for the patch Bill. dev.22 works for me.

On Fri, Dec 15, 2017 at 9:15 AM, William Allen Simpson <
william.allen.simp...@gmail.com> wrote:

> On 12/14/17 1:13 PM, William Allen Simpson wrote:
>
>> This is May 2015 code, based upon 2012 code.  Obviously, we haven't
>> been testing error responses ;)
>>
>> I wanted to add a personal thank you for such an excellent bug report.
> The patch went in the next branch yesterday, and should show up in the
> Ganesha dev.22 branch today.  You are credited in the log message.
>
> There are (at least) 2 different code paths for handling similar reply
> message output, and this one wasn't correct.  Back in 2005, I'd spent a
> fair amount of time trying to debug it (the reason for all those logging
> messages), and had found missing breaks and returns.
>
> But I wasn't looking at the formatting for error messages that weren't
> being tested, and missed the obvious   Next year, we should probably
> try to find all the code paths and consolidate.
>
> What versions do you need backported?
>
>
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Nfs-ganesha-devel mailing list
Nfs-ganesha-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs-ganesha-devel