Re: [PATCH wayland v2 2/4] wl_array: Set data to invalid address after free
On Thu, Sep 29, 2016 at 07:17:37AM -0700, Yong Bakos wrote: > Thanks Eric. Would you mind cc'ing the wayland-devel list, so that Patchwork > catches your RB? Sorry, on MLs I usually just hit reply-all without looking at the list of recipients. So, for patchwork: The series is: Reviewed-by: Eric Engestrom(BTW patchwork doesn't understand "the series is", does it? Or does it have to be a specific keyword?) Cheers, Eric ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH wayland v2 2/4] wl_array: Set data to invalid address after free
On Tue, Sep 27, 2016 at 01:03:48PM -0500, Yong Bakos wrote: > From: Yong Bakos> > Explicitly set the data member to an invalid memory address during > wl_array_release, such that re-using a freed wl_array without re-initializing > causes a crash. In addition, this pointer assignment makes wl_array_release > testable. > > Define a constant for the invalid memory address, and add documentation about > this behavior, starting at libwayland version 1.13. I actually did a similar thing in our internal codebase recently (although my focus was catching double-free). I used a small stack var as a sentinel, and set freed vars to its address with an assert first to make sure it wasn't already that. My implementation translated here would be roughly: in src/wayland-private.h: #ifndef NDEBUG extern char wl_array_sentinel; # define WL_ARRAY_POISON_PTR ((void*) _array_sentinel) #else # define WL_ARRAY_POISON_PTR NULL #endif in src/wayland-util.c: #ifndef NDEBUG char wl_array_sentinel; #endif in wl_array_release(), before `free(array->data)`: assert(array->data != WL_ARRAY_POISON_PTR); (same could be added in `wl_array_{add,copy}()`) The benefit of this is that you know the address isn't used by something else, and a char should be cheap enough to not have any impact :) Cheers, Eric ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH wayland v2 2/4] wl_array: Set data to invalid address after free
From: Yong BakosExplicitly set the data member to an invalid memory address during wl_array_release, such that re-using a freed wl_array without re-initializing causes a crash. In addition, this pointer assignment makes wl_array_release testable. Define a constant for the invalid memory address, and add documentation about this behavior, starting at libwayland version 1.13. See https://lists.freedesktop.org/archives/wayland-devel/2016-September/031116.html Signed-off-by: Yong Bakos --- v2: Set data pointer to invalid memory address (pq) src/wayland-private.h | 3 +++ src/wayland-util.c| 1 + src/wayland-util.h| 4 +++- 3 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/wayland-private.h b/src/wayland-private.h index ac712d9..ef58ccf 100644 --- a/src/wayland-private.h +++ b/src/wayland-private.h @@ -36,6 +36,9 @@ #include "wayland-util.h" +/* Invalid memory address */ +#define WL_ARRAY_POISON_PTR (void *) 4 + #define ARRAY_LENGTH(a) (sizeof (a) / sizeof (a)[0]) #define container_of(ptr, type, member) ({ \ diff --git a/src/wayland-util.c b/src/wayland-util.c index 639ccf8..077fec7 100644 --- a/src/wayland-util.c +++ b/src/wayland-util.c @@ -102,6 +102,7 @@ WL_EXPORT void wl_array_release(struct wl_array *array) { free(array->data); + array->data = WL_ARRAY_POISON_PTR; } WL_EXPORT void * diff --git a/src/wayland-util.h b/src/wayland-util.h index 9b7a4b9..42b3027 100644 --- a/src/wayland-util.h +++ b/src/wayland-util.h @@ -381,7 +381,9 @@ wl_array_init(struct wl_array *array); /** * Releases the array data. * - * \note Leaves the array in an invalid state. + * \note Leaves the wl_array in an invalid state. + * Since libwayland version 1.13, using a released wl_array without first + * re-initializing it before use will cause an intentional crash. * * \param array Array whose data is to be released * -- 2.7.2 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel