linux-kernel - re: cifs: Retain old ACEs when converting between mode bits and ACL.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <1ca0f87e-83b3-b4dd-4448-b44f2a9d1698@canonical.com>
Date:   Wed, 24 Feb 2021 12:43:52 +0000
From:   Colin Ian King <colin.king@...onical.com>
To:     Shyam Prasad N <sprasad@...rosoft.com>
Cc:     Steve French <sfrench@...ba.org>, linux-cifs@...r.kernel.org,
        samba-technical@...ts.samba.org,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: re: cifs: Retain old ACEs when converting between mode bits and ACL.

Hi,

Static analysis on linux-next with Coverity had detected a potential
null pointer dereference with the following commit:

commit f5065508897a922327f32223082325d10b069ebc
Author: Shyam Prasad N <sprasad@...rosoft.com>
Date:   Fri Feb 12 04:38:43 2021 -0800

    cifs: Retain old ACEs when converting between mode bits and ACL.

The analysis is as follows:

1258 /* Convert permission bits from mode to equivalent CIFS ACL */
1259 static int build_sec_desc(struct cifs_ntsd *pntsd, struct cifs_ntsd
*pnntsd,
1260        __u32 secdesclen, __u32 *pnsecdesclen, __u64 *pnmode, kuid_t
uid, kgid_t gid,
1261        bool mode_from_sid, bool id_from_sid, int *aclflag)
1262 {
1263        int rc = 0;
1264        __u32 dacloffset;
1265        __u32 ndacloffset;
1266        __u32 sidsoffset;
1267        struct cifs_sid *owner_sid_ptr, *group_sid_ptr;
1268        struct cifs_sid *nowner_sid_ptr = NULL, *ngroup_sid_ptr = NULL;

    1. assign_zero: Assigning: dacl_ptr = NULL.

1269        struct cifs_acl *dacl_ptr = NULL;  /* no need for SACL ptr */
1270        struct cifs_acl *ndacl_ptr = NULL; /* no need for SACL ptr */
1271        char *end_of_acl = ((char *)pntsd) + secdesclen;
1272        u16 size = 0;
1273
1274        dacloffset = le32_to_cpu(pntsd->dacloffset);

    2. Condition dacloffset, taking false branch.

1275        if (dacloffset) {
1276                dacl_ptr = (struct cifs_acl *)((char *)pntsd +
dacloffset);
1277                if (end_of_acl < (char *)dacl_ptr +
le16_to_cpu(dacl_ptr->size)) {
1278                        cifs_dbg(VFS, "Existing ACL size is wrong.
Discarding old ACL\n");
1279                        dacl_ptr = NULL;

NOTE: dacl_ptr is set to NULL and dacloffset is true

1280                }
1281        }
1282
1283        owner_sid_ptr = (struct cifs_sid *)((char *)pntsd +
1284                        le32_to_cpu(pntsd->osidoffset));
1285        group_sid_ptr = (struct cifs_sid *)((char *)pntsd +
1286                        le32_to_cpu(pntsd->gsidoffset));
1287

    3. Condition pnmode, taking true branch.
    4. Condition *pnmode != 18446744073709551615ULL, taking false
branch.

1288        if (pnmode && *pnmode != NO_CHANGE_64) { /* chmod */
1289                ndacloffset = sizeof(struct cifs_ntsd);
1290                ndacl_ptr = (struct cifs_acl *)((char *)pnntsd +
ndacloffset);
1291                ndacl_ptr->revision =
1292                        dacloffset ? dacl_ptr->revision :
cpu_to_le16(ACL_REVISION);
1293
1294                ndacl_ptr->size = cpu_to_le16(0);
1295                ndacl_ptr->num_aces = cpu_to_le32(0);
1296
1297                rc = set_chmod_dacl(dacl_ptr, ndacl_ptr,
owner_sid_ptr, group_sid_ptr,
1298                                    pnmode, mode_from_sid);
1299
1300                sidsoffset = ndacloffset + le16_to_cpu(ndacl_ptr->size);
1301                /* copy the non-dacl portion of secdesc */
1302                *pnsecdesclen = copy_sec_desc(pntsd, pnntsd, sidsoffset,
1303                                NULL, NULL);
1304
1305                *aclflag |= CIFS_ACL_DACL;
1306        } else {
1307                ndacloffset = sizeof(struct cifs_ntsd);
1308                ndacl_ptr = (struct cifs_acl *)((char *)pnntsd +
ndacloffset);

    5. Condition dacloffset, taking false branch.

1309                ndacl_ptr->revision =
1310                        dacloffset ? dacl_ptr->revision :
cpu_to_le16(ACL_REVISION);

    Explicit null dereferenced (FORWARD_NULL)

    6. var_deref_op: Dereferencing null pointer dacl_ptr.

1311                ndacl_ptr->num_aces = dacl_ptr->num_aces;


Line 1309..1311, when dacloffset and dacl_ptr is null we hit a null ptr
dereference on dacl_ptr.