[PATCH] Fix bug 9777 - vfs_dirsort uses non-stackable calls, dirfd(), malloc instead of talloc and doesn't cope with directories being modified whilst reading.

[Jeremy Allison]

On Thu, Apr 11, 2013 at 09:50:55AM +0200, Andreas Schneider wrote:
> On Tuesday 09 April 2013 17:00:21 Jeremy Allison wrote:
> > (Re-submitted with correct [PATCH] in the Subject: header,
> > sorry :-).
> > 
> > This patchset fixes a number of issues in the vfs_dirsort
> > module. Tested under valgrind. It's structured as a set
> > of micro-patches so hopefully will be easy to follow and
> > digest. It also adds vfs_dirsort into the test suite so
> > it should not be broken in future.
> > 
> > Please review and push to master if you're happy, then
> > I'll get it into 4.0.next.
> 
> [PATCH 08/10] Remove the use of dirfd inside the vfs_dirsort.c.
> 
> +	struct timespec *pmtime;
> +
> +	if (data->fsp) {
> +		ret = fsp_stat(data->fsp);
> +		pmtime = &data->fsp->fsp_name->st.st_ex_mtime;
> +	} else {
> +		ret = SMB_VFS_STAT(handle->conn, data->smb_fname);
> +		pmtime = &data->smb_fname->st.st_ex_mtime;
> +	}
>  
>  	if (ret == -1) {
>  		return false;
>  	}
>  
> -	ret_mtime->tv_sec = dir_stat.st_mtime;
> -	ret_mtime->tv_nsec = 0;
> +	*ret_mtime = *pmtime;
> 
> If you don't want to use ret_mtime directly, then pmtime shouldn't be a 
> pointer. In the end you just copy the struct, right?

The reason I use a pointer here is that I don't
want to modify *ret_mtime unless I know the
stat returned 0.

So if I have :

+     struct timespec mtime;
+
+     if (data->fsp) {
+             ret = fsp_stat(data->fsp);
+             mtime = &data->fsp->fsp_name->st.st_ex_mtime;
+     } else {
+             ret = SMB_VFS_STAT(handle->conn, data->smb_fname);
+             mtime = &data->smb_fname->st.st_ex_mtime;
+     }
..
+     *ret_mtime = mtime;

That is 2 structure copies rather than one. But I
suppose as an st.st_ex_mtime is 8 bytes, and on a
64-bit box a pointer assignment is also 8 bytes
then it doesn't really make a difference :-).

> The rest looks fine to me.

Thanks ! I'll add your reviewed-by and push.

Jeremy.