[PATCH 1/7] smb: Use loff_t for directory position in cached_dirents

[PATCH 1/7] smb: Use loff_t for directory position in cached_dirents

[Bharath SM]

Change the pos field in struct cached_dirents from int to loff_t
to support large directory offsets. This avoids overflow and
matches kernel conventions for directory positions.

Signed-off-by: Bharath SM <bharathsm@microsoft.com>
---
 fs/smb/client/cached_dir.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/smb/client/cached_dir.h b/fs/smb/client/cached_dir.h
index bc8a812ff95f..a28f7cae3caa 100644
--- a/fs/smb/client/cached_dir.h
+++ b/fs/smb/client/cached_dir.h
@@ -26,7 +26,7 @@ struct cached_dirents {
 			    * open file instance.
 			    */
 	struct mutex de_mutex;
-	int pos;		 /* Expected ctx->pos */
+	loff_t pos;		 /* Expected ctx->pos */
 	struct list_head entries;
 };
 
-- 
2.43.0

[PATCH 2/7] smb: minor fix to use sizeof to initialize flags_string buffer

[Bharath SM]

Replaced hardcoded length with sizeof(flags_string).

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

diff --git a/fs/smb/client/cifs_debug.c b/fs/smb/client/cifs_debug.c
index c0196be0e65f..3fdf75737d43 100644
--- a/fs/smb/client/cifs_debug.c
+++ b/fs/smb/client/cifs_debug.c
@@ -1105,7 +1105,7 @@ static ssize_t cifs_security_flags_proc_write(struct file *file,
 	if ((count < 1) || (count > 11))
 		return -EINVAL;
 
-	memset(flags_string, 0, 12);
+	memset(flags_string, 0, sizeof(flags_string));
 
 	if (copy_from_user(flags_string, buffer, count))
 		return -EFAULT;
-- 
2.43.0

Re: [PATCH 2/7] smb: minor fix to use sizeof to initialize flags_string buffer

[Paulo Alcantara]

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

> Replaced hardcoded length with sizeof(flags_string).
>
> Signed-off-by: Bharath SM <bharathsm@microsoft.com>
> ---
>  fs/smb/client/cifs_debug.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Paulo Alcantara (Red Hat) <pc@manguebit.org>

Re: [PATCH 2/7] smb: minor fix to use sizeof to initialize flags_string buffer

[Shyam Prasad N]

On Thu, Jun 19, 2025 at 9:06 PM Bharath SM <bharathsm.hsk@gmail.com> wrote:
>
> Replaced hardcoded length with sizeof(flags_string).
>
> Signed-off-by: Bharath SM <bharathsm@microsoft.com>
> ---
>  fs/smb/client/cifs_debug.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/smb/client/cifs_debug.c b/fs/smb/client/cifs_debug.c
> index c0196be0e65f..3fdf75737d43 100644
> --- a/fs/smb/client/cifs_debug.c
> +++ b/fs/smb/client/cifs_debug.c
> @@ -1105,7 +1105,7 @@ static ssize_t cifs_security_flags_proc_write(struct file *file,
>         if ((count < 1) || (count > 11))
>                 return -EINVAL;
>
> -       memset(flags_string, 0, 12);
> +       memset(flags_string, 0, sizeof(flags_string));
>
>         if (copy_from_user(flags_string, buffer, count))
>                 return -EFAULT;
> --
> 2.43.0
>
>

Why not define the size of the flags_string as a constant too?

-- 
Regards,
Shyam

[PATCH 3/7] smb: minor fix to use SMB2_NTLMV2_SESSKEY_SIZE for auth_key size

[Bharath SM]

Replaced hardcoded value 16 with SMB2_NTLMV2_SESSKEY_SIZE
in the auth_key definition and memcpy call.

Signed-off-by: Bharath SM <bharathsm@microsoft.com>
---
 fs/smb/client/cifs_ioctl.h | 2 +-
 fs/smb/client/ioctl.c      | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/smb/client/cifs_ioctl.h b/fs/smb/client/cifs_ioctl.h
index 26327442e383..b51ce64fcccf 100644
--- a/fs/smb/client/cifs_ioctl.h
+++ b/fs/smb/client/cifs_ioctl.h
@@ -61,7 +61,7 @@ struct smb_query_info {
 struct smb3_key_debug_info {
 	__u64	Suid;
 	__u16	cipher_type;
-	__u8	auth_key[16]; /* SMB2_NTLMV2_SESSKEY_SIZE */
+	__u8	auth_key[SMB2_NTLMV2_SESSKEY_SIZE];
 	__u8	smb3encryptionkey[SMB3_SIGN_KEY_SIZE];
 	__u8	smb3decryptionkey[SMB3_SIGN_KEY_SIZE];
 } __packed;
diff --git a/fs/smb/client/ioctl.c b/fs/smb/client/ioctl.c
index 56439da4f119..0a9935ce05a5 100644
--- a/fs/smb/client/ioctl.c
+++ b/fs/smb/client/ioctl.c
@@ -506,7 +506,7 @@ long cifs_ioctl(struct file *filep, unsigned int command, unsigned long arg)
 				le16_to_cpu(tcon->ses->server->cipher_type);
 			pkey_inf.Suid = tcon->ses->Suid;
 			memcpy(pkey_inf.auth_key, tcon->ses->auth_key.response,
-					16 /* SMB2_NTLMV2_SESSKEY_SIZE */);
+				  SMB2_NTLMV2_SESSKEY_SIZE);
 			memcpy(pkey_inf.smb3decryptionkey,
 			      tcon->ses->smb3decryptionkey, SMB3_SIGN_KEY_SIZE);
 			memcpy(pkey_inf.smb3encryptionkey,
-- 
2.43.0

Re: [PATCH 3/7] smb: minor fix to use SMB2_NTLMV2_SESSKEY_SIZE for auth_key size

[Paulo Alcantara]

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

> Replaced hardcoded value 16 with SMB2_NTLMV2_SESSKEY_SIZE
> in the auth_key definition and memcpy call.
>
> Signed-off-by: Bharath SM <bharathsm@microsoft.com>
> ---
>  fs/smb/client/cifs_ioctl.h | 2 +-
>  fs/smb/client/ioctl.c      | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/fs/smb/client/cifs_ioctl.h b/fs/smb/client/cifs_ioctl.h
> index 26327442e383..b51ce64fcccf 100644
> --- a/fs/smb/client/cifs_ioctl.h
> +++ b/fs/smb/client/cifs_ioctl.h
> @@ -61,7 +61,7 @@ struct smb_query_info {
>  struct smb3_key_debug_info {
>  	__u64	Suid;
>  	__u16	cipher_type;
> -	__u8	auth_key[16]; /* SMB2_NTLMV2_SESSKEY_SIZE */
> +	__u8	auth_key[SMB2_NTLMV2_SESSKEY_SIZE];
>  	__u8	smb3encryptionkey[SMB3_SIGN_KEY_SIZE];
>  	__u8	smb3decryptionkey[SMB3_SIGN_KEY_SIZE];
>  } __packed;
> diff --git a/fs/smb/client/ioctl.c b/fs/smb/client/ioctl.c
> index 56439da4f119..0a9935ce05a5 100644
> --- a/fs/smb/client/ioctl.c
> +++ b/fs/smb/client/ioctl.c
> @@ -506,7 +506,7 @@ long cifs_ioctl(struct file *filep, unsigned int command, unsigned long arg)
>  				le16_to_cpu(tcon->ses->server->cipher_type);
>  			pkey_inf.Suid = tcon->ses->Suid;
>  			memcpy(pkey_inf.auth_key, tcon->ses->auth_key.response,
> -					16 /* SMB2_NTLMV2_SESSKEY_SIZE */);
> +				  SMB2_NTLMV2_SESSKEY_SIZE);
                                  ^ sizeof(pkey_in.auth_key)?

Reviewed-by: Paulo Alcantara (Red Hat) <pc@manguebit.org>

Re: [PATCH 3/7] smb: minor fix to use SMB2_NTLMV2_SESSKEY_SIZE for auth_key size

[Shyam Prasad N]

On Thu, Jun 19, 2025 at 9:09 PM Bharath SM <bharathsm.hsk@gmail.com> wrote:
>
> Replaced hardcoded value 16 with SMB2_NTLMV2_SESSKEY_SIZE
> in the auth_key definition and memcpy call.
>
> Signed-off-by: Bharath SM <bharathsm@microsoft.com>
> ---
>  fs/smb/client/cifs_ioctl.h | 2 +-
>  fs/smb/client/ioctl.c      | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/fs/smb/client/cifs_ioctl.h b/fs/smb/client/cifs_ioctl.h
> index 26327442e383..b51ce64fcccf 100644
> --- a/fs/smb/client/cifs_ioctl.h
> +++ b/fs/smb/client/cifs_ioctl.h
> @@ -61,7 +61,7 @@ struct smb_query_info {
>  struct smb3_key_debug_info {
>         __u64   Suid;
>         __u16   cipher_type;
> -       __u8    auth_key[16]; /* SMB2_NTLMV2_SESSKEY_SIZE */
> +       __u8    auth_key[SMB2_NTLMV2_SESSKEY_SIZE];
>         __u8    smb3encryptionkey[SMB3_SIGN_KEY_SIZE];
>         __u8    smb3decryptionkey[SMB3_SIGN_KEY_SIZE];
>  } __packed;
> diff --git a/fs/smb/client/ioctl.c b/fs/smb/client/ioctl.c
> index 56439da4f119..0a9935ce05a5 100644
> --- a/fs/smb/client/ioctl.c
> +++ b/fs/smb/client/ioctl.c
> @@ -506,7 +506,7 @@ long cifs_ioctl(struct file *filep, unsigned int command, unsigned long arg)
>                                 le16_to_cpu(tcon->ses->server->cipher_type);
>                         pkey_inf.Suid = tcon->ses->Suid;
>                         memcpy(pkey_inf.auth_key, tcon->ses->auth_key.response,
> -                                       16 /* SMB2_NTLMV2_SESSKEY_SIZE */);
> +                                 SMB2_NTLMV2_SESSKEY_SIZE);
>                         memcpy(pkey_inf.smb3decryptionkey,
>                               tcon->ses->smb3decryptionkey, SMB3_SIGN_KEY_SIZE);
>                         memcpy(pkey_inf.smb3encryptionkey,
> --
> 2.43.0
>
>
Looks good to me.

-- 
Regards,
Shyam

[PATCH 4/7] smb: add NULL check after kzalloc in cifsConvertToUTF16

[Bharath SM]

Added a check to return -ENOMEM if kzalloc for wchar_to fails

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

diff --git a/fs/smb/client/cifs_unicode.c b/fs/smb/client/cifs_unicode.c
index 4cc6e0896fad..7bc2268d6881 100644
--- a/fs/smb/client/cifs_unicode.c
+++ b/fs/smb/client/cifs_unicode.c
@@ -466,6 +466,8 @@ cifsConvertToUTF16(__le16 *target, const char *source, int srclen,
 		return cifs_strtoUTF16(target, source, PATH_MAX, cp);
 
 	wchar_to = kzalloc(6, GFP_KERNEL);
+	if (wchar_to == NULL)
+		return -ENOMEM;
 
 	for (i = 0; i < srclen; j++) {
 		src_char = source[i];
-- 
2.43.0

Re: [PATCH 4/7] smb: add NULL check after kzalloc in cifsConvertToUTF16

[Paulo Alcantara]

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

> Added a check to return -ENOMEM if kzalloc for wchar_to fails
>
> Signed-off-by: Bharath SM <bharathsm@microsoft.com>
> ---
>  fs/smb/client/cifs_unicode.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/fs/smb/client/cifs_unicode.c b/fs/smb/client/cifs_unicode.c
> index 4cc6e0896fad..7bc2268d6881 100644
> --- a/fs/smb/client/cifs_unicode.c
> +++ b/fs/smb/client/cifs_unicode.c
> @@ -466,6 +466,8 @@ cifsConvertToUTF16(__le16 *target, const char *source, int srclen,
>  		return cifs_strtoUTF16(target, source, PATH_MAX, cp);
>  
>  	wchar_to = kzalloc(6, GFP_KERNEL);
> +	if (wchar_to == NULL)
> +		return -ENOMEM;

I wouldn't do that as there are several places that rely on
cifsConvertToUTF16() returning >= 0 and some other places that don't
even check its return value.

What about having it defined as

        wchar_t wchar_to[3] = {};

and then getting rid of the memory allocation altogether?

Re: [PATCH 4/7] smb: add NULL check after kzalloc in cifsConvertToUTF16

[Bharath SM]

makes sense. thanks. will update.


On Thu, Jun 19, 2025 at 9:24 AM Paulo Alcantara <pc@manguebit.org> wrote:
>
> Bharath SM <bharathsm.hsk@gmail.com> writes:
>
> > Added a check to return -ENOMEM if kzalloc for wchar_to fails
> >
> > Signed-off-by: Bharath SM <bharathsm@microsoft.com>
> > ---
> >  fs/smb/client/cifs_unicode.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/fs/smb/client/cifs_unicode.c b/fs/smb/client/cifs_unicode.c
> > index 4cc6e0896fad..7bc2268d6881 100644
> > --- a/fs/smb/client/cifs_unicode.c
> > +++ b/fs/smb/client/cifs_unicode.c
> > @@ -466,6 +466,8 @@ cifsConvertToUTF16(__le16 *target, const char *source, int srclen,
> >               return cifs_strtoUTF16(target, source, PATH_MAX, cp);
> >
> >       wchar_to = kzalloc(6, GFP_KERNEL);
> > +     if (wchar_to == NULL)
> > +             return -ENOMEM;
>
> I wouldn't do that as there are several places that rely on
> cifsConvertToUTF16() returning >= 0 and some other places that don't
> even check its return value.
>
> What about having it defined as
>
>         wchar_t wchar_to[3] = {};
>
> and then getting rid of the memory allocation altogether?

Re: [PATCH 4/7] smb: add NULL check after kzalloc in cifsConvertToUTF16

[Shyam Prasad N]

On Thu, Jun 19, 2025 at 9:06 PM Bharath SM <bharathsm.hsk@gmail.com> wrote:
>
> Added a check to return -ENOMEM if kzalloc for wchar_to fails
>
> Signed-off-by: Bharath SM <bharathsm@microsoft.com>
> ---
>  fs/smb/client/cifs_unicode.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/fs/smb/client/cifs_unicode.c b/fs/smb/client/cifs_unicode.c
> index 4cc6e0896fad..7bc2268d6881 100644
> --- a/fs/smb/client/cifs_unicode.c
> +++ b/fs/smb/client/cifs_unicode.c
> @@ -466,6 +466,8 @@ cifsConvertToUTF16(__le16 *target, const char *source, int srclen,
>                 return cifs_strtoUTF16(target, source, PATH_MAX, cp);
>
>         wchar_to = kzalloc(6, GFP_KERNEL);
> +       if (wchar_to == NULL)
> +               return -ENOMEM;
>
>         for (i = 0; i < srclen; j++) {
>                 src_char = source[i];
> --
> 2.43.0
>
>
Looks good to me.

-- 
Regards,
Shyam

[PATCH 5/7] cifs: minor cleanup to remove unused permission bit macros

[Bharath SM]

Dropped READ_BIT, WRITE_BIT, EXEC_BIT, UBITSHIFT,
and GBITSHIFT from cifsacl.h as they are unused

Signed-off-by: Bharath SM <bharathsm@microsoft.com>
---
 fs/smb/client/cifsacl.h | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/fs/smb/client/cifsacl.h b/fs/smb/client/cifsacl.h
index 31b51a8fc256..3d828f655e97 100644
--- a/fs/smb/client/cifsacl.h
+++ b/fs/smb/client/cifsacl.h
@@ -11,17 +11,10 @@
 
 #include "../common/smbacl.h"
 
-#define READ_BIT        0x4
-#define WRITE_BIT       0x2
-#define EXEC_BIT        0x1
-
 #define ACL_OWNER_MASK 0700
 #define ACL_GROUP_MASK 0070
 #define ACL_EVERYONE_MASK 0007
 
-#define UBITSHIFT	6
-#define GBITSHIFT	3
-
 /*
  * Security Descriptor length containing DACL with 3 ACEs (one each for
  * owner, group and world).
-- 
2.43.0

[PATCH 6/7] smb: Fix potential divide-by-zero issue when iface_min_speed is 0

[Bharath SM]

Address potential divide-by-zero issue when iface_min_speed is 0
by adding proper handling to prevent undefined behavior.

Signed-off-by: Bharath SM <bharathsm@microsoft.com>
---
 fs/smb/client/cifs_debug.c | 2 +-
 fs/smb/client/sess.c       | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/smb/client/cifs_debug.c b/fs/smb/client/cifs_debug.c
index 3fdf75737d43..bc56f315e2e0 100644
--- a/fs/smb/client/cifs_debug.c
+++ b/fs/smb/client/cifs_debug.c
@@ -599,7 +599,7 @@ static int cifs_debug_data_proc_show(struct seq_file *m, void *v)
 				seq_printf(m, "\n\t%d)", ++j);
 				cifs_dump_iface(m, iface);
 
-				iface_weight = iface->speed / iface_min_speed;
+				iface_weight = iface_min_speed ? (iface->speed / iface_min_speed) : 0;
 				seq_printf(m, "\t\tWeight (cur,total): (%zu,%zu)"
 					   "\n\t\tAllocated channels: %u\n",
 					   iface->weight_fulfilled,
diff --git a/fs/smb/client/sess.c b/fs/smb/client/sess.c
index ec0db32c7d98..697170be6591 100644
--- a/fs/smb/client/sess.c
+++ b/fs/smb/client/sess.c
@@ -218,7 +218,7 @@ int cifs_try_adding_channels(struct cifs_ses *ses)
 				continue;
 
 			/* check if we already allocated enough channels */
-			iface_weight = iface->speed / iface_min_speed;
+			iface_weight = iface_min_speed ? (iface->speed / iface_min_speed) : 0;
 
 			if (iface->weight_fulfilled >= iface_weight)
 				continue;
@@ -387,7 +387,7 @@ cifs_chan_update_iface(struct cifs_ses *ses, struct TCP_Server_Info *server)
 		}
 
 		/* check if we already allocated enough channels */
-		iface_weight = iface->speed / iface_min_speed;
+		iface_weight = iface_min_speed ? (iface->speed / iface_min_speed) : 0;
 
 		if (iface->weight_fulfilled >= iface_weight)
 			continue;
-- 
2.43.0

Re: [PATCH 6/7] smb: Fix potential divide-by-zero issue when iface_min_speed is 0

[Paulo Alcantara]

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

> Address potential divide-by-zero issue when iface_min_speed is 0
> by adding proper handling to prevent undefined behavior.
>
> Signed-off-by: Bharath SM <bharathsm@microsoft.com>
> ---
>  fs/smb/client/cifs_debug.c | 2 +-
>  fs/smb/client/sess.c       | 4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)

Isn't be7a6a776695 ("smb: client: fix oops due to unset link speed")
enough to prevent that already?

Re: [PATCH 6/7] smb: Fix potential divide-by-zero issue when iface_min_speed is 0

[Bharath SM]

On Thu, Jun 19, 2025 at 9:01 AM Paulo Alcantara <pc@manguebit.org> wrote:
>
> Bharath SM <bharathsm.hsk@gmail.com> writes:
>
> > Address potential divide-by-zero issue when iface_min_speed is 0
> > by adding proper handling to prevent undefined behavior.
> >
> > Signed-off-by: Bharath SM <bharathsm@microsoft.com>
> > ---
> >  fs/smb/client/cifs_debug.c | 2 +-
> >  fs/smb/client/sess.c       | 4 ++--
> >  2 files changed, 3 insertions(+), 3 deletions(-)
>
> Isn't be7a6a776695 ("smb: client: fix oops due to unset link speed")
> enough to prevent that already?

Right, I didn't notice this check. I think this fix should take of the issue.

Re: [PATCH 6/7] smb: Fix potential divide-by-zero issue when iface_min_speed is 0

[Shyam Prasad N]

On Thu, Jun 19, 2025 at 9:06 PM Bharath SM <bharathsm.hsk@gmail.com> wrote:
>
> Address potential divide-by-zero issue when iface_min_speed is 0
> by adding proper handling to prevent undefined behavior.
>
> Signed-off-by: Bharath SM <bharathsm@microsoft.com>
> ---
>  fs/smb/client/cifs_debug.c | 2 +-
>  fs/smb/client/sess.c       | 4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/fs/smb/client/cifs_debug.c b/fs/smb/client/cifs_debug.c
> index 3fdf75737d43..bc56f315e2e0 100644
> --- a/fs/smb/client/cifs_debug.c
> +++ b/fs/smb/client/cifs_debug.c
> @@ -599,7 +599,7 @@ static int cifs_debug_data_proc_show(struct seq_file *m, void *v)
>                                 seq_printf(m, "\n\t%d)", ++j);
>                                 cifs_dump_iface(m, iface);
>
> -                               iface_weight = iface->speed / iface_min_speed;
> +                               iface_weight = iface_min_speed ? (iface->speed / iface_min_speed) : 0;
>                                 seq_printf(m, "\t\tWeight (cur,total): (%zu,%zu)"
>                                            "\n\t\tAllocated channels: %u\n",
>                                            iface->weight_fulfilled,
> diff --git a/fs/smb/client/sess.c b/fs/smb/client/sess.c
> index ec0db32c7d98..697170be6591 100644
> --- a/fs/smb/client/sess.c
> +++ b/fs/smb/client/sess.c
> @@ -218,7 +218,7 @@ int cifs_try_adding_channels(struct cifs_ses *ses)
>                                 continue;
>
>                         /* check if we already allocated enough channels */
> -                       iface_weight = iface->speed / iface_min_speed;
> +                       iface_weight = iface_min_speed ? (iface->speed / iface_min_speed) : 0;
>
>                         if (iface->weight_fulfilled >= iface_weight)
>                                 continue;
> @@ -387,7 +387,7 @@ cifs_chan_update_iface(struct cifs_ses *ses, struct TCP_Server_Info *server)
>                 }
>
>                 /* check if we already allocated enough channels */
> -               iface_weight = iface->speed / iface_min_speed;
> +               iface_weight = iface_min_speed ? (iface->speed / iface_min_speed) : 0;
>
>                 if (iface->weight_fulfilled >= iface_weight)
>                         continue;
> --
> 2.43.0
>
>

Good spot.
Might want to add the Fixes tag:
Fixes: a6d8fb54a515 ("cifs: distribute channels across interfaces
based on speed")

-- 
Regards,
Shyam

[PATCH 7/7] smb: Fix memory allocation and ACL handling in cifs_xattr_set

[Bharath SM]

If set_acl is NULL, then there is a chance that memory allocated
for pacl memory doesn't get freed. Ensure proper memory deallocation
for pacl in `cifs_xattr_set` to avoid leaks.

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

diff --git a/fs/smb/client/xattr.c b/fs/smb/client/xattr.c
index b88fa04f5792..4d2b70020776 100644
--- a/fs/smb/client/xattr.c
+++ b/fs/smb/client/xattr.c
@@ -174,6 +174,7 @@ static int cifs_xattr_set(const struct xattr_handler *handler,
 		pacl = kmalloc(size, GFP_KERNEL);
 		if (!pacl) {
 			rc = -ENOMEM;
+			goto out;
 		} else {
 			memcpy(pacl, value, size);
 			if (pTcon->ses->server->ops->set_acl) {
@@ -211,8 +212,8 @@ static int cifs_xattr_set(const struct xattr_handler *handler,
 			}
 			if (rc == 0) /* force revalidate of the inode */
 				CIFS_I(inode)->time = 0;
-			kfree(pacl);
 		}
+		kfree(pacl);
 		break;
 	}
 	}
-- 
2.43.0

Re: [PATCH 7/7] smb: Fix memory allocation and ACL handling in cifs_xattr_set

[Paulo Alcantara]

Hi Bharath,

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

> If set_acl is NULL, then there is a chance that memory allocated
> for pacl memory doesn't get freed. Ensure proper memory deallocation
> for pacl in `cifs_xattr_set` to avoid leaks.

If ->set_acl is NULL, then the following is executed

			} else {
				rc = -EOPNOTSUPP;
			}
			if (rc == 0) /* force revalidate of the inode */
				CIFS_I(inode)->time = 0;
			kfree(pacl);

Can you elaborate where the potential leak is?

Re: [PATCH 1/7] smb: Use loff_t for directory position in cached_dirents

[Paulo Alcantara]

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

> Change the pos field in struct cached_dirents from int to loff_t
> to support large directory offsets. This avoids overflow and
> matches kernel conventions for directory positions.
>
> Signed-off-by: Bharath SM <bharathsm@microsoft.com>
> ---
>  fs/smb/client/cached_dir.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Paulo Alcantara (Red Hat) <pc@manguebit.org>

Re: [PATCH 1/7] smb: Use loff_t for directory position in cached_dirents

[Shyam Prasad N]

On Thu, Jun 19, 2025 at 9:26 PM Bharath SM <bharathsm.hsk@gmail.com> wrote:
>
> Change the pos field in struct cached_dirents from int to loff_t
> to support large directory offsets. This avoids overflow and
> matches kernel conventions for directory positions.
>
> Signed-off-by: Bharath SM <bharathsm@microsoft.com>
> ---
>  fs/smb/client/cached_dir.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/smb/client/cached_dir.h b/fs/smb/client/cached_dir.h
> index bc8a812ff95f..a28f7cae3caa 100644
> --- a/fs/smb/client/cached_dir.h
> +++ b/fs/smb/client/cached_dir.h
> @@ -26,7 +26,7 @@ struct cached_dirents {
>                             * open file instance.
>                             */
>         struct mutex de_mutex;
> -       int pos;                 /* Expected ctx->pos */
> +       loff_t pos;              /* Expected ctx->pos */
>         struct list_head entries;
>  };
>
> --
> 2.43.0
>
>
Looks good to me.

-- 
Regards,
Shyam