On 09/16/2015 07:34 PM, Keith Packard wrote: > Aaron Plattner <aplatt...@nvidia.com> writes: > >> The modesetting driver corrupts memory when used after a server regeneration >> because not enough memory is allocated for its pixmap privates. This happens >> because its call to dixRegisterScreenSpecificPrivateKey() does nothing >> because >> key->initialized is still TRUE from the first server generation. However, >> the >> key is not in the screen's linked list of screen-specific privates because >> that's freed and reallocated during the server generation loop in dix_main(). >> >> Fix this by clearing the screen-specific keys during CloseScreen, the same >> way >> other privates are cleared during dixResetPrivates(). > > Yeah, this makes sense. I think that we expected the screen-specific > privates to be allocated as a part of the screen initialization or > privates (and hence why dixFreeScreenSpecificPrivates is called before > CloseScreen when it might get freed if it were separately allocated). > > I have a slight fear that CloseScreen is going to reference these > privates, and so cleaning them before CloseScreen might turn out > badly. A lot of code gets executed in CloseScreen, after all.
Yeah, that's fair. I think it should be safe to move these calls to after CloseScreen, but I'll have to think through it harder to convince myself. > There's no interface for screen-specific privates to be allocated by the > privates.c code, so we can safely assume that they do not need to be > freed themselves. Unless the caller messed with the key internals, key->allocated should be set to FALSE by dixRegisterScreenSpecificPrivateKey(). > How about we just set key->initialized = FALSE and leave the rest of the > key alone? That does assume that no screens will get hot-added during > CloseScreen, which seems entirely reasonable to me. The whole thing will > then be neatly re-set when the server comes around again. We could certainly just set key->initialized to FALSE instead, yeah. But then why does dixResetPrivates() bother setting anything else? I agree that adding new screens during the CloseScreen loop is asking for trouble. I hope that can't happen today... > Alternatively, we could assert that dixRegisterScreenSpecificPrivateKey > is only called once per key and remove the check for key->initialized, > which is really only valid for keys used across multiple objects. I think? I don't think that works because we need these keys to be in pScreen->screenSpecificPrivates[*].key, and that's lost when the screen is recycled. -- Aaron _______________________________________________ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel