[PATCH] SMB2 AAPL create context (was: Mac OS Mavericks über slow)

Jeremy Allison jra at samba.org
Thu Sep 11 15:20:29 MDT 2014


On Thu, Sep 11, 2014 at 03:22:44PM +0200, Ralph Böhme wrote:
> 
> * repurpose several fields in SMB2/FIND reply for retrieval of the
>   following attributes: UNIX mode, resource fork length, FinderInfo
>   and max_acess
> 
> * use NFS customs SIDs in ACEs [1] for getting and setting UNIX mode
> 
> Please review and push if ok.
>     Couldn't find any other reference from MS that this one.

Don't expose get_file_handle_for_metadata() as public.
It's horrid and should stay hidden :-).

See comment below.

> diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h
> index 692f582..c4fed64 100644
> --- a/source3/smbd/proto.h
> +++ b/source3/smbd/proto.h
> @@ -284,6 +284,11 @@ struct timespec get_change_timespec(connection_struct *conn,
>  				struct files_struct *fsp,
>  				const struct smb_filename *smb_fname);
>  
> +NTSTATUS get_file_handle_for_metadata(connection_struct *conn,
> +				      struct smb_filename *smb_fname,
> +				      files_struct **ret_fsp,
> +				      bool *need_close);
> +

> + * Check and possibly read AFPINFO_STREAM stream from a file or dir
> + *
> + * If the file has an associated AFPINFO_STREAM stream, FinderInfo is
> + * returned in packed format in the buffer pointed to by
> + * compressed_finder_info. The buffer must suitably sized (16 bytes).
> + **/
> +static void get_compressed_finder_info(connection_struct *conn,
> +				       const struct smb_filename *smb_fname,
> +				       char *compressed_finder_info)
> +{
> +	struct smb_filename *infoname = NULL;
> +	files_struct *fsp;
> +	NTSTATUS status;
> +	bool need_close;
> +	ssize_t len;
> +	uint8_t ai[AFP_INFO_SIZE];
> +	uint32_t date_added;
> +	int ret;
> +
> +	memset(compressed_finder_info, '\0', 16);
> +
> +	infoname = synthetic_smb_fname(talloc_tos(),
> +				       smb_fname->base_name,
> +				       AFPINFO_STREAM,
> +				       NULL);
> +
> +	DEBUG(10, ("reading AFPINFO_STREAM for %s\n",
> +		   smb_fname_str_dbg(infoname)));
> +
> +	ret = SMB_VFS_STAT(conn, infoname);
> +	if (ret != 0) {
> +		goto error;
> +	}

get_file_handle_for_metadata is the wrong interface
to use here (despite its name).

It really needs to say private and conceal its
horrors inside dosmode.c :-).

What you need is an internal open for read handle.

Use this instead:

	firstly, set fsp = NULL above when defined:

        status = SMB_VFS_CREATE_FILE(
                conn,                                   /* conn */
                NULL,                                   /* req */
                0,                                      /* root_dir_fid */
                infoname,                               /* fname */
                FILE_READ_DATA,                         /* access_mask */
                (FILE_SHARE_READ | FILE_SHARE_WRITE |   /* share_access */
                        FILE_SHARE_DELETE),
                FILE_OPEN,                              /* create_disposition*/
                0,                                      /* create_options */
                0,                                      /* file_attributes */
                INTERNAL_OPEN_ONLY,                     /* oplock_request */
                NULL,                                   /* lease */
                0,                                      /* allocation_size */
                0,                                      /* private_flags */
                NULL,                                   /* sd */
                NULL,                                   /* ea_list */
                &fsp,                                   /* result */
                NULL);                                  /* pinfo */

        if (NT_STATUS_IS_OK(status)) {
                goto error;
        }

> +	status = get_file_handle_for_metadata(conn, infoname,
> +					      &fsp, &need_close);
> +	if (!NT_STATUS_IS_OK(status)) {
> +		DEBUG(10, ("get_file_handle_for_metadata %s: %s\n",
> +			   smb_fname_str_dbg(infoname),
> +			   nt_errstr(status)));
> +		goto error;
> +	}
> +
> +	len = SMB_VFS_PREAD(fsp, ai, AFP_INFO_SIZE, 0);
> +	if (len != AFP_INFO_SIZE) {
> +		DEBUG(1, ("bad length: %ju\n", (intmax_t)len));
> +		goto error;
> +	}
> +
> +	/* AD_DATE_DELTA = 946684800 */
> +	date_added = convert_time_t_to_uint32_t(
> +		smb_fname->st.st_ex_btime.tv_sec)
> +		- 946684800;
> +
> +	if (S_ISREG(smb_fname->st.st_ex_mode)) {
> +		/* finder_type */
> +		memcpy(compressed_finder_info, ai + 16, 4);
> +		/* finder_creator */
> +		memcpy(compressed_finder_info + 4, ai + 20, 4);
> +	}
> +	/* finder_flags */
> +	memcpy(compressed_finder_info + 8, ai + 24, 2);
> +	/* finder_ext_flags */
> +	memcpy(compressed_finder_info + 10, ai + 40, 2);
> +	/* finder_date_added, could also use stat_ex.btime instead */
> +	RSIVAL(compressed_finder_info, 12, date_added);
> +
> +error:

Change this:

> +	if (need_close) {
> +		SMB_VFS_CLOSE(fsp);
> +	}

to:

        if (fsp != NULL) {
               SMB_VFS_CLOSE(fsp);
        }

> +
> +	if (do_chmod) {
> +		/* Apply mode and return without calling VFS_NEXT */
> +		DEBUG(10, ("fruit_fset_nt_acl: %s, %04o\n",
> +			   fsp_str_dbg(fsp), mode));
> +		rc = SMB_VFS_CHMOD(handle->conn, fsp->fsp_name->base_name,
> +				   mode);

You have a file handle here - always use FCHMOD in preference to CHMOD.

Cheers,

	Jeremy.


More information about the samba-technical mailing list