[PATCH] cifs: Fix uninitialized variable in set_chmod_dacl()

[PATCH] cifs: Fix uninitialized variable in set_chmod_dacl()

[Dan Carpenter]

Initialize the "nmode" variable earlier to prevent an uninitialized
variable bug when we do "size += setup_special_mode_ACE(pntace, nmode);"

Fixes: 253374f7557e ("cifs: Fix unix perm bits to cifsacl conversion for "other" bits.")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
 fs/cifs/cifsacl.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/fs/cifs/cifsacl.c b/fs/cifs/cifsacl.c
index d7a6d0f533bf..8410db328e5e 100644
--- a/fs/cifs/cifsacl.c
+++ b/fs/cifs/cifsacl.c
@@ -920,6 +920,13 @@ static int set_chmod_dacl(struct cifs_acl *pndacl, struct cifs_sid *pownersid,
 	__u64 deny_group_mode = 0;
 	bool sticky_set = false;
 
+	/*
+	 * We'll try to keep the mode as requested by the user.
+	 * But in cases where we cannot meaningfully convert that
+	 * into ACL, return back the updated mode, so that it is
+	 * updated in the inode.
+	 */
+	nmode = *pnmode;
 	pnndacl = (struct cifs_acl *)((char *)pndacl + sizeof(struct cifs_acl));
 
 	if (modefromsid) {
@@ -931,14 +938,6 @@ static int set_chmod_dacl(struct cifs_acl *pndacl, struct cifs_sid *pownersid,
 		goto set_size;
 	}
 
-	/*
-	 * We'll try to keep the mode as requested by the user.
-	 * But in cases where we cannot meaningfully convert that
-	 * into ACL, return back the updated mode, so that it is
-	 * updated in the inode.
-	 */
-	nmode = *pnmode;
-
 	if (!memcmp(pownersid, pgrpsid, sizeof(struct cifs_sid))) {
 		/*
 		 * Case when owner and group SIDs are the same.
-- 
2.29.2

RE: [EXTERNAL] [PATCH] cifs: Fix uninitialized variable in set_chmod_dacl()

[Shyam Prasad]

Hi Dan,

Thanks for the catch. This patch is already in for-next branch here:
https://git.samba.org/sfrench/cifs-2.6.git

Can you please pull the latest and verify?

Regards,
Shyam

-----Original Message-----
From: Dan Carpenter <dan.carpenter@oracle.com> 
Sent: Monday, December 14, 2020 5:26 PM
To: Steve French <sfrench@samba.org>; Shyam Prasad <Shyam.Prasad@microsoft.com>
Cc: linux-cifs@vger.kernel.org; samba-technical@lists.samba.org; kernel-janitors@vger.kernel.org
Subject: [EXTERNAL] [PATCH] cifs: Fix uninitialized variable in set_chmod_dacl()

Initialize the "nmode" variable earlier to prevent an uninitialized variable bug when we do "size += setup_special_mode_ACE(pntace, nmode);"

Fixes: 253374f7557e ("cifs: Fix unix perm bits to cifsacl conversion for "other" bits.")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
 fs/cifs/cifsacl.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/fs/cifs/cifsacl.c b/fs/cifs/cifsacl.c index d7a6d0f533bf..8410db328e5e 100644
--- a/fs/cifs/cifsacl.c
+++ b/fs/cifs/cifsacl.c
@@ -920,6 +920,13 @@ static int set_chmod_dacl(struct cifs_acl *pndacl, struct cifs_sid *pownersid,
 	__u64 deny_group_mode = 0;
 	bool sticky_set = false;
 
+	/*
+	 * We'll try to keep the mode as requested by the user.
+	 * But in cases where we cannot meaningfully convert that
+	 * into ACL, return back the updated mode, so that it is
+	 * updated in the inode.
+	 */
+	nmode = *pnmode;
 	pnndacl = (struct cifs_acl *)((char *)pndacl + sizeof(struct cifs_acl));
 
 	if (modefromsid) {
@@ -931,14 +938,6 @@ static int set_chmod_dacl(struct cifs_acl *pndacl, struct cifs_sid *pownersid,
 		goto set_size;
 	}
 
-	/*
-	 * We'll try to keep the mode as requested by the user.
-	 * But in cases where we cannot meaningfully convert that
-	 * into ACL, return back the updated mode, so that it is
-	 * updated in the inode.
-	 */
-	nmode = *pnmode;
-
 	if (!memcmp(pownersid, pgrpsid, sizeof(struct cifs_sid))) {
 		/*
 		 * Case when owner and group SIDs are the same.
--
2.29.2

Re: [PATCH] cifs: Fix uninitialized variable in set_chmod_dacl()

[Aurélien Aptel]

Hi Dan,

Dan Carpenter via samba-technical <samba-technical@lists.samba.org>
writes:
> Initialize the "nmode" variable earlier to prevent an uninitialized
> variable bug when we do "size += setup_special_mode_ACE(pntace, nmode);"
>
> Fixes: 253374f7557e ("cifs: Fix unix perm bits to cifsacl conversion for "other" bits.")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

The current version of Steve's for-next already sets nmode earlier in
commit 0f22053e811ca ("cifs: Fix unix perm bits to cifsacl conversion
for "other" bits."). I believe this patch can be ignored.

Cheers,
-- 
Aurélien Aptel / SUSE Labs Samba Team
GPG: 1839 CB5F 9F5B FB9B AA97  8C99 03C8 A49B 521B D5D3
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg, DE
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah HRB 247165 (AG München)