[PATCH] Small enhancement to vfs_fruit

[Jeremy Allison]

On Fri, Nov 30, 2018 at 01:28:26PM +0100, Ralph Böhme wrote:
> Hi!
> 
> Attached is a small improvement for vfs_fruit that helps users that appyl
> additional patches on-top of Samba. The commit messages has the nasty
> details. :)
> 
> Please review & push if happy. Thanks!

OK, I *think* I understand this (but can you post
the gdb backtrace just to be sure ? :-).

RB+ and pushed.

> -- 
> Ralph Boehme, Samba Team                https://samba.org/
> Samba Developer, SerNet GmbH   https://sernet.de/en/samba/
> GPG-Fingerprint   FAE2C6088A24252051C559E4AA1E9B7126399E46

> From 37b714865657c791416f992a6c0e6d9926601630 Mon Sep 17 00:00:00 2001
> From: Ralph Boehme <slow at samba.org>
> Date: Fri, 30 Nov 2018 10:27:19 +0100
> Subject: [PATCH] vfs_fruit: avoid dereferencing fsp->base_fsp in
>  fruit_fstat_meta_stream()
> 
> This helps avoiding a NULL dereference on systems where additional
> patches modify the following condition in open_file()
> 
>   if ((open_access_mask & (FILE_READ_DATA|FILE_WRITE_DATA|FILE_APPEND_DATA|FILE_EXECUTE)) ||
>       (!file_existed && (local_flags & O_CREAT)) ||
>       ((local_flags & O_TRUNC) == O_TRUNC) ) {
> 
> to
> 
>   if ((open_access_mask & (FILE_READ_DATA|FILE_WRITE_DATA|FILE_APPEND_DATA|FILE_EXECUTE|DELETE_ACCESS)) ||
>       (!file_existed && (local_flags & O_CREAT)) ||
>       ((local_flags & O_TRUNC) == O_TRUNC) ) {
> 
> Ie addtionally check open_access_mask against DELETE_ACCESS. As a result
> opens with DELETE_ACCESS go through the code that does an fd_open() plus
> a subsequent fstat().
> 
> That will trigger a crash in fruit_fstat_meta_stream() when a client
> wants to delete a file for deletion. When we open base file for delete,
> we call open_streams_for_delete() which internally calls create-file
> with NTCREATEX_OPTIONS_PRIVATE_STREAM_DELETE which prevents opening of
> the base_fsp. Voila, combined with the change described above you get a
> NULL deref.
> 
> Signed-off-by: Ralph Boehme <slow at samba.org>
> ---
>  source3/modules/vfs_fruit.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
> index 50b6fac8b95..19101efba74 100644
> --- a/source3/modules/vfs_fruit.c
> +++ b/source3/modules/vfs_fruit.c
> @@ -5204,6 +5204,7 @@ static int fruit_fstat_meta_stream(vfs_handle_struct *handle,
>  				   SMB_STRUCT_STAT *sbuf)
>  {
>  	struct fio *fio = (struct fio *)VFS_FETCH_FSP_EXTENSION(handle, fsp);
> +	struct smb_filename smb_fname;
>  	ino_t ino;
>  	int ret;
>  
> @@ -5223,11 +5224,15 @@ static int fruit_fstat_meta_stream(vfs_handle_struct *handle,
>  		return 0;
>  	}
>  
> -	ret = fruit_stat_base(handle, fsp->base_fsp->fsp_name, false);
> +	smb_fname = (struct smb_filename) {
> +		.base_name = fsp->fsp_name->base_name,
> +	};
> +
> +	ret = fruit_stat_base(handle, &smb_fname, false);
>  	if (ret != 0) {
>  		return -1;
>  	}
> -	*sbuf = fsp->base_fsp->fsp_name->st;
> +	*sbuf = smb_fname.st;
>  
>  	ino = fruit_inode(sbuf, fsp->fsp_name->stream_name);
>  
> -- 
> 2.17.2
>