Fix bugs discovered by static analyzers and fix buffer damage calculation

* compositor.h (XLAssert): Make a macro.
* dnd.c (HandleCirculateNotify, HandleReparentNotify): Fix NULL
checks.
(ReadProtocolProperties): Return suitable values for windows
that aren't in the cache.
* egl.c (HaveEglExtension1): Avoid redundant assignment to n.
* fns.c (XLAssert): Delete function.
* picture_renderer.c (GetRenderDevice): Remove redundant TODO.
(BufferFromShm): Assert that pict_format is non-NULL.
(ValidateShmParams): Likewise.
* pointer_constraints.c (ApplyLines): Remove redundant
assignment to i.
* renderer.c (PickRenderer): Fix build with non-GCC compilers.
* seat.c (ComputeHotspot): Return values when surface is NULL.
(XLSeatExplicitlyGrabSurface): Don't save keyboard grab state.
* shm.c (CreatePool): Close fd and return if pool could not be
allocated.
* subcompositor.c (GetContentScale): Move earlier.
(ViewDamageBuffer, ViewGetContentScale): New functions.
(SubcompositorUpdate): Remove redundant assignment.
* surface.c (ApplyViewport): Make dest_width and dest_height
double.
(ApplyDamage): Call ViewDamageBuffer.
(ScaleToWindow):
* text_input.c (HandleNewIM):
* xdg_toplevel.c (SendDecorationConfigure1, HandleWmStateChange)
(HandleAllowedActionsChange, HandleDecorationResourceDestroy):
Avoid NULL pointer dereferences in various cases.
This commit is contained in:
oldosfan 2022-10-17 04:44:49 +00:00
parent c1959e5f22
commit 167001689a
13 changed files with 147 additions and 123 deletions

View file

