[PATCH 1/3] smb: client: show smb lease key in open_files output

[PATCH 1/3] smb: client: show smb lease key in open_files output

[Bharath SM]

Add the SMB lease key in /proc/fs/cifs/open_files for
debugging purposes.

Signed-off-by: Bharath SM <bharathsm@microsoft.com>
---
 fs/smb/client/cifs_debug.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/fs/smb/client/cifs_debug.c b/fs/smb/client/cifs_debug.c
index 1fb71d2d31b5..1383caa5a034 100644
--- a/fs/smb/client/cifs_debug.c
+++ b/fs/smb/client/cifs_debug.c
@@ -249,9 +249,9 @@ static int cifs_debug_files_proc_show(struct seq_file *m, void *v)
 	seq_puts(m, "# Format:\n");
 	seq_puts(m, "# <tree id> <ses id> <persistent fid> <flags> <count> <pid> <uid>");
 #ifdef CONFIG_CIFS_DEBUG2
-	seq_puts(m, " <filename> <lease> <mid>\n");
+	seq_puts(m, " <filename> <lease> <lease-key> <mid>\n");
 #else
-	seq_puts(m, " <filename> <lease>\n");
+	seq_puts(m, " <filename> <lease> <lease-key>\n");
 #endif /* CIFS_DEBUG2 */
 	spin_lock(&cifs_tcp_ses_lock);
 	list_for_each_entry(server, &cifs_tcp_ses_list, tcp_ses_list) {
@@ -274,6 +274,7 @@ static int cifs_debug_files_proc_show(struct seq_file *m, void *v)
 
 					/* Append lease/oplock caching state as RHW letters */
 					inode = d_inode(cfile->dentry);
+					cinode = NULL;
 					n = 0;
 					if (inode) {
 						cinode = CIFS_I(inode);
@@ -291,6 +292,12 @@ static int cifs_debug_files_proc_show(struct seq_file *m, void *v)
 					else
 						seq_puts(m, "NONE");
 
+					seq_puts(m, " ");
+					if (cinode && cinode->lease_granted)
+						seq_printf(m, "%pUl", cinode->lease_key);
+					else
+						seq_puts(m, "-");
+
 #ifdef CONFIG_CIFS_DEBUG2
 					seq_printf(m, " %llu", cfile->fid.mid);
 #endif /* CONFIG_CIFS_DEBUG2 */
-- 
2.48.1

[PATCH 2/3] smb: client: show smb lease key in open_dirs output

[Bharath SM]

Show cached directory smb lease key in /proc/fs/cifs/open_dirs
for debugging purposes.

Signed-off-by: Bharath SM <bharathsm@microsoft.com>
---
 fs/smb/client/cifs_debug.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/fs/smb/client/cifs_debug.c b/fs/smb/client/cifs_debug.c
index 1383caa5a034..7fdcaf9feb16 100644
--- a/fs/smb/client/cifs_debug.c
+++ b/fs/smb/client/cifs_debug.c
@@ -324,7 +324,7 @@ static int cifs_debug_dirs_proc_show(struct seq_file *m, void *v)
 
 	seq_puts(m, "# Version:1\n");
 	seq_puts(m, "# Format:\n");
-	seq_puts(m, "# <tree id> <sess id> <persistent fid> <path>\n");
+	seq_puts(m, "# <tree id> <sess id> <persistent fid> <lease-key> <path>\n");
 
 	spin_lock(&cifs_tcp_ses_lock);
 	list_for_each(stmp, &cifs_tcp_ses_list) {
@@ -343,11 +343,15 @@ static int cifs_debug_dirs_proc_show(struct seq_file *m, void *v)
 						(unsigned long)atomic_long_read(&cfids->total_dirents_entries),
 						(unsigned long long)atomic64_read(&cfids->total_dirents_bytes));
 				list_for_each_entry(cfid, &cfids->entries, entry) {
-					seq_printf(m, "0x%x 0x%llx 0x%llx     %s",
+					seq_printf(m, "0x%x 0x%llx 0x%llx ",
 						tcon->tid,
 						ses->Suid,
-						cfid->fid.persistent_fid,
-						cfid->path);
+						cfid->fid.persistent_fid);
+					if (cfid->has_lease)
+						seq_printf(m, "%pUl ", cfid->fid.lease_key);
+					else
+						seq_puts(m, "- ");
+					seq_printf(m, "%s", cfid->path);
 					if (cfid->file_all_info_is_valid)
 						seq_printf(m, "\tvalid file info");
 					if (cfid->dirents.is_valid)
-- 
2.48.1

[PATCH 3/3] smb: client: show directory lease state in /proc/fs/cifs/open_dirs

[Bharath SM]

Expose the SMB directory lease bits in the cached-dir proc
output for debugging purposes.

Signed-off-by: Bharath SM <bharathsm@microsoft.com>
---
 fs/smb/client/cached_dir.c |  7 +++++++
 fs/smb/client/cached_dir.h |  1 +
 fs/smb/client/cifs_debug.c | 23 +++++++++++++++++++----
 3 files changed, 27 insertions(+), 4 deletions(-)

diff --git a/fs/smb/client/cached_dir.c b/fs/smb/client/cached_dir.c
index b8ac7b7faf61..1703b5477fec 100644
--- a/fs/smb/client/cached_dir.c
+++ b/fs/smb/client/cached_dir.c
@@ -66,6 +66,7 @@ static struct cached_fid *find_or_create_cached_dir(struct cached_fids *cfids,
 	 * zero.
 	 */
 	cfid->has_lease = true;
+	cfid->lease_state = 0;
 
 	return cfid;
 }
@@ -350,6 +351,7 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon,
 		spin_unlock(&cfids->cfid_list_lock);
 		goto oshr_free;
 	}
+	cfid->lease_state = oplock;
 	qi_rsp = (struct smb2_query_info_rsp *)rsp_iov[1].iov_base;
 	if (le32_to_cpu(qi_rsp->OutputBufferLength) < sizeof(struct smb2_file_all_info)) {
 		spin_unlock(&cfids->cfid_list_lock);
@@ -388,6 +390,7 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon,
 			 * lease. Release one here, and the second below.
 			 */
 			cfid->has_lease = false;
+			cfid->lease_state = 0;
 			kref_put(&cfid->refcount, smb2_close_cached_fid);
 		}
 		spin_unlock(&cfids->cfid_list_lock);
@@ -478,6 +481,7 @@ void drop_cached_dir_by_name(const unsigned int xid, struct cifs_tcon *tcon,
 	spin_lock(&cfid->cfids->cfid_list_lock);
 	if (cfid->has_lease) {
 		cfid->has_lease = false;
+		cfid->lease_state = 0;
 		kref_put(&cfid->refcount, smb2_close_cached_fid);
 	}
 	spin_unlock(&cfid->cfids->cfid_list_lock);
@@ -577,6 +581,7 @@ void invalidate_all_cached_dirs(struct cifs_tcon *tcon)
 			 * so steal that reference.
 			 */
 			cfid->has_lease = false;
+			cfid->lease_state = 0;
 		} else
 			kref_get(&cfid->refcount);
 	}
@@ -632,6 +637,7 @@ bool cached_dir_lease_break(struct cifs_tcon *tcon, __u8 lease_key[16])
 			    cfid->fid.lease_key,
 			    SMB2_LEASE_KEY_SIZE)) {
 			cfid->has_lease = false;
+			cfid->lease_state = 0;
 			cfid->time = 0;
 			/*
 			 * We found a lease remove it from the list
@@ -738,6 +744,7 @@ static void cfids_laundromat_worker(struct work_struct *work)
 				 * server. Steal that reference.
 				 */
 				cfid->has_lease = false;
+				cfid->lease_state = 0;
 			} else
 				kref_get(&cfid->refcount);
 		}
