frederik
/
xserver
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

dix: move props into WindowRec and fix potential NULL deref 

The pointer to the window properties is currently inside the WindowOptional
structure, which may or may not exist at any given time. Thus, before accessing
those fields, at least need to check whether it exists, potentially need to
create it first.

Since a pointer is small (in relation to WindowRec) and windows having properties
is a pretty common, we can make our life much simpler here by moving the pointer
directly into WindowRec, so we don't need extra WindowOptionalRec allocation.

This also fixes an analyzer warning on potential NULL dereference issue:

| ../dix/property.c: In function ‘dixChangeWindowProperty’:
|../dix/property.c:343:37: warning: dereference of NULL ‘*pWin.optional’ [CWE-476] [-Wanalyzer-null-dereference]
|  343 |         pProp->next = pWin->optional->userProps;
|      |                       ~~~~~~~~~~~~~~^~~~~~~~~~~

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
This commit is contained in:
Enrico Weigelt, metux IT consult 2025-05-06 13:26:38 +02:00
parent faae695d0a
commit 6a8ee31e1b
5 changed files with 28 additions and 77 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
Xext/xselinux_ext.c
View File

@ -423,7 +423,7 @@ ProcSELinuxListProperties(ClientPtr client)
/* Count the number of properties and allocate items */ /* Count the number of properties and allocate items */
count = 0; count = 0;
for (pProp = wUserProps(pWin); pProp; pProp = pProp->next) for (pProp = pWin->properties; pProp; pProp = pProp->next)
count++; count++;
items = calloc(count, sizeof(SELinuxListItemRec)); items = calloc(count, sizeof(SELinuxListItemRec));
if (count && !items) if (count && !items)
@ -432,7 +432,7 @@ ProcSELinuxListProperties(ClientPtr client)
/* Fill in the items and calculate size */ /* Fill in the items and calculate size */
i = 0; i = 0;
size = 0; size = 0;
for (pProp = wUserProps(pWin); pProp; pProp = pProp->next) { for (pProp = pWin->properties; pProp; pProp = pProp->next) {
id = pProp->propertyName; id = pProp->propertyName;
rc = SELinuxPopulateItem(items + i, &pProp->devPrivates, id, &size); rc = SELinuxPopulateItem(items + i, &pProp->devPrivates, id, &size);
if (rc != Success) { if (rc != Success) {

32
dix/property.c
View File

@ -77,7 +77,7 @@ PrintPropertys(WindowPtr pWin)
PropertyPtr pProp; PropertyPtr pProp;
int j; int j;
pProp = pWin->userProps; pProp = pWin->properties;
while (pProp) { while (pProp) {
ErrorF("[dix] %x %x\n", pProp->propertyName, pProp->type); ErrorF("[dix] %x %x\n", pProp->propertyName, pProp->type);
ErrorF("[dix] property format: %d\n", pProp->format); ErrorF("[dix] property format: %d\n", pProp->format);
@ -98,7 +98,7 @@ dixLookupProperty(PropertyPtr *result, WindowPtr pWin, Atom propertyName,
client->errorValue = propertyName; client->errorValue = propertyName;
for (pProp = wUserProps(pWin); pProp; pProp = pProp->next) for (pProp = pWin->properties; pProp; pProp = pProp->next)
if (pProp->propertyName == propertyName) if (pProp->propertyName == propertyName)
break; break;
@ -299,8 +299,8 @@ dixChangeWindowProperty(ClientPtr pClient, WindowPtr pWin, Atom property,
pClient->errorValue = property; pClient->errorValue = property;
return rc; return rc;
} }
pProp->next = pWin->optional->userProps; pProp->next = pWin->properties;
pWin->optional->userProps = pProp; pWin->properties = pProp;
} }
else if (rc == Success) { else if (rc == Success) {
/* To append or prepend to a property the request format and type /* To append or prepend to a property the request format and type
@ -384,14 +384,14 @@ DeleteProperty(ClientPtr client, WindowPtr pWin, Atom propName)
return Success; /* Succeed if property does not exist */ return Success; /* Succeed if property does not exist */
if (rc == Success) { if (rc == Success) {
if (pWin->optional->userProps == pProp) { if (pWin->properties == pProp) {
/* Takes care of head */ /* Takes care of head */
if (!(pWin->optional->userProps = pProp->next)) if (!(pWin->properties = pProp->next))
CheckWindowOptionalNeed(pWin); CheckWindowOptionalNeed(pWin);
} }
else { else {
/* Need to traverse to find the previous element */ /* Need to traverse to find the previous element */
prevProp = pWin->optional->userProps; prevProp = pWin->properties;
while (prevProp->next != pProp) while (prevProp->next != pProp)
prevProp = prevProp->next; prevProp = prevProp->next;
prevProp->next = pProp->next; prevProp->next = pProp->next;
@ -407,19 +407,17 @@ DeleteProperty(ClientPtr client, WindowPtr pWin, Atom propName)
void void
DeleteAllWindowProperties(WindowPtr pWin) DeleteAllWindowProperties(WindowPtr pWin)
{ {
PropertyPtr pProp, pNextProp; PropertyPtr pProp = pWin->properties;
pProp = wUserProps(pWin);
while (pProp) { while (pProp) {
deliverPropertyNotifyEvent(pWin, PropertyDelete, pProp); deliverPropertyNotifyEvent(pWin, PropertyDelete, pProp);
pNextProp = pProp->next; PropertyPtr pNextProp = pProp->next;
free(pProp->data); free(pProp->data);
dixFreeObjectWithPrivates(pProp, PRIVATE_PROPERTY); dixFreeObjectWithPrivates(pProp, PRIVATE_PROPERTY);
pProp = pNextProp; pProp = pNextProp;
} }
if (pWin->optional) pWin->properties = NULL;
pWin->optional->userProps = NULL;
} }
static int static int
@ -554,14 +552,14 @@ ProcGetProperty(ClientPtr client)
if (stuff->delete && (reply.bytesAfter == 0)) { if (stuff->delete && (reply.bytesAfter == 0)) {
/* Delete the Property */ /* Delete the Property */
if (pWin->optional->userProps == pProp) { if (pWin->properties == pProp) {
/* Takes care of head */ /* Takes care of head */
if (!(pWin->optional->userProps = pProp->next)) if (!(pWin->properties = pProp->next))
CheckWindowOptionalNeed(pWin); CheckWindowOptionalNeed(pWin);
} }
else { else {
/* Need to traverse to find the previous element */ /* Need to traverse to find the previous element */
prevProp = pWin->optional->userProps; prevProp = pWin->properties;
while (prevProp->next != pProp) while (prevProp->next != pProp)
prevProp = prevProp->next; prevProp = prevProp->next;
prevProp->next = pProp->next; prevProp->next = pProp->next;
@ -589,7 +587,7 @@ ProcListProperties(ClientPtr client)
if (rc != Success) if (rc != Success)
return rc; return rc;
for (pProp = wUserProps(pWin); pProp; pProp = pProp->next) for (pProp = pWin->properties; pProp; pProp = pProp->next)
numProps++; numProps++;
if (numProps) { if (numProps) {
@ -599,7 +597,7 @@ ProcListProperties(ClientPtr client)
numProps = 0; numProps = 0;
temppAtoms = pAtoms; temppAtoms = pAtoms;
for (pProp = wUserProps(pWin); pProp; pProp = pProp->next) { for (pProp = pWin->properties; pProp; pProp = pProp->next) {
realProp = pProp; realProp = pProp;
rc = XaceHookPropertyAccess(client, pWin, &realProp, DixGetAttrAccess); rc = XaceHookPropertyAccess(client, pWin, &realProp, DixGetAttrAccess);
if (rc == Success && realProp == pProp) { if (rc == Success && realProp == pProp) {

6
dix/window.c
View File

@ -205,7 +205,7 @@ get_window_name(WindowPtr pWin)
return overlay_win_name; return overlay_win_name;
#endif #endif
for (prop = wUserProps(pWin); prop; prop = prop->next) { for (prop = pWin->properties; prop; prop = prop->next) {
if (prop->propertyName == XA_WM_NAME && prop->type == XA_STRING && if (prop->propertyName == XA_WM_NAME && prop->type == XA_STRING &&
prop->data) { prop->data) {
len = min(prop->size, WINDOW_NAME_BUF_LEN - 1); len = min(prop->size, WINDOW_NAME_BUF_LEN - 1);
@ -599,7 +599,6 @@ CreateRootWindow(ScreenPtr pScreen)
pWin->optional->otherEventMasks = 0; pWin->optional->otherEventMasks = 0;
pWin->optional->otherClients = NULL; pWin->optional->otherClients = NULL;
pWin->optional->passiveGrabs = NULL; pWin->optional->passiveGrabs = NULL;
pWin->optional->userProps = NULL;
pWin->optional->backingBitPlanes = ~0L; pWin->optional->backingBitPlanes = ~0L;
pWin->optional->backingPixel = 0; pWin->optional->backingPixel = 0;
pWin->optional->boundingShape = NULL; pWin->optional->boundingShape = NULL;
@ -3349,8 +3348,6 @@ CheckWindowOptionalNeed(WindowPtr w)
return; return;
if (optional->passiveGrabs != NULL) if (optional->passiveGrabs != NULL)
return; return;
if (optional->userProps != NULL)
return;
if (optional->backingBitPlanes != (CARD32)~0L) if (optional->backingBitPlanes != (CARD32)~0L)
return; return;
if (optional->backingPixel != 0) if (optional->backingPixel != 0)
@ -3406,7 +3403,6 @@ MakeWindowOptional(WindowPtr pWin)
optional->otherEventMasks = 0; optional->otherEventMasks = 0;
optional->otherClients = NULL; optional->otherClients = NULL;
optional->passiveGrabs = NULL; optional->passiveGrabs = NULL;
optional->userProps = NULL;
optional->backingBitPlanes = ~0L; optional->backingBitPlanes = ~0L;
optional->backingPixel = 0; optional->backingPixel = 0;
optional->boundingShape = NULL; optional->boundingShape = NULL;

57
hw/xwin/winmultiwindowclass.c
View File

@ -46,8 +46,6 @@ DEFINE_ATOM_HELPER(AtmWmWindowRole, "WM_WINDOW_ROLE")
int int
winMultiWindowGetClassHint(WindowPtr pWin, char **res_name, char **res_class) winMultiWindowGetClassHint(WindowPtr pWin, char **res_name, char **res_class)
{ {
struct _Window *pwin;
struct _Property *prop;
int len_name, len_class; int len_name, len_class;
if (!pWin || !res_name || !res_class) { if (!pWin || !res_name || !res_class) {
@ -56,12 +54,7 @@ winMultiWindowGetClassHint(WindowPtr pWin, char **res_name, char **res_class)
return 0; return 0;
} }
pwin = (struct _Window *) pWin; PropertyPtr prop = pWin->properties;
if (pwin->optional)
prop = (struct _Property *) pwin->optional->userProps;
else
prop = NULL;
*res_name = *res_class = NULL; *res_name = *res_class = NULL;
@ -116,20 +109,12 @@ winMultiWindowGetClassHint(WindowPtr pWin, char **res_name, char **res_class)
int int
winMultiWindowGetWMHints(WindowPtr pWin, WinXWMHints * hints) winMultiWindowGetWMHints(WindowPtr pWin, WinXWMHints * hints)
{ {
struct _Window *pwin;
struct _Property *prop;
if (!pWin || !hints) { if (!pWin || !hints) {
ErrorF("winMultiWindowGetWMHints - pWin or hints was NULL\n"); ErrorF("winMultiWindowGetWMHints - pWin or hints was NULL\n");
return 0; return 0;
} }
pwin = (struct _Window *) pWin; PropertyPtr prop = pWin->properties;
if (pwin->optional)
prop = (struct _Property *) pwin->optional->userProps;
else
prop = NULL;
memset(hints, 0, sizeof(WinXWMHints)); memset(hints, 0, sizeof(WinXWMHints));
@ -148,19 +133,12 @@ winMultiWindowGetWMHints(WindowPtr pWin, WinXWMHints * hints)
int int
winMultiWindowGetWindowRole(WindowPtr pWin, char **res_role) winMultiWindowGetWindowRole(WindowPtr pWin, char **res_role)
{ {
struct _Window *pwin;
struct _Property *prop;
int len_role; int len_role;
if (!pWin || !res_role) if (!pWin || !res_role)
return 0; return 0;
pwin = (struct _Window *) pWin; PropertyPtr prop = pWin->properties;
if (pwin->optional)
prop = (struct _Property *) pwin->optional->userProps;
else
prop = NULL;
*res_role = NULL; *res_role = NULL;
while (prop) { while (prop) {
@ -190,20 +168,12 @@ winMultiWindowGetWindowRole(WindowPtr pWin, char **res_role)
int int
winMultiWindowGetWMNormalHints(WindowPtr pWin, WinXSizeHints * hints) winMultiWindowGetWMNormalHints(WindowPtr pWin, WinXSizeHints * hints)
{ {
struct _Window *pwin;
struct _Property *prop;
if (!pWin || !hints) { if (!pWin || !hints) {
ErrorF("winMultiWindowGetWMNormalHints - pWin or hints was NULL\n"); ErrorF("winMultiWindowGetWMNormalHints - pWin or hints was NULL\n");
return 0; return 0;
} }
pwin = (struct _Window *) pWin; PropertyPtr prop = pWin->properties;
if (pwin->optional)
prop = (struct _Property *) pwin->optional->userProps;
else
prop = NULL;
memset(hints, 0, sizeof(WinXSizeHints)); memset(hints, 0, sizeof(WinXSizeHints));
@ -222,20 +192,12 @@ winMultiWindowGetWMNormalHints(WindowPtr pWin, WinXSizeHints * hints)
int int
winMultiWindowGetTransientFor(WindowPtr pWin, Window *pDaddyId) winMultiWindowGetTransientFor(WindowPtr pWin, Window *pDaddyId)
{ {
struct _Window *pwin;
struct _Property *prop;
if (!pWin) { if (!pWin) {
ErrorF("winMultiWindowGetTransientFor - pWin was NULL\n"); ErrorF("winMultiWindowGetTransientFor - pWin was NULL\n");
return 0; return 0;
} }
pwin = (struct _Window *) pWin; PropertyPtr prop = pWin->properties;
if (pwin->optional)
prop = (struct _Property *) pwin->optional->userProps;
else
prop = NULL;
if (pDaddyId) if (pDaddyId)
*pDaddyId = 0; *pDaddyId = 0;
@ -256,8 +218,6 @@ winMultiWindowGetTransientFor(WindowPtr pWin, Window *pDaddyId)
int int
winMultiWindowGetWMName(WindowPtr pWin, char **wmName) winMultiWindowGetWMName(WindowPtr pWin, char **wmName)
{ {
struct _Window *pwin;
struct _Property *prop;
int len_name; int len_name;
if (!pWin || !wmName) { if (!pWin || !wmName) {
@ -266,12 +226,7 @@ winMultiWindowGetWMName(WindowPtr pWin, char **wmName)
return 0; return 0;
} }
pwin = (struct _Window *) pWin; PropertyPtr prop = pWin->properties;
if (pwin->optional)
prop = (struct _Property *) pwin->optional->userProps;
else
prop = NULL;
*wmName = NULL; *wmName = NULL;

6
include/windowstr.h
View File

@ -85,7 +85,6 @@ typedef struct _WindowOpt {
Mask otherEventMasks; /* default: 0 */ Mask otherEventMasks; /* default: 0 */
struct _OtherClients *otherClients; /* default: NULL */ struct _OtherClients *otherClients; /* default: NULL */
struct _GrabRec *passiveGrabs; /* default: NULL */ struct _GrabRec *passiveGrabs; /* default: NULL */
PropertyPtr userProps; /* default: NULL */
CARD32 backingBitPlanes; /* default: ~0L */ CARD32 backingBitPlanes; /* default: ~0L */
CARD32 backingPixel; /* default: 0 */ CARD32 backingPixel; /* default: 0 */
RegionPtr boundingShape; /* default: NULL */ RegionPtr boundingShape; /* default: NULL */
@ -165,6 +164,8 @@ typedef struct _Window {
unsigned damagedDescendants:1; /* some descendants are damaged */ unsigned damagedDescendants:1; /* some descendants are damaged */
unsigned inhibitBGPaint:1; /* paint the background? */ unsigned inhibitBGPaint:1; /* paint the background? */
#endif #endif
PropertyPtr properties; /* default: NULL */
} WindowRec; } WindowRec;
/* /*
@ -189,7 +190,6 @@ extern _X_EXPORT Mask DontPropagateMasks[];
#define wOtherClients(w) wUseDefault(w, otherClients, NULL) #define wOtherClients(w) wUseDefault(w, otherClients, NULL)
#define wOtherInputMasks(w) wUseDefault(w, inputMasks, NULL) #define wOtherInputMasks(w) wUseDefault(w, inputMasks, NULL)
#define wPassiveGrabs(w) wUseDefault(w, passiveGrabs, NULL) #define wPassiveGrabs(w) wUseDefault(w, passiveGrabs, NULL)
#define wUserProps(w) wUseDefault(w, userProps, NULL)
#define wBackingBitPlanes(w) wUseDefault(w, backingBitPlanes, ~0L) #define wBackingBitPlanes(w) wUseDefault(w, backingBitPlanes, ~0L)
#define wBackingPixel(w) wUseDefault(w, backingPixel, 0) #define wBackingPixel(w) wUseDefault(w, backingPixel, 0)
#define wBoundingShape(w) wUseDefault(w, boundingShape, NULL) #define wBoundingShape(w) wUseDefault(w, boundingShape, NULL)
@ -198,6 +198,8 @@ extern _X_EXPORT Mask DontPropagateMasks[];
#define wClient(w) (clients[CLIENT_ID((w)->drawable.id)]) #define wClient(w) (clients[CLIENT_ID((w)->drawable.id)])
#define wBorderWidth(w) ((int) (w)->borderWidth) #define wBorderWidth(w) ((int) (w)->borderWidth)
static inline PropertyPtr wUserProps(WindowPtr pWin) { return pWin->properties; }
/* true when w needs a border drawn. */ /* true when w needs a border drawn. */
#define HasBorder(w) ((w)->borderWidth || wClipShape(w)) #define HasBorder(w) ((w)->borderWidth || wClipShape(w))