[PATCH] smb: client: fix first failure in negotiation after server reboot

zhangjian (CG) posted 1 patch 7 months, 4 weeks ago
fs/smb/client/connect.c | 1 +
1 file changed, 1 insertion(+)
[PATCH] smb: client: fix first failure in negotiation after server reboot
Posted by zhangjian (CG) 7 months, 4 weeks ago
After fabc4ed200f9, server_unresponsive add a condition to check whether 
client need to reconnect depending on server->lstrp. When client failed 
to reconnect in 180s, client will abort connection and update server->lstrp 
for the last time. In the following scene, server->lstrp is too 
old, which may cause failure for the first negotiation.

client                                                 | server
-------------------------------------------------------+------------------
mount to cifs server                                   |
ls                                                     |
                                                       | reboot
    stuck for 180s and return EHOSTDOWN                |
    abort connection and update server->lstrp          |
                                                       | sleep 21s
                                                       | service smb restart
ls                                                     |
    smb_negotiate                                      |
        server_unresponsive cause reconnect [in cifsd] |
        ( tcpStatus == CifsInNegotiate &&              |
	            jiffies > server->lstrp + 20s )        |
        cifs_sync_mid_result return EAGAIN             |
    smb_negotiate return EHOSTDOWN                     |
ls failed                                              |

The condition (tcpStatus == CifsInNegotiate && jiffies > server->lstrp + 20s)
expect client stay in CifsInNegotiate state for more than 20s. So we update 
server->lstrp before last switching into CifsInNegotiate state to avoid 
this failure.

Fixes: fabc4ed200f9 ("smb: client: fix hang in wait_for_response() for 
negproto")
Signed-off-by: zhangjian <zhangjian496@huawei.com>
---
 fs/smb/client/connect.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/smb/client/connect.c b/fs/smb/client/connect.c
index 28bc33496..f9aef60f1 100644
--- a/fs/smb/client/connect.c
+++ b/fs/smb/client/connect.c
@@ -4193,6 +4193,7 @@ cifs_negotiate_protocol(const unsigned int xid, struct cifs_ses *ses,
 		return 0;
 	}
 
+	server->lstrp = jiffies;
 	server->tcpStatus = CifsInNegotiate;
 	spin_unlock(&server->srv_lock);
 
-- 
2.33.0
[Question] nfsacl: why deny owner mode when deny user
Posted by zhangjian (CG) 2 months ago
When user read bit is denied by nfs4_setfacl, owner read bit is also
denied.
Example:

[root@localhost ~]# nfs4_getfacl test/a
# file: test/a
A::OWNER@:rwatTcCy
A::1000:rwatcy
A::GROUP@:rtcy
A::EVERYONE@:rtcy

[root@localhost ~]# nfs4_setfacl -a D::1000:r test/a
[root@localhost ~]# nfs4_getfacl test/a
# file: test/a
D::OWNER@:r
A::OWNER@:watTcCy
D::1000:r
A::1000:watcy
A::GROUP@:rtcy
A::EVERYONE@:rtcy

In function process_one_v4_ace, I see read bit is denied for owner:
case ACL_USER:
	i = find_uid(state, state->users, ace->who);
	if (ace->type == NFS4_ACE_ACCESS_ALLOWED_ACE_TYPE) {
		allow_bits(&state->users->aces[i].perms, mask);
	} else {
		deny_bits(&state->users->aces[i].perms, mask);
		mask = state->users->aces[i].perms.deny;
		deny_bits(&state->owner, mask);
	}
This change is commit in 09229ed. But I wonder why it is implemented
like this.
Re: [Question] nfsacl: why deny owner mode when deny user
Posted by Chuck Lever 1 month, 2 weeks ago
[ bfields@redhat.com dropped because it's a dead address ]

On Mon, Dec 8, 2025, at 4:56 AM, zhangjian (CG) wrote:
> When user read bit is denied by nfs4_setfacl, owner read bit is also
> denied.
> Example:
>
> [root@localhost ~]# nfs4_getfacl test/a
> # file: test/a
> A::OWNER@:rwatTcCy
> A::1000:rwatcy
> A::GROUP@:rtcy
> A::EVERYONE@:rtcy
>
> [root@localhost ~]# nfs4_setfacl -a D::1000:r test/a
> [root@localhost ~]# nfs4_getfacl test/a
> # file: test/a
> D::OWNER@:r
> A::OWNER@:watTcCy
> D::1000:r
> A::1000:watcy
> A::GROUP@:rtcy
> A::EVERYONE@:rtcy
>
> In function process_one_v4_ace, I see read bit is denied for owner:
> case ACL_USER:
> 	i = find_uid(state, state->users, ace->who);
> 	if (ace->type == NFS4_ACE_ACCESS_ALLOWED_ACE_TYPE) {
> 		allow_bits(&state->users->aces[i].perms, mask);
> 	} else {
> 		deny_bits(&state->users->aces[i].perms, mask);
> 		mask = state->users->aces[i].perms.deny;
> 		deny_bits(&state->owner, mask);
> 	}
> This change is commit in 09229ed. But I wonder why it is implemented
> like this.

I'm not an ACL expert. But here's a stab at an answer.

NFSD must translate incoming NFSv4 ACLs to POSIX ACLs before
they can be stored by local POSIX file systems. The root issue
is the semantic mismatch between NFSv4 ACLs and POSIX ACLs. 

In particular, NFSv4 ACL evaluation is strictly ordered
top-to-bottom. A DENY ACE early in the list blocks access even
if a later ALLOW would grant it.

However, POSIX ACL evaluation uses a fixed priority:
1. If user is the file owner -> use ACL_USER_OBJ entry
2. Else if user matches a named user -> use that ACL_USER
   entry (masked)
3. Else if user's groups match -> use group entries (masked)
4. Else -> use ACL_OTHER

For example:
  DENY uid=1000: READ
  ALLOW OWNER@: READ

Under NFSv4 semantics, if the file owner IS uid=1000, they are
denied READ (the DENY comes first in the ordered evaluation).

But in POSIX ACL semantics, the owner entry (ACL_USER_OBJ) is
checked before any named user entries. If the code only denied
READ to the named user entry without also denying it to the
owner entry, then when uid=1000 owns the file, they would get
READ through the owner check -- bypassing the intended denial.

Commit 09229edb68a3 explicitly states:
> errs on the side of restricting permissions...the posix acl
> produced should be the most permissive acl that is not more
> permissive than the given nfsv4 acl.

The algorithm doesn't know at ACL-setting time whether the
owner might match a named user (or might later via chown), so
it defensively denies to both. This guarantees the POSIX ACL
is never more permissive than the NFSv4 ACL was intended to be.

The downside is that when the owner is NOT the named user, the
owner's permissions are unnecessarily restricted. This is the
cost of the conservative approach: it may deny more than
strictly necessary, but it will never allow more than intended.

A "perfect" translation isn't possible because the semantic
models differ fundamentally. The algorithm  chose correctness
over permissiveness.


-- 
Chuck Lever