Project

General

Profile

Bug #5548

Attempt to read ACLs from Illumos NFS client is toxic

Added by Alexander Kolbasov almost 5 years ago. Updated almost 5 years ago.

Status:
New
Priority:
Normal
Assignee:
-
Category:
nfs - NFS server and client
Start date:
2015-01-20
Due date:
% Done:

0%

Estimated time:
Difficulty:
Medium
Tags:
needs-triage

Description

When you mount using NFSv3 illumos client to illumos server and attempt to cp -p some file (using GNU cp which tries to copy ACLs) the client panics with

panic message: copyout: kaddr argument below kernelbase

> ffffff4c01e10440::findstack -v
stack pointer for thread ffffff4c01e10440: ffffff01ec86bd80
  ffffff01ec86be40 cacl+0x4bc(4, 3, 8119f10, ffffff6b778f7e00, ffffff01ec86be5c)
  ffffff01ec86bea0 facl+0x60(3, 4, 3, 8119f10)
  ffffff01ec86bf00 _sys_sysenter_post_swapgs+0x241()

The op was ACE_GETACL. We panic in copout() and the code was:

static int
cacl(int cmd, int nentries, void *aclbufp, vnode_t *vp, int *rv)
{
...
    case ACE_GETACL:
        if (aclbufp == NULL)
            return (EFAULT);

        vsecattr.vsa_mask = VSA_ACE | VSA_ACECNT;
        if (error = VOP_GETSECATTR(vp, &vsecattr, 0, CRED(), NULL))
            return (error);

        aclbsize = vsecattr.vsa_aclcnt * sizeof (ace_t);
        if (vsecattr.vsa_aclcnt > nentries) {
            error = ENOSPC;
            goto errout;
        }

        if (aclbsize > 0) {
            if ((error = copyout(vsecattr.vsa_aclentp,
                aclbufp, aclbsize)) != 0) {
                goto errout;
            }
        }
...

Here is what is actually going on.
When we run GNU cp command with '-p' flag it requests file properties, including its ACL info. This creates RPC request to the server. The server replies and we decode the reply in the following way:

bool_t
xdr_secattr(XDR *xdrs, vsecattr_t *objp)
{
    uint_t count;

    if (!xdr_u_int(xdrs, &objp->vsa_mask))
        return (FALSE);
    if (!xdr_int(xdrs, &objp->vsa_aclcnt))
        return (FALSE);
    if (objp->vsa_aclentp != NULL) 
        count = (uint_t)objp->vsa_aclcnt;
    else
        count = 0;
    if (!xdr_array(xdrs, (char **)&objp->vsa_aclentp, &count,
        NFS_ACL_MAX_ENTRIES, sizeof (aclent_t), (xdrproc_t)xdr_aclent))
        return (FALSE);
    if (count != 0 && count != (uint_t)objp->vsa_aclcnt) {
        /*
         * Assign the actual array size to vsa_aclcnt before
         * aborting on error
         */
        objp->vsa_aclcnt = (int)count;
        return (FALSE);
    }
...

Now the xdd_array() function reads the size of the array from the xdr header and in this case it is zero. So the length reported in vsa_alccnt is 3 and length of the actual array is 0. Since the count is now 0 (xdr_array changes the count from 3 to 0) the check above doesn't catch the problem and we continue with visa_alccnt set to 3 and the via_aclentp being NULL. Later on we see that we have 3 entries and try to copyout them from a NULL pointer. Not good.
So there are two bugs here - NFS server is returning inconsistent data and the NFS client deserializer doesn't handle this case correctly.

The reason the server is returning inconsistent data is because it isn't supposed to return NFSv4 ACLs. I'll file a separate issue for this.


Related issues

Related to illumos gate - Bug #5549: NFS Server returns inconsistent information for ACLPROC_GETACL requestNew2015-01-20

Actions

History

#1

Updated by Alexander Kolbasov almost 5 years ago

I think the right fix for this is to reject NFSv4 style attributes for V3 and fail in xdd_secattr(). In this case the upper layers will actually simulate v4 style attributes locally.

#2

Updated by Marcel Telka almost 5 years ago

  • Category set to nfs - NFS server and client

Also available in: Atom PDF