[PATCH wayland 3/5] server: don't proceed in posting no-memory error on client destruction

[Bryce Harrington]

On Mon, May 16, 2016 at 12:22:02PM +0300, Pekka Paalanen wrote:
> On Fri, 13 May 2016 15:01:20 +0200
> Marek Chalupa <mchqwerty at gmail.com> wrote:

Hi Marek,

Thanks for providing the test cases to go along with this, much
appreciated.

The first patch in this set looked fine to me, and with pq's r-b I've
landed it for the release.  The remaining patches sound like they need a
bit more work so am deferring them for post-1.11.

Bryce
 
> > When a client is being destroyed, the display_resource is set to NULL.
> > If then some destroy handler calls wl_client_post_no_memory() or
> > wl_resource_post_no_memory() we crash.
> > 
> > Signed-off-by: Marek Chalupa <mchqwerty at gmail.com>
> > ---
> >  src/wayland-server.c | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> > 
> > diff --git a/src/wayland-server.c b/src/wayland-server.c
> > index c93a426..b26a48d 100644
> > --- a/src/wayland-server.c
> > +++ b/src/wayland-server.c
> > @@ -546,6 +546,11 @@ wl_client_get_object(struct wl_client *client, uint32_t id)
> >  WL_EXPORT void
> >  wl_client_post_no_memory(struct wl_client *client)
> >  {
> > +	/* don't send any other errors
> > +	 * if we are destroying the client */
> > +	if (!client->display_resource)
> > +		return;
> > +
> >  	wl_resource_post_error(client->display_resource,
> >  			       WL_DISPLAY_ERROR_NO_MEMORY, "no memory");
> >  }
> > @@ -553,6 +558,11 @@ wl_client_post_no_memory(struct wl_client *client)
> >  WL_EXPORT void
> >  wl_resource_post_no_memory(struct wl_resource *resource)
> >  {
> > +	/* don't send any other errors
> > +	 * if we are destroying the client */
> > +	if (!resource->client->display_resource)
> > +		return;
> > +
> >  	wl_resource_post_error(resource->client->display_resource,
> >  			       WL_DISPLAY_ERROR_NO_MEMORY, "no memory");
> >  }
> 
> Hi Marek,
> 
> wl_resource_post_error() already checks and escapes if display_resource
> is NULL, but first it uses the passed in resource to get 'client',
> which is where it would crash if there wasn't a display_resource.
> 
> Ok, so the way to trigger this is to send a no-memory from an object
> destroy handler. A little strange, but should work still indeed.
> 
> The alternative to this patch would be to make
> wl_resource_post_error(NULL, ...) not crash, but ABI-wise that is a
> change rather than just a fix.
> 
> Therefore:
> Reviewed-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
> 
> 
> Thanks,
> pq

> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v2
> 
> iQIVAwUBVzmROiNf5bQRqqqnAQgK/A//Y0XBxWLVdLfxr0mC2koE41ggKbjIfU4A
> l1Hcy5Z0wZFzOQWJr+Wzv7CPrMYFldJRAFYeABl1nSZAuP94ns8ji1Y4gWLQ5goD
> iSo43V6ZN6kKpt07c7lgJwbLJSZ6vDisKAmRsS/6Z4AH+fYy4GGmrsk/aoX1Vh18
> 5SFbDv1RZIoRP3tWhP89Uwa4EzRD+8xgs8YTO9un/78rhE7lknviOYX9R/WsJfFb
> DfzO8hB3sUa912eyrLqHpIPG6PngcUO2UQTiVFaLe76e6DTEM97Yt9IVOMOzkS6Y
> 5+YXyClrjEdc36lTMtlYCxHzjcL+aGhAfUttdixF9j8srpjB/bfON8ufUJStlRwQ
> NMBiPVJYwXM4+XotfxvwalbPG6Et+ZILtbGJKuXtdgigxRtm8zdx5+WhnSnATtT4
> Gf3bxzG+rRvnqi23MQHZkLrsobacY/tH59o0WrrxT4LKLKonD3aIx1Ajo7ohbYWO
> JVSROwMV3JFKCiBigFDaNUSXmCbYejFldOjcPbCJzdRwYuvWnr590NLr7zCSPqS8
> Hm2zfQA+lJ5PRElY1G8TbG5Aqw0rTjM/Zwf/rU1S+yBueryyyxhn/kfedfKR8edu
> 2voLaaXchvtk0BpzmrrWE77ShU1pCVYfX3AwGG51hwKlVHRQ154O4KL8CodOQzYQ
> eA+NsN8HH/k=
> =EW5S
> -----END PGP SIGNATURE-----


> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel