[PATCH] vfs module for VxFS

[Abhidnya Joshi]

Hi Jeremy,

Thanks! Please find attached path including the changes. 

In vxfs_sort_acl(), I still have kept 

                switch(smb_ace->a_type) { .....
                default:
                        id = -1;
                }

As is. Here id will be filled as -1 for ACL_MASK and ACL_OTHER. 
Also if type is invalid, it will be handled before id assignment so this should be fine.

Thanks and Regards
Abhidnya Joshi
-----Original Message-----
From: Jeremy Allison [mailto:jra at samba.org] 
Sent: Saturday, August 30, 2014 12:12 AM
To: Abhidnya Joshi
Cc: Jeremy Allison; samba-technical at samba.org
Subject: Re: [PATCH] vfs module for VxFS

On Thu, Aug 28, 2014 at 01:00:14AM -0700, Abhidnya Joshi wrote:
> Hi Jeremy,
> 
> Thanks for the quick review.
> Please find attached patch with changes.

Ok - full review follows:

Inside :

static char * vxfs_sort_acl(SMB_ACL_T theacl, TALLOC_CTX *mem_ctx,
                            uint32_t o_uid,
                            uint32_t o_gid) {

you have:

                switch(smb_ace->a_type) { .....
                default:
                        type = -1;
                }

                switch(smb_ace->a_type) { .....
                default:
                        id = -1;
                }

The 'default' cases IMHO are error conditions, and you should treat as such (i.e. return false here).

Inside:

static char * vxfs_compact_buf(char *e_buf, int *new_count, int count,
                               TALLOC_CTX *mem_ctx)

You have:

        c_buf = talloc_zero_size(mem_ctx, count * 8);

followed by:

        /*Copy first two enries from e_buf to c_buf
         *These are USER_OBJ and GROUP_OBJ
         */

        memcpy(c_buf, e_buf, 16);

There are no checks that count >= 2, meaning the memcpy could overflow c_buf is count == 1.

Inside:

static bool vxfs_compare_acls(char *e_buf, char *n_buf, int n_count,
                              int e_count) {

You have:

        n_type = SVAL(n_buf, offset + (8 * (n_count-1)));
        e_type = SVAL(e_buf, offset + (8 * (e_count-1)));

You need checks that n_count and e_count are big enough.

Inside:

static bool vxfs_compare(connection_struct *conn, char *name, SMB_ACL_T the_acl,
                         SMB_ACL_TYPE_T the_acl_type)

You directly call:

        if (stat(name, &st) == -1) {

For a stackable VFS module you can't call stat() directly, this must be SMB_VFS_STAT() instead.

Looking really good though ! Give me another go around fixing these issues please and I'll get on it asap.

Cheers,

	Jeremy.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-s3-vfs-module-Adding-new-vfs-module-for-Symantec-VxF.patch
Type: application/octet-stream
Size: 18051 bytes
Desc: 0001-s3-vfs-module-Adding-new-vfs-module-for-Symantec-VxF.patch
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20140901/c94b6020/attachment.obj>