On Jul 11, 2022, at 11:01 AM, Chuck Lever III [off-list ref] wrote:

quoted

On Jul 10, 2022, at 6:10 PM, Rick Macklem [off-list ref] wrote:

Hi,

I have been trying to improve the behaviour of the FreeBSD
NFSv4.1/4.2 client when using the "intr" mount option.

I have come up with the following scheme:
- When RPCs are interrupted, mark the session slot as potentially bad.
- When all session slots are marked potentially bad, do a
DestroySession (only op in RPC) to destroy the session.
- When the server replies NFS4ERR_BAD_SESSION,
do a CreateSession (only op in RPC) to acquire a new session and
continue on.

When testing against a Linux 5.15 server, the CreateSession
succeeds, but returns the same sessionid as the old session.
Then all subsequent RPCs get the NFS4ERR_BAD_SESSION reply.
(The client repeatedly does CreateSession RPCs that reply NFS_OK,
but always with the same sessionid as the destroyed one.)

Here's what I see in the packet trace:
(everything works normally until all session slots are marked
potentially bad at packet# 14216)
packet# RPC
14216 DestroySession request for sessionid 2725cb62002ed418040...0
14302 DestroySession reply NFS_OK
14304 Getattr request (using above sessionid)
14305 Getattr reply NFS4ERR_BAD_SESSION
14306 CreateSession request
*** Now here is where I see a problem...
14307 CreateSession reply NFS_OK with sessionid 
2725cb62002ed418040...0 (same as above)
14308 Getattr request (using above sessionid)
14309 Getattr reply NFS4ERR_BAD_SESSION
- and then this just repeats...
The whole packet trace can be found here, in case you are interested:
https://people.freebsd.org/~rmacklem/linux.pcap

It seems to me that a successful CreateSession should always return
a new unique sessionid?

Hi Rick, thanks for the bug report.

CREATE_SESSION has a built-in reply cache to thwart replay attacks.
It can legitimately return the same sessionid as a previous request.
Granted, DESTROY_SESSION is supposed to wipe that reply cache...

I'd like to see if there's a test in pynfs that replicates or is close
to the series of operations in your trace so that I can reproduce on
my lab systems and watch it fail up close.

I constructed a pynfs test that does something similar to your
reproducer:

diff --git a/nfs4.1/server41tests/st_destroy_session.py b/nfs4.1/server41tests/st_destroy_session.py
index b8be62582366..014330e7d623 100644
--- a/nfs4.1/server41tests/st_destroy_session.py
+++ b/nfs4.1/server41tests/st_destroy_session.py

@@ -1,12 +1,33 @@
 from .st_create_session import create_session
 from xdrdef.nfs4_const import *
-from .environment import check, fail, create_file, open_file
+from .environment import check, fail, create_file, open_file, close_file
 from xdrdef.nfs4_type import open_owner4, openflag4, createhow4, open_claim4
 import nfs_ops
 op = nfs_ops.NFS4ops()
 import threading
 import rpc.rpc as rpc
 
+def testDestroyBasic(t, env):
+    """Ensure operations outside a session fail with BADSESSION
+
+    FLAGS: destroy_session all
+    CODE: DSESS1
+    """
+    c = env.c1.new_client(env.testname(t))
+    sess1 = c.create_session()
+    sess1.compound([op.reclaim_complete(FALSE)])
+    res = c.c.compound([op.destroy_session(sess1.sessionid)])
+    res = create_file(sess1, env.testname(t),
+                      access=OPEN4_SHARE_ACCESS_READ)
+    check(res, NFS4ERR_BADSESSION)
+    sess2 = c.create_session()
+    res = create_file(sess2, env.testname(t),
+                      access=OPEN4_SHARE_ACCESS_READ)
+    check(res)
+    fh = res.resarray[-1].object
+    open_stateid = res.resarray[-2].stateid
+    close_file(sess2, fh, stateid=open_stateid)
+
 def testDestroy(t, env):
     """
    - create a session

I'm not able to reproduce the problem on 5.19-rc5, but that
probably means there's something going on that we haven't
discovered yet.


--
Chuck Lever