Re: [RFC PATCH samba] smbd: break directory leases before sending notify events

Ralph Boehme <[email protected]>
Newsgroups org.kernel.vger.linux-cifs
Message-ID <[email protected]>
Hi!

Thanks for looking into this and trying to address the issue!

Unfortunately, iirc it won't be as simple due to the way Samba 
internally integrates inotify with changes coming in over SMB. I have 
this sitting on my todo list for the next months, the plan is add a new 
change-notify backend based on fanotify, where, in contrast to inotify, 
change notifications contain the pid of the triggering process. We need 
that information as a differentiator in notifyd to correctly drive the 
machinery.

As an example, iirc with just your patch, we would break the dirlease 
for changes done by the client owning it and the change correctly 
referencing it via the parent dirlease key in the handle triggering the 
change.

Thanks!
-slow

On 8/10/26 10:29 AM, ChenXiaoSong wrote:
> From: ChenXiaoSong <[email protected]>
> 
> Reproducer:
> 
>    1. samba: `smb.conf`:
>       [global]
>           smb3 directory leases = yes
>    2. samba:
>       systemctl start smb # fedora
>    3. Windows 11 File Explorer:
>       Mount the share and enter the top-level directory of the mount point.
>    4. samba: touch /export/file
>    5. Windows 11 File Explorer:
>       `file` does not appear in the top-level directory of the mount point.
> 
> Windows can keep directory data in a cache while it has a directory
> lease. A file change made on the server does not use the SMB request
> path, so the lease is not broken.
> 
> The server sends a notify event, but Windows may still use the old data.
> Some new files are then not shown in File Explorer.
> 
> Signed-off-by: ChenXiaoSong <[email protected]>
> ---
>   source3/smbd/notify.c           |  7 ++++++-
>   source3/smbd/notifyd/fcn_wait.c |  2 +-
>   source3/smbd/notifyd/notifyd.c  |  2 +-
>   source3/smbd/proto.h            |  1 +
>   source3/smbd/smb2_oplock.c      | 37 +++++++++++++++++++++++++++++++++
>   5 files changed, 46 insertions(+), 3 deletions(-)
> 
> diff --git a/source3/smbd/notify.c b/source3/smbd/notify.c
> index 5970a7a9220..24dcedb9dba 100644
> --- a/source3/smbd/notify.c
> +++ b/source3/smbd/notify.c
> @@ -262,10 +262,15 @@ static struct files_struct *notify_fsp_cb(struct files_struct *fsp,
>   					  void *private_data)
>   {
>   	struct notify_fsp_state *state = private_data;
> +	uint32_t action = state->e->action;
>   
>   	if (fsp == state->notified_fsp) {
>   		DBG_DEBUG("notify_callback called for %s\n", fsp_str_dbg(fsp));
> -		notify_fsp(fsp, state->when, state->e->action, state->e->path);
> +		if (action & NOTIFY_ACTION_DIRLEASE_BREAK) {
> +			contend_dirleases_by_fsp(fsp);
> +			action &= ~NOTIFY_ACTION_DIRLEASE_BREAK;
> +		}
> +		notify_fsp(fsp, state->when, action, state->e->path);
>   		return fsp;
>   	}
>   
> diff --git a/source3/smbd/notifyd/fcn_wait.c b/source3/smbd/notifyd/fcn_wait.c
> index e32240d911a..6b5b6e66d43 100644
> --- a/source3/smbd/notifyd/fcn_wait.c
> +++ b/source3/smbd/notifyd/fcn_wait.c
> @@ -256,7 +256,7 @@ NTSTATUS fcn_wait_recv(
>   		*when = evt->msg.when;
>   	}
>   	if (action != NULL) {
> -		*action = evt->msg.action;
> +		*action = evt->msg.action & ~NOTIFY_ACTION_DIRLEASE_BREAK;
>   	}
>   
>   	DLIST_REMOVE(state->events, evt);
> diff --git a/source3/smbd/notifyd/notifyd.c b/source3/smbd/notifyd/notifyd.c
> index 0b07ab3e435..a42a7a9162e 100644
> --- a/source3/smbd/notifyd/notifyd.c
> +++ b/source3/smbd/notifyd/notifyd.c
> @@ -579,7 +579,7 @@ static void notifyd_sys_callback(struct sys_notify_context *ctx,
>   
>   	msg = (struct notify_trigger_msg) {
>   		.when = timespec_current(),
> -		.action = ev->action,
> +		.action = ev->action | NOTIFY_ACTION_DIRLEASE_BREAK,
>   		.filter = filter,
>   	};
>   
> diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h
> index 7ced3a3ffd5..012039c7a5f 100644
> --- a/source3/smbd/proto.h
> +++ b/source3/smbd/proto.h
> @@ -730,6 +730,7 @@ void smbd_contend_level2_oplocks_end(files_struct *fsp,
>   void contend_dirleases(struct connection_struct *conn,
>   		       const struct smb_filename *smb_fname,
>   		       const struct smb2_lease *lease);
> +void contend_dirleases_by_fsp(struct files_struct *fsp);
>   bool init_oplocks(struct smbd_server_connection *sconn);
>   void init_kernel_oplocks(struct smbd_server_connection *sconn);
>   
> diff --git a/source3/smbd/smb2_oplock.c b/source3/smbd/smb2_oplock.c
> index 2489090af55..4de33889eff 100644
> --- a/source3/smbd/smb2_oplock.c
> +++ b/source3/smbd/smb2_oplock.c
> @@ -1398,6 +1398,43 @@ void contend_dirleases(struct connection_struct *conn,
>   	TALLOC_FREE(lck);
>   }
>   
> +void contend_dirleases_by_fsp(struct files_struct *fsp)
> +{
> +	struct dirlease_break_state state = {
> +		.sconn = fsp->conn->sconn,
> +		.file_id = fsp->file_id,
> +	};
> +	struct share_mode_lock *lck = NULL;
> +	uint32_t access_mask, share_mode;
> +	bool ok;
> +
> +	if (!lp_smb3_directory_leases() || !fsp->fsp_flags.is_directory) {
> +		return;
> +	}
> +
> +	lck = get_existing_share_mode_lock(talloc_tos(), state.file_id);
> +	if (lck == NULL) {
> +		return;
> +	}
> +
> +	ok = share_mode_forall_leases(lck, do_dirlease_break_to_none, &state);
> +	if (!ok) {
> +		DBG_WARNING("share_mode_forall_leases failed\n");
> +	}
> +
> +	share_mode_flags_get(lck,
> +			     &access_mask,
> +			     &share_mode,
> +			     NULL);
> +	share_mode_flags_set(lck,
> +			     access_mask,
> +			     share_mode,
> +			     state.total_lease_types,
> +			     NULL);
> +
> +	TALLOC_FREE(lck);
> +}
> +
>   /****************************************************************************
>    This function is called on any file modification or lock request. If a file
>    is level 2 oplocked then it must tell all other level 2 holders to break to
OpenPGP_signature.asc (application/pgp-signature, 840 B)
-----BEGIN PGP SIGNATURE-----

wsF5BAABCAAjFiEE+uLGCIokJSBRxVnkqh6bcSY5nkYFAmqCv7UFAwAAAAAACgkQqh6bcSY5nkYS
1w//cD2XaDfyyYMuY7O+EdDMOT9kCG6KSfxyOBwHy4U+PzjbcsAtpj7fqVj0bSUzGiNmxsoY8CnK
sxT48yl6bqnV492GOangltcVtB355MtU1MUxuY4JLEoUJ6W8RYtX+aPz/7DY4KnKd/wrfJnVKW8U
CyAxZ3J7ayKga9qmXfXlDQftArna1WZlF9nHkLr9Q85iVgFs20s25yIQqnCWxfEPhxGQ3DVQJ7dD
SgTT8LB9PlhYQ6c62Tykc57Ryh9r3CfVgyMXd1XCJYJZT+1IOJWQybdUD0IVLY/Jztfkc+3kLXHP
ZUDclnYCg2rchvnHDYbthnM4Bucu8MdStDdIVRBihUWcKZdRFaJODOhlU1hTn9caQYqMu8vUVEQ5
8tJL+7oArGbLsogT48bpj9+wPT2FWjNV2k3u5ox+SSim2WFs3+cn9TgZjZfV1Ro7iRch5c4bm5fw
ohlGxHA+A+e6Ohw/+Czs2SzAg621ChZE54vibM5pH5ZuPe+hIuTcFOS4lVzqEVcC3FR0Ur3byzV+
Z3qULOCGLNICtw8mwRkhR7HkplXVeJVEfHgEsIMApo42eWHvSitr2+FCzmfqGyal4tuoreGL67u4
Qbnhv4uE7mGqugFIb0PADRRYv2Zia0m/im4RfPZCTmbBMwBWqq60a2fpKLy0j/n2O2iKVyBx7TtA
PUg=
=z9wx
-----END PGP SIGNATURE-----
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.