[bug report] cifs: refactor create_sd_buf() and and avoid corrupting the buffer

[Dan Carpenter <dan.carpenter@oracle.com>]

Hello Ronnie Sahlberg,

The patch ea64370bcae1: "cifs: refactor create_sd_buf() and and avoid
corrupting the buffer" from Nov 30, 2020, leads to the following
Smatch static checker warning:

	fs/smbfs_client/smb2pdu.c:2425 create_sd_buf()
	warn: struct type mismatch 'smb3_acl vs cifs_acl'

fs/smbfs_client/smb2pdu.c
    2344 static struct crt_sd_ctxt *
    2345 create_sd_buf(umode_t mode, bool set_owner, unsigned int *len)
    2346 {
    2347         struct crt_sd_ctxt *buf;
    2348         __u8 *ptr, *aclptr;
    2349         unsigned int acelen, acl_size, ace_count;
    2350         unsigned int owner_offset = 0;
    2351         unsigned int group_offset = 0;
    2352         struct smb3_acl acl;
                 ^^^^^^^^^^^^^^^^^^^

    2353 
    2354         *len = roundup(sizeof(struct crt_sd_ctxt) + (sizeof(struct cifs_ace) * 4), 8);
    2355 
    2356         if (set_owner) {
    2357                 /* sizeof(struct owner_group_sids) is already multiple of 8 so no need to round */
    2358                 *len += sizeof(struct owner_group_sids);
    2359         }
    2360 
    2361         buf = kzalloc(*len, GFP_KERNEL);
    2362         if (buf == NULL)
    2363                 return buf;
    2364 
    2365         ptr = (__u8 *)&buf[1];
    2366         if (set_owner) {
    2367                 /* offset fields are from beginning of security descriptor not of create context */
    2368                 owner_offset = ptr - (__u8 *)&buf->sd;
    2369                 buf->sd.OffsetOwner = cpu_to_le32(owner_offset);
    2370                 group_offset = owner_offset + offsetof(struct owner_group_sids, group);
    2371                 buf->sd.OffsetGroup = cpu_to_le32(group_offset);
    2372 
    2373                 setup_owner_group_sids(ptr);
    2374                 ptr += sizeof(struct owner_group_sids);
    2375         } else {
    2376                 buf->sd.OffsetOwner = 0;
    2377                 buf->sd.OffsetGroup = 0;
    2378         }
    2379 
    2380         buf->ccontext.DataOffset = cpu_to_le16(offsetof(struct crt_sd_ctxt, sd));
    2381         buf->ccontext.NameOffset = cpu_to_le16(offsetof(struct crt_sd_ctxt, Name));
    2382         buf->ccontext.NameLength = cpu_to_le16(4);
    2383         /* SMB2_CREATE_SD_BUFFER_TOKEN is "SecD" */
    2384         buf->Name[0] = 'S';
    2385         buf->Name[1] = 'e';
    2386         buf->Name[2] = 'c';
    2387         buf->Name[3] = 'D';
    2388         buf->sd.Revision = 1;  /* Must be one see MS-DTYP 2.4.6 */
    2389 
    2390         /*
    2391          * ACL is "self relative" ie ACL is stored in contiguous block of memory
    2392          * and "DP" ie the DACL is present
    2393          */
    2394         buf->sd.Control = cpu_to_le16(ACL_CONTROL_SR | ACL_CONTROL_DP);
    2395 
    2396         /* offset owner, group and Sbz1 and SACL are all zero */
    2397         buf->sd.OffsetDacl = cpu_to_le32(ptr - (__u8 *)&buf->sd);
    2398         /* Ship the ACL for now. we will copy it into buf later. */
    2399         aclptr = ptr;
    2400         ptr += sizeof(struct cifs_acl);
    2401 
    2402         /* create one ACE to hold the mode embedded in reserved special SID */
    2403         acelen = setup_special_mode_ACE((struct cifs_ace *)ptr, (__u64)mode);
    2404         ptr += acelen;
    2405         acl_size = acelen + sizeof(struct smb3_acl);
                                            ^^^^^^^^^^^^^^^
    2406         ace_count = 1;
    2407 
    2408         if (set_owner) {
    2409                 /* we do not need to reallocate buffer to add the two more ACEs. plenty of space */
    2410                 acelen = setup_special_user_owner_ACE((struct cifs_ace *)ptr);
    2411                 ptr += acelen;
    2412                 acl_size += acelen;
    2413                 ace_count += 1;
    2414         }
    2415 
    2416         /* and one more ACE to allow access for authenticated users */
    2417         acelen = setup_authusers_ACE((struct cifs_ace *)ptr);
    2418         ptr += acelen;
    2419         acl_size += acelen;
    2420         ace_count += 1;
    2421 
    2422         acl.AclRevision = ACL_REVISION; /* See 2.4.4.1 of MS-DTYP */
    2423         acl.AclSize = cpu_to_le16(acl_size);
    2424         acl.AceCount = cpu_to_le16(ace_count);
--> 2425         memcpy(aclptr, &acl, sizeof(struct cifs_acl));
                                      ^^^^^^^^^^^^^^^^^^^^^^^
smb3_acl and cifs_acl structs are both 8 bytes, but their data is quite
different.

(This old warnings are all showing up as new warnings because the file
was moved).

    2426 
    2427         buf->ccontext.DataLength = cpu_to_le32(ptr - (__u8 *)&buf->sd);
    2428         *len = roundup(ptr - (__u8 *)buf, 8);
    2429 
    2430         return buf;
    2431 }

regards,
dan carpenter