[PATCH xserver 15/15] dri3: correctly handle failed get_supported_modifiers requests
Emil Velikov
emil.l.velikov at gmail.com
Mon Apr 2 15:41:26 UTC 2018
From: Emil Velikov <emil.velikov at collabora.com>
Currently depending on the code path hit, the helper will set some of
the output values and not others.
It could also leak memory ;-)
At the same time the caller was:
- working around the broken behaviour - by initialising the variables
- outright ignoring if the helper fails
Fix all that by consistently setting the output variables and returning
a protocol error if we fail to get the data required.
Bonus points: current code does not attribute if we cannot get the
modifiers info for certain format. A smaller format array is available,
yet the original length is stored.
Fixes: cef12efc15c ("glamor: Implement GetSupportedModifiers")
Cc: Louis-Francis Ratté-Boulianne <lfrb at collabora.com>
Cc: Daniel Stone <daniels at collabora.com>
Signed-off-by: Emil Velikov <emil.velikov at collabora.com>
---
We could still return "success" to the user - I've opted for this
solution since we already bail on dixLookupWindow failure.
---
dri3/dri3_request.c | 18 ++++++++++--------
dri3/dri3_screen.c | 38 +++++++++++++++++++++-----------------
2 files changed, 31 insertions(+), 25 deletions(-)
diff --git a/dri3/dri3_request.c b/dri3/dri3_request.c
index edcd0e782..9b7d81ea3 100644
--- a/dri3/dri3_request.c
+++ b/dri3/dri3_request.c
@@ -335,10 +335,10 @@ proc_dri3_get_supported_modifiers(ClientPtr client)
};
WindowPtr window;
ScreenPtr pScreen;
- CARD64 *window_modifiers = NULL;
- CARD64 *screen_modifiers = NULL;
- CARD32 nwindowmodifiers = 0;
- CARD32 nscreenmodifiers = 0;
+ CARD64 *window_modifiers;
+ CARD64 *screen_modifiers;
+ CARD32 nwindowmodifiers;
+ CARD32 nscreenmodifiers;
int status;
int i;
@@ -349,10 +349,12 @@ proc_dri3_get_supported_modifiers(ClientPtr client)
return status;
pScreen = window->drawable.pScreen;
- dri3_get_supported_modifiers(pScreen, &window->drawable,
- stuff->depth, stuff->bpp,
- &nwindowmodifiers, &window_modifiers,
- &nscreenmodifiers, &screen_modifiers);
+ status = dri3_get_supported_modifiers(pScreen, &window->drawable,
+ stuff->depth, stuff->bpp,
+ &nwindowmodifiers, &window_modifiers,
+ &nscreenmodifiers, &screen_modifiers);
+ if (status != Success)
+ return status;
rep.numWindowModifiers = nwindowmodifiers;
rep.numScreenModifiers = nscreenmodifiers;
diff --git a/dri3/dri3_screen.c b/dri3/dri3_screen.c
index 2b0e139fa..de97d55bc 100644
--- a/dri3/dri3_screen.c
+++ b/dri3/dri3_screen.c
@@ -178,7 +178,9 @@ dri3_get_supported_modifiers(ScreenPtr screen, DrawablePtr drawable,
int ret;
CARD32 num_drawable_mods;
CARD64 *drawable_mods;
+ CARD32 num_window_mods = 0;
CARD64 *window_mods = NULL;
+ CARD32 num_screen_mods = 0;
CARD64 *screen_mods = NULL;
CARD32 format;
dri3_dmabuf_format_ptr screen_format = NULL;
@@ -202,11 +204,8 @@ dri3_get_supported_modifiers(ScreenPtr screen, DrawablePtr drawable,
if (screen_format == NULL)
return BadMatch;
- if (screen_format->num_modifiers == 0) {
- *num_screen_modifiers = 0;
- *num_window_modifiers = 0;
- return Success;
- }
+ if (screen_format->num_modifiers == 0)
+ goto done;
if (!info->get_drawable_modifiers ||
!info->get_drawable_modifiers(drawable, format,
@@ -221,17 +220,13 @@ dri3_get_supported_modifiers(ScreenPtr screen, DrawablePtr drawable,
*/
screen_mods = malloc(screen_format->num_modifiers * sizeof(CARD64));
if (!screen_mods)
- return BadAlloc;
+ goto bad_alloc;
if (num_drawable_mods > 0) {
window_mods = malloc(screen_format->num_modifiers * sizeof(CARD64));
- if (!window_mods) {
- free(screen_mods);
- return BadAlloc;
- }
+ if (!window_mods)
+ goto bad_alloc;
}
- *num_screen_modifiers = 0;
- *num_window_modifiers = 0;
for (i = 0; i < screen_format->num_modifiers; i++) {
CARD64 modifier = screen_format->modifiers[i];
Bool window_mod = FALSE;
@@ -245,18 +240,27 @@ dri3_get_supported_modifiers(ScreenPtr screen, DrawablePtr drawable,
if (window_mod) {
window_mods[*num_window_modifiers] = modifier;
- *num_window_modifiers += 1;
+ num_window_mods++;
} else {
screen_mods[*num_screen_modifiers] = modifier;
- *num_screen_modifiers += 1;
+ num_screen_mods++;
}
}
- assert(*num_window_modifiers + *num_screen_modifiers == screen_format->num_modifiers);
+ assert(num_window_mods + num_screen_mods == screen_format->num_modifiers);
- *window_modifiers = window_mods;
- *screen_modifiers = screen_mods;
+done:
free(drawable_mods);
+ *num_window_modifiers = num_window_mods;
+ *window_modifiers = window_mods;
+ *num_screen_modifiers = num_screen_mods;
+ *screen_modifiers = screen_mods;
return Success;
+
+bad_alloc:
+ free(drawable_mods);
+ free(screen_mods);
+ free(window_mods);
+ return BadAlloc;
}
--
2.16.0
More information about the xorg-devel
mailing list