2023-05-26 20:56 GMT+09:00, Dan Carpenter <dan.carpenter@xxxxxxxxxx>:
> Hello Namjae Jeon,
>
> The patch e2f34481b24d: "cifsd: add server-side procedures for SMB3"
> from Mar 16, 2021, leads to the following Smatch static checker
> warning:
>
> fs/smb/server/smbacl.c:1296 smb_check_perm_dacl()
> error: 'posix_acls' dereferencing possible ERR_PTR()
> fs/smb/server/vfs.c:1323 ksmbd_vfs_make_xattr_posix_acl()
> error: 'posix_acls' dereferencing possible ERR_PTR()
> fs/smb/server/vfs.c:1830 ksmbd_vfs_inherit_posix_acl()
> error: 'acls' dereferencing possible ERR_PTR()
I will fix it.
Thanks for your report!
>
> fs/smb/server/smbacl.c
> 1281 if (*pdaccess & FILE_MAXIMAL_ACCESS_LE && found) {
> 1282 granted = READ_CONTROL | WRITE_DAC |
> FILE_READ_ATTRIBUTES |
> 1283 DELETE;
> 1284
> 1285 granted |= le32_to_cpu(ace->access_req);
> 1286
> 1287 if (!pdacl->num_aces)
> 1288 granted = GENERIC_ALL_FLAGS;
> 1289 }
> 1290
> 1291 if (IS_ENABLED(CONFIG_FS_POSIX_ACL)) {
> 1292 posix_acls = get_inode_acl(d_inode(path->dentry),
> ACL_TYPE_ACCESS);
>
> __get_acl() returns a mix of error pointers and NULL. I don't really
> understand the rules here. There are no comments explaining it.
>
> 1293 if (posix_acls && !found) {
> 1294 unsigned int id = -1;
> 1295
> --> 1296 pa_entry = posix_acls->a_entries;
> ^^^^^^^^^^^^
> Potential error pointer dereference.
>
> 1297 for (i = 0; i < posix_acls->a_count; i++,
> pa_entry++) {
> 1298 if (pa_entry->e_tag == ACL_USER)
> 1299 id =
> posix_acl_uid_translate(idmap, pa_entry);
> 1300 else if (pa_entry->e_tag ==
> ACL_GROUP)
> 1301 id =
> posix_acl_gid_translate(idmap, pa_entry);
> 1302 else
>
> regards,
> dan carpenter
>
