just changed the one place you noted (to keep the patch smaller) - see attached. On Wed, Jun 23, 2021 at 9:17 AM Steve French wrote: > > On Wed, Jun 23, 2021 at 6:41 AM Aurélien Aptel wrote: > > > > Steve French writes: > > > > > smb3: fix possible access to uninitialized pointer to DACL > > > > > > dacl_ptr can be null so we must check for it (ie if dacloffset is > > > set) everywhere dacl_ptr is > > > used in build_sec_desc - and we were missing one check > > > > > > Addresses-Coverity: 1475598 ("Explicit null dereference") > > > > Looks OK since dacl_ptr is only set if dacloffset is set but it would > > be clearer if you check for dacl_ptr directly no? Any reason you are > > checking this way? > > > > I think this is clearer, unless I'm missing something: > > > > ndacl_ptr->num_aces = dacl_ptr ? dacl_ptr->num_aces : 0; > > I agree that your suggestion is clearer but I was trying to match the > existing checks in the same code. > Will change both to your suggestion which is clearer. > > > > > > > > > > > -- > > > Thanks, > > > > > > Steve > > > From ec06cb04376e5abc927a9b85dd768ce8728965bb Mon Sep 17 00:00:00 2001 > > > From: Steve French > > > Date: Tue, 22 Jun 2021 17:54:50 -0500 > > > Subject: [PATCH] smb3: fix possible access to uninitialized pointer to DACL > > > > > > dacl_ptr can be null so we must check for it everywhere it is > > > used in build_sec_desc. > > > > > > Addresses-Coverity: 1475598 ("Explicit null dereference") > > > Signed-off-by: Steve French > > > --- > > > fs/cifs/cifsacl.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/fs/cifs/cifsacl.c b/fs/cifs/cifsacl.c > > > index 784407f9280f..25a8139336fa 100644 > > > --- a/fs/cifs/cifsacl.c > > > +++ b/fs/cifs/cifsacl.c > > > @@ -1308,7 +1308,7 @@ static int build_sec_desc(struct cifs_ntsd *pntsd, struct cifs_ntsd *pnntsd, > > > ndacl_ptr = (struct cifs_acl *)((char *)pnntsd + ndacloffset); > > > ndacl_ptr->revision = > > > dacloffset ? dacl_ptr->revision : cpu_to_le16(ACL_REVISION); > > > - ndacl_ptr->num_aces = dacl_ptr->num_aces; > > > + ndacl_ptr->num_aces = dacloffset ? dacl_ptr->num_aces : 0; > > > > > > if (uid_valid(uid)) { /* chown */ > > > uid_t id; > > > -- > > > 2.30.2 > > > > > > > -- > > 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) > > > > > -- > Thanks, > > Steve -- Thanks, Steve