cifsd: fix static checker warning from smb_check_perm_dacl()
authorNamjae Jeon <namjae.jeon@samsung.com>
Fri, 19 Mar 2021 04:52:12 +0000 (13:52 +0900)
committerSteve French <stfrench@microsoft.com>
Tue, 11 May 2021 00:15:21 +0000 (19:15 -0500)
Dan reported static checker warning:

  fs/cifsd/smbacl.c:1140 smb_check_perm_dacl()
  error: we previously assumed 'pntsd' could be null (see line 1137)

This patch validate bounds of pntsd buffer.

Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Namjae Jeon <namjae.jeon@samsung.com>
Signed-off-by: Steve French <stfrench@microsoft.com>
fs/cifsd/smbacl.c

index 294c5a8fe9af93b44eafae07eadd72bffd7e6867..77c79cf4afd01a3d2506e0f3e337fb5ca804ac0f 100644 (file)
@@ -800,9 +800,13 @@ int parse_sec_desc(struct smb_ntsd *pntsd, int acl_len,
                 le32_to_cpu(pntsd->gsidoffset),
                 le32_to_cpu(pntsd->sacloffset), dacloffset);
 
-       if (dacloffset && dacl_ptr)
+       if (dacloffset) {
+               if (end_of_acl <= (char *)dacl_ptr ||
+                   end_of_acl < (char *)dacl_ptr + le16_to_cpu(dacl_ptr->size))
+                       return -EIO;
                total_ace_size =
                        le16_to_cpu(dacl_ptr->size) - sizeof(struct smb_acl);
+       }
 
        pntsd_type = le16_to_cpu(pntsd->type);
 
@@ -1131,13 +1135,28 @@ int smb_check_perm_dacl(struct ksmbd_conn *conn, struct dentry *dentry,
        struct smb_ace *others_ace = NULL;
        struct posix_acl_entry *pa_entry;
        unsigned int sid_type = SIDOWNER;
+       char *end_of_acl;
 
        ksmbd_debug(SMB, "check permission using windows acl\n");
        acl_size = ksmbd_vfs_get_sd_xattr(conn, dentry, &pntsd);
-       if (acl_size <= 0 || (pntsd && !pntsd->dacloffset))
+       if (acl_size <= 0 || !pntsd || !pntsd->dacloffset) {
+               kfree(pntsd);
                return 0;
+       }
 
        pdacl = (struct smb_acl *)((char *)pntsd + le32_to_cpu(pntsd->dacloffset));
+       end_of_acl = ((char *)pntsd) + acl_size;
+       if (end_of_acl <= (char *)pdacl) {
+               kfree(pntsd);
+               return 0;
+       }
+
+       if (end_of_acl < (char *)pdacl + le16_to_cpu(pdacl->size) ||
+           le16_to_cpu(pdacl->size) < sizeof(struct smb_acl)) {
+               kfree(pntsd);
+               return 0;
+       }
+
        if (!pdacl->num_aces) {
                if (!(le16_to_cpu(pdacl->size) - sizeof(struct smb_acl)) &&
                    *pdaccess & ~(FILE_READ_CONTROL_LE | FILE_WRITE_DAC_LE)) {
@@ -1156,6 +1175,8 @@ int smb_check_perm_dacl(struct ksmbd_conn *conn, struct dentry *dentry,
                for (i = 0; i < le32_to_cpu(pdacl->num_aces); i++) {
                        granted |= le32_to_cpu(ace->access_req);
                        ace = (struct smb_ace *) ((char *)ace + le16_to_cpu(ace->size));
+                       if (end_of_acl < (char *)ace)
+                               goto err_out;
                }
 
                if (!pdacl->num_aces)
@@ -1177,6 +1198,8 @@ int smb_check_perm_dacl(struct ksmbd_conn *conn, struct dentry *dentry,
                        others_ace = ace;
 
                ace = (struct smb_ace *) ((char *)ace + le16_to_cpu(ace->size));
+               if (end_of_acl < (char *)ace)
+                       goto err_out;
        }
 
        if (*pdaccess & FILE_MAXIMAL_ACCESS_LE && found) {