[PATCH] Fix smbd crash when printing lease record.

Alexander Bokovoy ab at samba.org
Wed May 17 18:25:34 UTC 2017


On ke, 17 touko 2017, Jeremy Allison wrote:
> This is the fix for the crash reproduced by doing:
> 
> make -j test TESTS="samba3.smbtorture_s3.crypt_client" SMBD_OPTIONS=-d11 WINBINDD_OPTIONS=-d11
> 
> BUG: https://bugzilla.samba.org/show_bug.cgi?id=12793
> 
> Inside struct share_mode_entry there is a pointer to
> a struct share_mode_lease. This pointer is fixed
> up by hand to point to an entry in the leases[]
> array in the containing struct share_mode_data,
> with an offset of share_mode_entry->lease_idx.
> 
> If the share_mode_entry isn't pointing to a
> valid lease, then share_mode_entry->lease_idx is
> set to 0xFFFFFFFF and share_mode_entry->lease
> is an invalid pointer, but the ndr printing
> code still tries to print it out in a debug
> when the debug level is set high enough.
> 
> Patch passes full make test. Please review
> and push if happy.

Thanks, Jeremy. This crash now makes sense, after I went through the
code in locking.c where lease_idx is set to UINT32_MAX (0xFFFFFFFF).

RB+, please push with other patches.

> From 11b2520bfb28d77cf6356cc67127d498f9c5372b Mon Sep 17 00:00:00 2001
> From: Jeremy Allison <jra at samba.org>
> Date: Tue, 16 May 2017 16:12:19 -0700
> Subject: [PATCH] s3: smbd: Fix open_files.idl to correctly ignore
>  share_mode_lease *lease in share_mode_entry.
> 
> This is currently marked 'skip', which means it isn't stored in the
> db, but printed out in ndr dump. However, this pointer can be invalid
> if the lease_idx is set to 0xFFFFFFFF (invalid).
> 
> This is fixed up inside parse_share_modes(), but not until after
> ndr_pull_share_mode_data() is called. If lease_idx == 0xFFFFFFFF
> then ndr_print_share_mode_lease() prints an invalid value and
> crashes.
> 
> BUG: https://bugzilla.samba.org/show_bug.cgi?id=12793
> 
> Signed-off-by: Jeremy Allison <jra at samba.org>
> ---
>  source3/librpc/idl/open_files.idl | 2 +-
>  source3/locking/share_mode_lock.c | 4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/source3/librpc/idl/open_files.idl b/source3/librpc/idl/open_files.idl
> index 6f74340497b..1f85f245fca 100644
> --- a/source3/librpc/idl/open_files.idl
> +++ b/source3/librpc/idl/open_files.idl
> @@ -62,7 +62,7 @@ interface open_files
>  		 * to store this share_mode_entry on disk.
>  		 */
>  		[skip] boolean8	stale;
> -		[skip] share_mode_lease *lease;
> +		[ignore] share_mode_lease *lease;
>  	} share_mode_entry;
>  
>  	typedef [public] struct {
> diff --git a/source3/locking/share_mode_lock.c b/source3/locking/share_mode_lock.c
> index 0333b0d7965..cee00458079 100644
> --- a/source3/locking/share_mode_lock.c
> +++ b/source3/locking/share_mode_lock.c
> @@ -324,8 +324,8 @@ static struct share_mode_data *parse_share_modes(TALLOC_CTX *mem_ctx,
>  	}
>  
>  	/*
> -	 * Initialize the values that are [skip] in the idl. The NDR code does
> -	 * not initialize them.
> +	 * Initialize the values that are [skip] or [ignore]
> +	 * in the idl. The NDR code does not initialize them.
>  	 */
>  
>  	for (i=0; i<d->num_share_modes; i++) {
> -- 
> 2.13.0.303.g4ebf302169-goog
> 


-- 
/ Alexander Bokovoy



More information about the samba-technical mailing list