Bug 8848 - get_acl_blob in source3/modules/vfs_acl_xattr.c does not handle XATTRs larger than 1024 bytes
get_acl_blob in source3/modules/vfs_acl_xattr.c does not handle XATTRs larger...
Status: RESOLVED INVALID
Product: Samba 3.6
Classification: Unclassified
Component: File services
unspecified
All All
: P5 normal
: ---
Assigned To: Volker Lendecke
Samba QA Contact
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-04-06 22:39 UTC by Richard Sharpe
Modified: 2012-04-06 23:02 UTC (History)
0 users

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Richard Sharpe 2012-04-06 22:39:38 UTC
If an SD encodes to more than 1024 bytes it will be successfully stored, but upon retrieval will be truncated. 

Then the routine that unmarshals it will fail.

Since the underlying SMB_VFS_xGETXATTR routines are supposed to return the actual length of the XATTR, we can detect this and retry with a correctly sized buffer.

I will prepare a patch and attach it to this bug.
Comment 1 Jeremy Allison 2012-04-06 22:46:15 UTC
I thought this code in get_acl_blob()

  again:

        tmp = TALLOC_REALLOC_ARRAY(ctx, val, uint8_t, size);
        if (tmp == NULL) {
                TALLOC_FREE(val);
                return NT_STATUS_NO_MEMORY;
        }
        val = tmp;

        become_root();
        if (fsp && fsp->fh->fd != -1) {
                sizeret = SMB_VFS_FGETXATTR(fsp, XATTR_NTACL_NAME, val, size);
        } else {
                sizeret = SMB_VFS_GETXATTR(handle->conn, name,
                                        XATTR_NTACL_NAME, val, size);
        }
        if (sizeret == -1) {
                saved_errno = errno;
        }
        unbecome_root();

        /* Max ACL size is 65536 bytes. */
        if (sizeret == -1) {
                errno = saved_errno;
                if ((errno == ERANGE) && (size != 65536)) {
                        /* Too small, try again. */
                        size = 65536;
                        goto again;
                }


was supposed to handle this ? Where does it screw up ?
Comment 2 Richard Sharpe 2012-04-06 23:02:03 UTC
(In reply to comment #1)
> I thought this code in get_acl_blob()
> 
>   again:
> 
>         tmp = TALLOC_REALLOC_ARRAY(ctx, val, uint8_t, size);
>         if (tmp == NULL) {
>                 TALLOC_FREE(val);
>                 return NT_STATUS_NO_MEMORY;
>         }
>         val = tmp;
> 
>         become_root();
>         if (fsp && fsp->fh->fd != -1) {
>                 sizeret = SMB_VFS_FGETXATTR(fsp, XATTR_NTACL_NAME, val, size);
>         } else {
>                 sizeret = SMB_VFS_GETXATTR(handle->conn, name,
>                                         XATTR_NTACL_NAME, val, size);
>         }
>         if (sizeret == -1) {
>                 saved_errno = errno;
>         }
>         unbecome_root();
> 
>         /* Max ACL size is 65536 bytes. */
>         if (sizeret == -1) {
>                 errno = saved_errno;
>                 if ((errno == ERANGE) && (size != 65536)) {
>                         /* Too small, try again. */
>                         size = 65536;
>                         goto again;
>                 }
> 
> 
> was supposed to handle this ? Where does it screw up ?

You are right. I must have been on drugs when I looked at the code.

There is a problem with our VFS, but for some reason I thought there was also a problem in that code. However, there is no problem.

Closing the bug as invalid.