Quantcast

[PATCH] Fix smbd crash when printing lease record.

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH] Fix smbd crash when printing lease record.

Samba - samba-technical mailing list
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.

Cheers,

        Jeremy.

0001-s3-smbd-Fix-open_files.idl-to-correctly-ignore-share.patch (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH] Fix smbd crash when printing lease record.

Samba - samba-technical mailing list
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 <[hidden email]>
> 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 <[hidden email]>
> ---
>  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

Loading...