dri: Prevent abuse of the Resource database

The Resource database is only designed to store a single value for a
particular type associated with an XID. Due to the asynchronous nature
of the vblank/flip requests, we would often associate multiple frame
events with a particular drawable/client. Upon freeing the resource, we
would not necessarily decouple the right value, leaving a stale pointer
behind. Later when the client disappeared, we would write through that
stale pointer upsetting valgrind and causing memory corruption. MDK.

Instead, we need to implement an extra layer for tracking multiple
frames within a single Resource.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=37700
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
This commit is contained in:
Chris Wilson 2011-07-11 16:28:15 +01:00
parent ab1000821a
commit 2608a367ac
3 changed files with 176 additions and 42 deletions

View File

@ -476,11 +476,12 @@ typedef void (*DRI2SwapEventPtr)(ClientPtr client, void *data, int type,
typedef struct _DRI2FrameEvent {
XID drawable_id;
XID client_id; /* fake client ID to track client destruction */
ClientPtr client;
enum DRI2FrameEventType type;
int frame;
struct list drawable_resource, client_resource;
/* for swaps & flips only */
DRI2SwapEventPtr event_complete;
void *event_data;

View File

@ -71,6 +71,8 @@ typedef struct {
PixmapPtr pixmap;
} I830DRI2BufferPrivateRec, *I830DRI2BufferPrivatePtr;
static DevPrivateKeyRec i830_client_key;
static uint32_t pixmap_flink(PixmapPtr pixmap)
{
struct intel_pixmap *priv = intel_get_pixmap_private(pixmap);
@ -608,22 +610,73 @@ I830DRI2DrawablePipe(DrawablePtr pDraw)
static RESTYPE frame_event_client_type, frame_event_drawable_type;
struct i830_dri2_resource {
XID id;
RESTYPE type;
struct list list;
};
static struct i830_dri2_resource *
get_resource(XID id, RESTYPE type)
{
struct i830_dri2_resource *resource;
void *ptr;
ptr = NULL;
dixLookupResourceByType(&ptr, id, type, NULL, DixWriteAccess);
if (ptr)
return ptr;
resource = malloc(sizeof(*resource));
if (resource == NULL)
return NULL;
if (!AddResource(id, type, resource)) {
free(resource);
return NULL;
}
resource->id = id;
resource->type = type;
list_init(&resource->list);
return resource;
}
static int
i830_dri2_frame_event_client_gone(void *data, XID id)
{
DRI2FrameEventPtr frame_event = data;
struct i830_dri2_resource *resource = data;
while (!list_is_empty(&resource->list)) {
DRI2FrameEventPtr info =
list_first_entry(&resource->list,
DRI2FrameEventRec,
client_resource);
list_del(&info->client_resource);
info->client = NULL;
}
free(resource);
frame_event->client = NULL;
frame_event->client_id = None;
return Success;
}
static int
i830_dri2_frame_event_drawable_gone(void *data, XID id)
{
DRI2FrameEventPtr frame_event = data;
struct i830_dri2_resource *resource = data;
while (!list_is_empty(&resource->list)) {
DRI2FrameEventPtr info =
list_first_entry(&resource->list,
DRI2FrameEventRec,
drawable_resource);
list_del(&info->drawable_resource);
info->drawable_id = None;
}
free(resource);
frame_event->drawable_id = None;
return Success;
}
@ -641,34 +694,48 @@ i830_dri2_register_frame_event_resource_types(void)
return TRUE;
}
static XID
get_client_id(ClientPtr client)
{
XID *ptr = dixGetPrivateAddr(&client->devPrivates, &i830_client_key);
if (*ptr == 0)
*ptr = FakeClientID(client->index);
return *ptr;
}
/*
* Hook this frame event into the server resource
* database so we can clean it up if the drawable or
* client exits while the swap is pending
*/
static Bool
i830_dri2_add_frame_event(DRI2FrameEventPtr frame_event)
i830_dri2_add_frame_event(DRI2FrameEventPtr info)
{
frame_event->client_id = FakeClientID(frame_event->client->index);
struct i830_dri2_resource *resource;
if (!AddResource(frame_event->client_id, frame_event_client_type, frame_event))
resource = get_resource(get_client_id(info->client),
frame_event_client_type);
if (resource == NULL)
return FALSE;
if (!AddResource(frame_event->drawable_id, frame_event_drawable_type, frame_event)) {
FreeResourceByType(frame_event->client_id, frame_event_client_type, TRUE);
list_add(&info->client_resource, &resource->list);
resource = get_resource(info->drawable_id, frame_event_drawable_type);
if (resource == NULL) {
list_del(&info->client_resource);
return FALSE;
}
list_add(&info->drawable_resource, &resource->list);
return TRUE;
}
static void
i830_dri2_del_frame_event(DRI2FrameEventPtr frame_event)
i830_dri2_del_frame_event(DRI2FrameEventPtr info)
{
if (frame_event->client_id)
FreeResourceByType(frame_event->client_id, frame_event_client_type, TRUE);
if (frame_event->drawable_id)
FreeResourceByType(frame_event->drawable_id, frame_event_drawable_type, TRUE);
list_del(&info->client_resource);
list_del(&info->drawable_resource);
}
static void
@ -1350,6 +1417,10 @@ Bool I830DRI2ScreenInit(ScreenPtr screen)
return FALSE;
}
if (!dixRegisterPrivateKey(&i830_client_key, PRIVATE_CLIENT, sizeof(XID)))
return FALSE;
#if DRI2INFOREC_VERSION >= 4
if (serverGeneration != dri2_server_generation) {
dri2_server_generation = serverGeneration;

View File

@ -93,16 +93,24 @@ struct sna_dri_private {
struct kgem_bo *bo;
};
struct sna_dri_resource {
XID id;
RESTYPE type;
struct list list;
};
struct sna_dri_frame_event {
struct sna *sna;
XID drawable_id;
XID client_id; /* fake client ID to track client destruction */
ClientPtr client;
enum frame_event_type type;
int frame;
int pipe;
int count;
struct list drawable_resource;
struct list client_resource;
/* for swaps & flips only */
DRI2SwapEventPtr event_complete;
void *event_data;
@ -120,6 +128,8 @@ struct sna_dri_frame_event {
struct sna_dri_frame_event *chain;
};
static DevPrivateKeyRec sna_client_key;
static inline struct sna_dri_frame_event *
to_frame_event(void *data)
{
@ -531,22 +541,67 @@ sna_dri_get_pipe(DrawablePtr pDraw)
static RESTYPE frame_event_client_type, frame_event_drawable_type;
static struct sna_dri_resource *
get_resource(XID id, RESTYPE type)
{
struct sna_dri_resource *resource;
void *ptr;
ptr = NULL;
dixLookupResourceByType(&ptr, id, type, NULL, DixWriteAccess);
if (ptr)
return ptr;
resource = malloc(sizeof(*resource));
if (resource == NULL)
return NULL;
if (!AddResource(id, type, resource)) {
free(resource);
return NULL;
}
resource->id = id;
resource->type = type;
list_init(&resource->list);
return resource;
}
static int
sna_dri_frame_event_client_gone(void *data, XID id)
{
struct sna_dri_frame_event *frame_event = data;
struct sna_dri_resource *resource = data;
while (!list_is_empty(&resource->list)) {
struct sna_dri_frame_event *info =
list_first_entry(&resource->list,
struct sna_dri_frame_event,
client_resource);
list_del(&info->client_resource);
info->client = NULL;
}
free(resource);
frame_event->client = NULL;
frame_event->client_id = None;
return Success;
}
static int
sna_dri_frame_event_drawable_gone(void *data, XID id)
{
struct sna_dri_frame_event *frame_event = data;
struct sna_dri_resource *resource = data;
while (!list_is_empty(&resource->list)) {
struct sna_dri_frame_event *info =
list_first_entry(&resource->list,
struct sna_dri_frame_event,
drawable_resource);
list_del(&info->drawable_resource);
info->drawable_id = None;
}
free(resource);
frame_event->drawable_id = None;
return Success;
}
@ -568,45 +623,48 @@ sna_dri_register_frame_event_resource_types(void)
return TRUE;
}
static XID
get_client_id(ClientPtr client)
{
XID *ptr = dixGetPrivateAddr(&client->devPrivates, &sna_client_key);
if (*ptr == 0)
*ptr = FakeClientID(client->index);
return *ptr;
}
/*
* Hook this frame event into the server resource
* database so we can clean it up if the drawable or
* client exits while the swap is pending
*/
static Bool
sna_dri_add_frame_event(struct sna_dri_frame_event *frame_event)
sna_dri_add_frame_event(struct sna_dri_frame_event *info)
{
frame_event->client_id = FakeClientID(frame_event->client->index);
struct sna_dri_resource *resource;
if (!AddResource(frame_event->client_id,
frame_event_client_type,
frame_event))
resource = get_resource(get_client_id(info->client),
frame_event_client_type);
if (resource == NULL)
return FALSE;
if (!AddResource(frame_event->drawable_id,
frame_event_drawable_type,
frame_event)) {
FreeResourceByType(frame_event->client_id,
frame_event_client_type,
TRUE);
list_add(&info->client_resource, &resource->list);
resource = get_resource(info->drawable_id, frame_event_drawable_type);
if (resource == NULL) {
list_del(&info->client_resource);
return FALSE;
}
list_add(&info->drawable_resource, &resource->list);
return TRUE;
}
static void
sna_dri_frame_event_info_free(struct sna_dri_frame_event *info)
{
if (info->client_id)
FreeResourceByType(info->client_id,
frame_event_client_type,
TRUE);
if (info->drawable_id)
FreeResourceByType(info->drawable_id,
frame_event_drawable_type,
TRUE);
list_del(&info->client_resource);
list_del(&info->drawable_resource);
if (info->front)
_sna_dri_destroy_buffer(info->sna, info->front);
@ -1606,6 +1664,10 @@ Bool sna_dri_open(struct sna *sna, ScreenPtr screen)
return FALSE;
}
}
if (!dixRegisterPrivateKey(&sna_client_key, PRIVATE_CLIENT, sizeof(XID)))
return FALSE;
sna->deviceName = drmGetDeviceNameFromFd(sna->kgem.fd);
memset(&info, '\0', sizeof(info));
info.fd = sna->kgem.fd;