diff --git a/fs/smb/client/cached_dir.h b/fs/smb/client/cached_dir.h
index 1e383db7c337..dd12e67870be 100644
--- a/fs/smb/client/cached_dir.h
+++ b/fs/smb/client/cached_dir.h
@@ -40,6 +40,7 @@ struct cached_fid {
 	bool is_open:1;
 	bool on_list:1;
 	bool file_all_info_is_valid:1;
+	u8 lease_state;
 	unsigned long time; /* jiffies of when lease was taken */
 	unsigned long last_access_time; /* jiffies of when last accessed */
 	struct kref refcount;
diff --git a/fs/smb/client/cifs_debug.c b/fs/smb/client/cifs_debug.c
index 7fdcaf9feb16..3ff17513f363 100644
--- a/fs/smb/client/cifs_debug.c
+++ b/fs/smb/client/cifs_debug.c
@@ -19,6 +19,7 @@
 #include "cifs_debug.h"
 #include "cifsfs.h"
 #include "fs_context.h"
+#include "smb2pdu.h"
 #ifdef CONFIG_CIFS_DFS_UPCALL
 #include "dfs_cache.h"
 #endif
@@ -320,11 +321,14 @@ static int cifs_debug_dirs_proc_show(struct seq_file *m, void *v)
 	struct cifs_tcon *tcon;
 	struct cached_fids *cfids;
 	struct cached_fid *cfid;
+	char lease[4];
+	int n;
+	u8 lease_state;
 	LIST_HEAD(entry);
 
 	seq_puts(m, "# Version:1\n");
 	seq_puts(m, "# Format:\n");
-	seq_puts(m, "# <tree id> <sess id> <persistent fid> <lease-key> <path>\n");
+	seq_puts(m, "# <tree id> <sess id> <persistent fid> <lease> <lease-key> <path>\n");
 
 	spin_lock(&cifs_tcp_ses_lock);
 	list_for_each(stmp, &cifs_tcp_ses_list) {
@@ -343,17 +347,28 @@ static int cifs_debug_dirs_proc_show(struct seq_file *m, void *v)
 						(unsigned long)atomic_long_read(&cfids->total_dirents_entries),
 						(unsigned long long)atomic64_read(&cfids->total_dirents_bytes));
 				list_for_each_entry(cfid, &cfids->entries, entry) {
-					seq_printf(m, "0x%x 0x%llx 0x%llx ",
+					lease_state = cfid->has_lease ? cfid->lease_state : 0;
+					n = 0;
+					if (lease_state & SMB2_LEASE_READ_CACHING_HE)
+						lease[n++] = 'R';
+					if (lease_state & SMB2_LEASE_HANDLE_CACHING_HE)
+						lease[n++] = 'H';
+					if (lease_state & SMB2_LEASE_WRITE_CACHING_HE)
+						lease[n++] = 'W';
+					lease[n] = '\0';
+
+					seq_printf(m, "0x%x 0x%llx 0x%llx %s ",
 						tcon->tid,
 						ses->Suid,
-						cfid->fid.persistent_fid);
+						cfid->fid.persistent_fid,
+						n ? lease : "NONE");
 					if (cfid->has_lease)
 						seq_printf(m, "%pUl ", cfid->fid.lease_key);
 					else
 						seq_puts(m, "- ");
 					seq_printf(m, "%s", cfid->path);
 					if (cfid->file_all_info_is_valid)
-						seq_printf(m, "\tvalid file info");
+						seq_puts(m, " valid file info");
 					if (cfid->dirents.is_valid)
 						seq_printf(m, ", valid dirents");
 					if (!list_empty(&cfid->dirents.entries))
-- 
2.48.1

Re: [PATCH 3/3] smb: client: show directory lease state in /proc/fs/cifs/open_dirs

[Paulo Alcantara]

Bharath SM <bharathsm.hsk@gmail.com> writes:

> Expose the SMB directory lease bits in the cached-dir proc
> output for debugging purposes.
>
> Signed-off-by: Bharath SM <bharathsm@microsoft.com>
> ---
>  fs/smb/client/cached_dir.c |  7 +++++++
>  fs/smb/client/cached_dir.h |  1 +
>  fs/smb/client/cifs_debug.c | 23 +++++++++++++++++++----
>  3 files changed, 27 insertions(+), 4 deletions(-)

Are you increasing cached_fid structure just for debugging purposes?
That makes no sense.

cached_fid structure has a dentry pointer, so what about accessing lease
flags as below

        u8 lease_state = CIFS_I(d_inode(cfid->dentry))->oplock;

Re: [PATCH 3/3] smb: client: show directory lease state in /proc/fs/cifs/open_dirs

[Enzo Matsumiya]

On 10/30, Paulo Alcantara wrote:
>Bharath SM <bharathsm.hsk@gmail.com> writes:
>
>> Expose the SMB directory lease bits in the cached-dir proc
>> output for debugging purposes.
>>
>> Signed-off-by: Bharath SM <bharathsm@microsoft.com>
>> ---
>>  fs/smb/client/cached_dir.c |  7 +++++++
>>  fs/smb/client/cached_dir.h |  1 +
>>  fs/smb/client/cifs_debug.c | 23 +++++++++++++++++++----
>>  3 files changed, 27 insertions(+), 4 deletions(-)
>
>Are you increasing cached_fid structure just for debugging purposes?
>That makes no sense.
>
>cached_fid structure has a dentry pointer, so what about accessing lease
>flags as below
>
>        u8 lease_state = CIFS_I(d_inode(cfid->dentry))->oplock;

Also, I don't think we can even get anything different than RH caching
for dirs.

Even on RH -> R lease breaks (IIRC this can happen), we don't handle it
and cfid is gone anyway.

Re: [PATCH 3/3] smb: client: show directory lease state in /proc/fs/cifs/open_dirs

[Paulo Alcantara]

Enzo Matsumiya <ematsumiya@suse.de> writes:

> On 10/30, Paulo Alcantara wrote:
>>Bharath SM <bharathsm.hsk@gmail.com> writes:
>>
>>> Expose the SMB directory lease bits in the cached-dir proc
>>> output for debugging purposes.
>>>
>>> Signed-off-by: Bharath SM <bharathsm@microsoft.com>
>>> ---
>>>  fs/smb/client/cached_dir.c |  7 +++++++
>>>  fs/smb/client/cached_dir.h |  1 +
>>>  fs/smb/client/cifs_debug.c | 23 +++++++++++++++++++----
>>>  3 files changed, 27 insertions(+), 4 deletions(-)
>>
>>Are you increasing cached_fid structure just for debugging purposes?
>>That makes no sense.
>>
>>cached_fid structure has a dentry pointer, so what about accessing lease
>>flags as below
>>
>>        u8 lease_state = CIFS_I(d_inode(cfid->dentry))->oplock;
>
> Also, I don't think we can even get anything different than RH caching
> for dirs.

Yes.

> Even on RH -> R lease breaks (IIRC this can happen), we don't handle it
> and cfid is gone anyway.

Yep, that can happen.  The code also doesn't seem to handle any sort of
reference count on the @cfid being dumped out, which would definitely
cause UAF.

Re: [PATCH 3/3] smb: client: show directory lease state in /proc/fs/cifs/open_dirs

[Shyam Prasad N]

Hi Enzo,

On Fri, Oct 31, 2025 at 3:42 AM Enzo Matsumiya <ematsumiya@suse.de> wrote:
>
> On 10/30, Paulo Alcantara wrote:
> >Bharath SM <bharathsm.hsk@gmail.com> writes:
> >
> >> Expose the SMB directory lease bits in the cached-dir proc
> >> output for debugging purposes.
> >>
> >> Signed-off-by: Bharath SM <bharathsm@microsoft.com>
> >> ---
> >>  fs/smb/client/cached_dir.c |  7 +++++++
> >>  fs/smb/client/cached_dir.h |  1 +
> >>  fs/smb/client/cifs_debug.c | 23 +++++++++++++++++++----
> >>  3 files changed, 27 insertions(+), 4 deletions(-)
> >
> >Are you increasing cached_fid structure just for debugging purposes?
> >That makes no sense.
> >
> >cached_fid structure has a dentry pointer, so what about accessing lease
> >flags as below
> >
> >        u8 lease_state = CIFS_I(d_inode(cfid->dentry))->oplock;
>
> Also, I don't think we can even get anything different than RH caching
> for dirs.
I don't think we can make an assumption about what all servers return.
I don't mind dumping this extra info about the lease state.
>
> Even on RH -> R lease breaks (IIRC this can happen), we don't handle it
> and cfid is gone anyway.
>


-- 
Regards,
Shyam

Re: [PATCH 3/3] smb: client: show directory lease state in /proc/fs/cifs/open_dirs

[Shyam Prasad N]

On Fri, Oct 31, 2025 at 3:28 AM Paulo Alcantara <pc@manguebit.org> wrote:
>
> Bharath SM <bharathsm.hsk@gmail.com> writes:
>
> > Expose the SMB directory lease bits in the cached-dir proc
> > output for debugging purposes.
> >
> > Signed-off-by: Bharath SM <bharathsm@microsoft.com>
> > ---
> >  fs/smb/client/cached_dir.c |  7 +++++++
> >  fs/smb/client/cached_dir.h |  1 +
> >  fs/smb/client/cifs_debug.c | 23 +++++++++++++++++++----
> >  3 files changed, 27 insertions(+), 4 deletions(-)
>
> Are you increasing cached_fid structure just for debugging purposes?
> That makes no sense.
>
> cached_fid structure has a dentry pointer, so what about accessing lease
> flags as below
>
>         u8 lease_state = CIFS_I(d_inode(cfid->dentry))->oplock;
>
I agree with Paulo here. We should avoid duplicating data if it's
already available elsewhere.

-- 
Regards,
Shyam

Re: [PATCH 3/3] smb: client: show directory lease state in /proc/fs/cifs/open_dirs

[Bharath SM]

On Thu, Oct 30, 2025 at 8:20 PM Shyam Prasad N <nspmangalore@gmail.com> wrote:
>
> On Fri, Oct 31, 2025 at 3:28 AM Paulo Alcantara <pc@manguebit.org> wrote:
> >
> > Bharath SM <bharathsm.hsk@gmail.com> writes:
> >
> > > Expose the SMB directory lease bits in the cached-dir proc
> > > output for debugging purposes.
> > >
> > > Signed-off-by: Bharath SM <bharathsm@microsoft.com>
> > > ---
> > >  fs/smb/client/cached_dir.c |  7 +++++++
> > >  fs/smb/client/cached_dir.h |  1 +
> > >  fs/smb/client/cifs_debug.c | 23 +++++++++++++++++++----
> > >  3 files changed, 27 insertions(+), 4 deletions(-)
> >
> > Are you increasing cached_fid structure just for debugging purposes?
> > That makes no sense.
> >
> > cached_fid structure has a dentry pointer, so what about accessing lease
> > flags as below
> >
> >         u8 lease_state = CIFS_I(d_inode(cfid->dentry))->oplock;
> >
> I agree with Paulo here. We should avoid duplicating data if it's
> already available elsewhere.

Thanks for your comments.

For directory leases, I don't see we are populating/storing lease
state anywhere including cinode
in current code, unless i am missing something. Today we check if we
have R lease then set
 has_lease = true.
I added a new u8 lease_state, the idea is to use this flag,
1) For showing state in debug files in this patch
2) Adding trace points to print lease state and other info, I don't
see any directory caching tracepoints
3) Importantly today we don't differentiate the directory caching when
we get R vs RH from the server, i think we need to.