@ -17,6 +17,8 @@ for more details.
You should have received a copy of the GNU General Public License
along with 12to11. If not, see <https://www.gnu.org/licenses/>. */
#include <stdlib.h>
#include <sys/param.h>
#include <sys/stat.h>
@ -627,7 +629,6 @@ extern void *XLLookUpAssoc (XLAssocTable *, XID);
extern void XLDeleteAssoc (XLAssocTable *, XID);
extern void XLDestroyAssocTable (XLAssocTable *);
extern void XLAssert (Bool);
extern int XLOpenShm (void);
extern void XLScaleRegion (pixman_region32_t *, pixman_region32_t *,
@ -787,8 +788,10 @@ extern void ViewTranslate (View *, int, int, int *, int *);
extern void ViewFree (View *);
extern void ViewDamage (View *, pixman_region32_t *);
extern void ViewDamageBuffer (View *, pixman_region32_t *);
extern void ViewSetOpaque (View *, pixman_region32_t *);
extern void ViewSetInput (View *, pixman_region32_t *);
extern double ViewGetContentScale (View *);
extern int ViewWidth (View *);
extern int ViewHeight (View *);
extern void ViewSetScale (View *, int);
@ -1661,3 +1664,5 @@ extern void XLRelativePointerSendRelativeMotion (struct wl_resource *,
#define IntSubtractWrapv(a, b, r) __builtin_sub_overflow (a, b, r)
#define IntMultiplyWrapv(a, b, r) __builtin_mul_overflow (a, b, r)
/* This is a macro in order to be more static analyzer friendly. */
#define XLAssert(cond) (!(cond) ? abort () : ((void) 0))

16
dnd.c
View file

@ -1632,10 +1632,9 @@ HandleCirculateNotify (WindowCache *cache, XEvent *event)
window = XLLookUpAssoc (cache->entries, event->xcirculate.window);
if (window)
if (!window)
return;
XLAssert (window->parent == event->xcirculate.event);
/* If the window has been recirculated to the top, relink it
@ -1870,7 +1869,7 @@ HandleReparentNotify (WindowCache *cache, XEvent *event)
window = XLLookUpAssoc (cache->entries, event->xreparent.window);
if (window)
if (!window)
return;
/* First, unlink window. */
@ -2230,8 +2229,15 @@ ReadProtocolProperties (Window window, int *version_return,
entry = XLLookUpAssoc (drag_state.window_cache->entries, window);
if (!entry)
/* The entry is not in the window cache... */
return;
{
/* Return some suitable values for a window that isn't in the
window cache. */
*version_return = 0;
*proxy_return = None;
/* The entry is not in the window cache... */
return;
}
if (entry->flags & IsPropertyRead)
{

2
egl.c
View file

@ -466,8 +466,6 @@ HaveEglExtension1 (const char *extensions, const char *extension)
while (extensions < end)
{
n = 0;
/* Skip spaces, if any. */
if (*extensions == ' ')
{

7
fns.c
View file

@ -337,13 +337,6 @@ XLDestroyAssocTable (XLAssocTable *table)
XLFree (table);
}
void
XLAssert (Bool condition)
{
if (!condition)
abort ();
}
void
XLScaleRegion (pixman_region32_t *dst, pixman_region32_t *src,
float scale_x, float scale_y)

View file

@ -1609,8 +1609,6 @@ GetRenderDevice (Bool *error)
int *fds, fd;
struct stat dev_stat;
/* TODO: if this ever calls exec, set FD_CLOEXEC, and implement
multiple providers. */
cookie = xcb_dri3_open (compositor.conn,
DefaultRootWindow (compositor.display),
None);
@ -2022,10 +2020,17 @@ BufferFromShm (SharedMemoryAttributes *attributes, Bool *error)
Picture picture;
int fd, depth, format, bpp;
PictureBuffer *buffer;
XRenderPictFormat *pict_format;
depth = DepthForFormat (attributes->format, &bpp);
format = attributes->format;
if (!depth)
{
*error = True;
return (RenderBuffer) NULL;
}
/* Duplicate the fd, since XCB closes file descriptors after sending
them. */
fd = fcntl (attributes->fd, F_DUPFD_CLOEXEC, 0);
@ -2036,6 +2041,10 @@ BufferFromShm (SharedMemoryAttributes *attributes, Bool *error)
return (RenderBuffer) NULL;
}
/* Obtain the picture format. */
pict_format = PictFormatForFormat (format);
XLAssert (pict_format != NULL);
/* Now, allocate the XIDs for the shm segment and pixmap. */
seg = xcb_generate_id (compositor.conn);
pixmap = xcb_generate_id (compositor.conn);
@ -2050,8 +2059,7 @@ BufferFromShm (SharedMemoryAttributes *attributes, Bool *error)
/* Create the picture for the pixmap. */
picture = XRenderCreatePicture (compositor.display, pixmap,
PictFormatForFormat (format),
0, &picture_attrs);
pict_format, 0, &picture_attrs);
/* Create the wrapper object. */
buffer = XLCalloc (1, sizeof *buffer);
@ -2072,7 +2080,7 @@ BufferFromShm (SharedMemoryAttributes *attributes, Bool *error)
buffer->activity.buffer_last = &buffer->activity;
/* If the format is presentable, mark the buffer as presentable. */
if (PictFormatIsPresentable (PictFormatForFormat (format)))
if (PictFormatIsPresentable (pict_format))
buffer->flags |= CanPresent;
/* Return the picture. */
@ -2109,7 +2117,7 @@ ValidateShmParams (uint32_t format, uint32_t width, uint32_t height,
/* Obtain the depth and bpp. */
depth = DepthForFormat (format, &bpp);
XLAssert (depth != -1);
XLAssert (depth != 0);
/* If any signed values are negative, return. */
if (offset < 0 || stride < 0)

View file

@ -1323,8 +1323,6 @@ ApplyLines (Window window, PointerConfinement *confinement,
FreeSingleBarrier);
confinement->applied_barriers = NULL;
i = 0;
/* Set the pointer device. */
device_id = XLSeatGetPointerDevice (confinement->seat);

View file

@ -466,9 +466,10 @@ PickRenderer (void)
/* Fall back to the default renderer. */
fprintf (stderr, "Defaulting to renderer %s, as %s was not found\n",
renderers->name, selected);
fall_back:
}
fall_back:
if (!InstallRenderer (renderers))
abort ();
}

17
seat.c
View file

@ -1038,7 +1038,12 @@ ComputeHotspot (SeatCursor *cursor, int min_x, int min_y,
int hotspot_x, hotspot_y;
if (!cursor->role.surface)
return;
{
/* Can this really happen? */
*x = min_x + cursor->hotspot_x;
*y = min_y + cursor->hotspot_y;
return;
}
/* Scale the hotspot coordinates up by the scale. */
hotspot_x = cursor->hotspot_x * cursor->role.surface->factor;
@ -5028,12 +5033,12 @@ XLSeatExplicitlyGrabSurface (Seat *seat, Surface *surface, uint32_t serial)
CancelGrabEarly (seat);
/* Now, grab the keyboard. Note that we just grab the keyboard so
that keyboard focus cannot be changed; key events are still
reported based on raw events. */
that keyboard focus cannot be changed, which is not very crucial,
so it is allowed to fail. */
state = XIGrabDevice (compositor.display, seat->master_keyboard,
window, time, None, XIGrabModeAsync,
XIGrabModeAsync, True, &mask);
XIGrabDevice (compositor.display, seat->master_keyboard,
window, time, None, XIGrabModeAsync,
XIGrabModeAsync, True, &mask);
/* And record the grab surface, so that owner_events can be
implemented correctly. */

6
shm.c
View file

@ -430,7 +430,11 @@ CreatePool (struct wl_client *client, struct wl_resource *resource,
pool = XLSafeMalloc (sizeof *pool);
if (!pool)
wl_resource_post_no_memory (resource);
{
wl_resource_post_no_memory (resource);
close (fd);
return;
}
memset (pool, 0, sizeof *pool);

View file

@ -1604,6 +1604,78 @@ ViewDamage (View *view, pixman_region32_t *damage)
damage);
}
static double
GetContentScale (int scale)
{
if (scale > 0)
return 1.0 / (scale + 1);
return -scale + 1;
}
void
ViewDamageBuffer (View *view, pixman_region32_t *damage)
{
pixman_region32_t temp;
double x_factor, y_factor;
double crop_width, stretch_width;
double crop_height, stretch_height;
if (!view->buffer)
return;
if (!view->scale && !IsViewported (view))
/* There is no scale nor viewport. Just damage the view
directly. */
ViewDamage (view, damage);
else
{
/* Otherwise, apply the transform to the view. */
pixman_region32_init (&temp);
/* First, apply the content scale. */
x_factor = GetContentScale (view->scale);
y_factor = GetContentScale (view->scale);
/* Scale the region. */
XLScaleRegion (&temp, damage, x_factor, y_factor);
/* Next, apply the viewport. */
if (IsViewported (view))
{
crop_width = view->crop_width;
crop_height = view->crop_height;
stretch_width = view->dest_width;
stretch_height = view->dest_height;
/* Offset the region. */
if (view->src_x != 1.0 || view->src_y != 1.0)
pixman_region32_translate (&temp, -view->src_x,
-view->src_y);
/* If the crop width or height were not specified, use the
current buffer width/height. */
if (crop_width == -1)
{
crop_width = (XLBufferWidth (view->buffer)
* GetContentScale (view->scale));
crop_height = (XLBufferHeight (view->buffer)
* GetContentScale (view->scale));
}
x_factor = stretch_width / crop_width;
y_factor = stretch_height / crop_height;
/* Scale the region again. */
XLScaleRegion (&temp, &temp, x_factor, y_factor);
}
/* Damage the view. */
pixman_region32_union (&view->damage, &view->damage, &temp);
pixman_region32_fini (&temp);
}
}
void
ViewSetOpaque (View *view, pixman_region32_t *opaque)
{
@ -1631,13 +1703,10 @@ ViewGetSubcompositor (View *view)
return view->subcompositor;
}
static double
GetContentScale (int scale)
double
ViewGetContentScale (View *view)
{
if (scale > 0)
return 1.0 / (scale + 1);
return -scale + 1;
return GetContentScale (view->scale);
}
int
@ -1698,7 +1767,6 @@ ViewSetScale (View *view, int scale)
ViewAfterSizeUpdate (view);
}
void
ViewSetViewport (View *view, double src_x, double src_y,
double crop_width, double crop_height,
@ -2053,7 +2121,6 @@ SubcompositorUpdate (Subcompositor *subcompositor)
presented = False;
start = subcompositor->inferiors->next->view;
original_start = subcompositor->inferiors->next->view;
age = RenderTargetAge (subcompositor->target);
/* If there is not enough prior damage available to satisfy age, set
@ -2269,7 +2336,7 @@ SubcompositorUpdate (Subcompositor *subcompositor)
/* If the damage extends outside the area known to be
obscured by the current start, reset start back to
the original starting point. */
if (start != original_start)
if (start != original_start && original_start)
{
pixman_region32_subtract (&temp, &list->view->damage,
&start_opaque);

View file

@ -644,7 +644,7 @@ static void
ApplyViewport (Surface *surface)
{
State *state;
int dest_width, dest_height;
double dest_width, dest_height;
double crop_width, crop_height, src_x, src_y;
double max_width, max_height;
@ -840,79 +840,9 @@ HandleScaleChanged (void *data, int new_scale)
static void
ApplyDamage (Surface *surface)
{
pixman_region32_t temp;
int scale;
float x_factor, y_factor;
double src_width, src_height;
double dest_width, dest_height;
scale = GetEffectiveScale (surface->current_state.buffer_scale);
/* If no surface was attached, just return. */
if (!surface->current_state.buffer)
return;
/* N.B. that this must come after the scale is applied. */
if (scale || (surface->current_state.src_x != -1
&& surface->current_state.src_x != 0.0
&& surface->current_state.src_y != 0.0)
|| surface->current_state.dest_width != -1)
{
pixman_region32_init (&temp);
if (!scale)
x_factor = y_factor = 1.0;
if (scale > 0)
x_factor = y_factor = 1.0 / (scale + 1);
else
x_factor = y_factor = abs (scale) + 1;
/* If a viewport dest size is set, add that to the scale as
well. */
if (surface->current_state.dest_width != -1)
{
if (surface->current_state.src_width != -1)
{
src_width = surface->current_state.src_width;
src_height = surface->current_state.src_height;
}
else
{
src_width = XLBufferWidth (surface->current_state.buffer);
src_height = XLBufferHeight (surface->current_state.buffer);
/* Now scale these buffer dimensions down by the buffer
scale, so they can be turned into surface
coordinates. */
src_width /= surface->current_state.buffer_scale;
src_height /= surface->current_state.buffer_scale;
}
dest_width = surface->current_state.dest_width;
dest_height = surface->current_state.dest_height;
x_factor *= (float) (src_width / dest_width);
y_factor *= (float) (src_height / dest_height);
}
if (x_factor != 1.0f || y_factor != 1.0f)
XLScaleRegion (&temp, &surface->current_state.damage,
x_factor, y_factor);
/* If a viewport is set, translate the damage region by the
src_x and src_y. This is lossy. */
if (surface->current_state.src_x != -1.0)
pixman_region32_translate (&temp,
floor (surface->current_state.src_x),
floor (surface->current_state.src_y));
ViewDamage (surface->view, &temp);
pixman_region32_fini (&temp);
}
else
ViewDamage (surface->view, &surface->current_state.damage);
/* N.B. that this must come after the scale and viewport is
applied. */
ViewDamageBuffer (surface->view, &surface->current_state.damage);
}
static void
@ -1809,7 +1739,7 @@ SurfaceToWindow (Surface *surface, double x, double y,
void
ScaleToWindow (Surface *surface, double width, double height,
double *width_out, double *height_out)
double *width_out, double *height_out)
{
*width_out = width * surface->factor;
*height_out = height * surface->factor;

View file

@ -2781,7 +2781,11 @@ HandleNewIM (XIM xim)
while (input != &info->inputs)
{
/* Try to create the IC for this one input. */
if (input->current_state.enabled)
if (input->current_state.enabled
/* If this is NULL, then the IC will only be created
upon the next commit after the focus is actually
transferred to the text input. */
&& input->client_info->focus_surface)
{
CreateIC (input);

View file

@ -375,6 +375,8 @@ AddState (XdgToplevel *toplevel, uint32_t state)
static void
SendDecorationConfigure1 (XdgToplevel *toplevel)
{
XLAssert (toplevel->decoration != NULL);
#define ServerSide ZXDG_TOPLEVEL_DECORATION_V1_MODE_SERVER_SIDE
#define ClientSide ZXDG_TOPLEVEL_DECORATION_V1_MODE_CLIENT_SIDE
@ -676,13 +678,11 @@ HandleWmStateChange (XdgToplevel *toplevel)
&actual_format, &actual_size,
&bytes_remaining, &tmp_data);
if (rc != Success
if (rc != Success || !tmp_data
|| actual_type != XA_ATOM || actual_format != 32
|| bytes_remaining)
goto empty_states;
XLAssert (tmp_data != NULL);
states = (Atom *) tmp_data;
/* First, reset relevant states. */
@ -790,13 +790,11 @@ HandleAllowedActionsChange (XdgToplevel *toplevel)
&actual_format, &actual_size,
&bytes_remaining, &tmp_data);
if (rc != Success
if (rc != Success || !tmp_data
|| actual_type != XA_ATOM || actual_format != 32
|| bytes_remaining)
goto empty_states;
XLAssert (tmp_data != NULL);
states = (Atom *) tmp_data;
/* First, reset the actions that we will change. */
@ -2322,7 +2320,14 @@ HandleDecorationResourceDestroy (struct wl_resource *resource)
/* Detach the decoration from the toplevel if the latter still
exists. */
if (decoration->toplevel)
decoration->toplevel->decoration = NULL;
{
decoration->toplevel->decoration = NULL;
/* Clear StateNeedDecorationConfigure, as not doing so may
result in decoration->toplevel (NULL) being dereferenced by
SendDecorationConfigure1. */
decoration->toplevel->state &= ~StateNeedDecorationConfigure;
}
/* Free the decoration. */
XLFree (decoration);