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