We can probably repurpose the has_lease field by refactoring code.
But I am fine if we want to defer this patch until we have #2 and #3.

Re: [PATCH 3/3] smb: client: show directory lease state in /proc/fs/cifs/open_dirs

[Enzo Matsumiya]

On 10/31, Shyam Prasad N wrote:
>Hi Enzo,
>
>On Fri, Oct 31, 2025 at 3:42 AM Enzo Matsumiya <ematsumiya@suse.de> wrote:
>>
>> On 10/30, Paulo Alcantara wrote:
>> >Bharath SM <bharathsm.hsk@gmail.com> writes:
>> >
>> >> Expose the SMB directory lease bits in the cached-dir proc
>> >> output for debugging purposes.
>> >>
>> >> Signed-off-by: Bharath SM <bharathsm@microsoft.com>
>> >> ---
>> >>  fs/smb/client/cached_dir.c |  7 +++++++
>> >>  fs/smb/client/cached_dir.h |  1 +
>> >>  fs/smb/client/cifs_debug.c | 23 +++++++++++++++++++----
>> >>  3 files changed, 27 insertions(+), 4 deletions(-)
>> >
>> >Are you increasing cached_fid structure just for debugging purposes?
>> >That makes no sense.
>> >
>> >cached_fid structure has a dentry pointer, so what about accessing lease
>> >flags as below
>> >
>> >        u8 lease_state = CIFS_I(d_inode(cfid->dentry))->oplock;
>>
>> Also, I don't think we can even get anything different than RH caching
>> for dirs.
>I don't think we can make an assumption about what all servers return.
>I don't mind dumping this extra info about the lease state.

