I noticed recently (the hard way) that the data pointed to by the "block"
parameter of xf86InterpretEDID() is simply assigned to xf86MonRc::rawData
verbatim, referencing the block of memory passed in directly.  It is never
freed through the xf86MonRec as far as I can tell, although the xf86MonRec may
itself be freed by the server if xf86OutputSetEDID() is called from
xf86ProbeOutputModes().

This creates a problem:
- If the caller free()s the EDID blob after passing it to xf86OutputSetEDID (as
  the intel driver seems to do), then we have a dangling pointer and EDID
  parsing done later (e.g., due to an xf86OutputGetEDIDModes) will reference
  freed data.
- If the caller doesn't free the data (as xf86DoEEDID() used by, e.g., the nv
  driver seems to do), then it is leaked (of course).  There isn't a very good
  place to free the data since the xf86MonRec may itself be freed by the
  server.

I've worked around this for my use by stashing the pointer for later freeing in
a driver-private structure, but I think it would make sense for the server to
do a malloc() && memcpy() in xf86InterpretEDID so that the other drivers can be
fixed.  With that modification (plus the code to later free that blob), the
intel driver's behavior would "just work", and xf86DoEEDID() could be updated
to free() its blob or simply allocate it on the stack.  Thoughts?

Thanks,
Robert
_______________________________________________
[email protected]: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Reply via email to