[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