It's not an assumption, it's a fact -- if a server returns anything
different than RH for a dir lease, the server is broken and we shouldn't
even trust/cache it.

>> Even on RH -> R lease breaks (IIRC this can happen), we don't handle it
>> and cfid is gone anyway.

My point is a dir is either cached (RH, cfid->has_lease == true) or we
don't (lease break with 0 or R-cache only, cfid->has_lease == false).

So unless we're changing/handling the behaviour for RH -> R downgrades
(which I really think we shouldn't), creating such level of granularity
makes no sense and will only cause confusion (imagine seeing W-caching
for a dir).


Cheers,

Enzo

Re: [PATCH 3/3] smb: client: show directory lease state in /proc/fs/cifs/open_dirs

[Steve French]

dropped that patch from for-next (kept the other two small debug ones,
although those are low priority)

On Fri, Oct 31, 2025 at 7:55 AM Enzo Matsumiya <ematsumiya@suse.de> wrote:
>
> On 10/31, Shyam Prasad N wrote:
> >Hi Enzo,
> >
> >On Fri, Oct 31, 2025 at 3:42 AM Enzo Matsumiya <ematsumiya@suse.de> wrote:
> >>
> >> On 10/30, Paulo Alcantara wrote:
> >> >Bharath SM <bharathsm.hsk@gmail.com> writes:
> >> >
> >> >> Expose the SMB directory lease bits in the cached-dir proc
> >> >> output for debugging purposes.
> >> >>
> >> >> Signed-off-by: Bharath SM <bharathsm@microsoft.com>
> >> >> ---
> >> >>  fs/smb/client/cached_dir.c |  7 +++++++
> >> >>  fs/smb/client/cached_dir.h |  1 +
> >> >>  fs/smb/client/cifs_debug.c | 23 +++++++++++++++++++----
> >> >>  3 files changed, 27 insertions(+), 4 deletions(-)
> >> >
> >> >Are you increasing cached_fid structure just for debugging purposes?
> >> >That makes no sense.
> >> >
> >> >cached_fid structure has a dentry pointer, so what about accessing lease
> >> >flags as below
> >> >
> >> >        u8 lease_state = CIFS_I(d_inode(cfid->dentry))->oplock;
> >>
> >> Also, I don't think we can even get anything different than RH caching
> >> for dirs.
> >I don't think we can make an assumption about what all servers return.
> >I don't mind dumping this extra info about the lease state.
>
> It's not an assumption, it's a fact -- if a server returns anything
> different than RH for a dir lease, the server is broken and we shouldn't
> even trust/cache it.
>
> >> Even on RH -> R lease breaks (IIRC this can happen), we don't handle it
> >> and cfid is gone anyway.
>
> My point is a dir is either cached (RH, cfid->has_lease == true) or we
> don't (lease break with 0 or R-cache only, cfid->has_lease == false).
>
> So unless we're changing/handling the behaviour for RH -> R downgrades
> (which I really think we shouldn't), creating such level of granularity
> makes no sense and will only cause confusion (imagine seeing W-caching
> for a dir).
>
>
> Cheers,
>
> Enzo



-- 
Thanks,

Steve