Using OpenBSD's new memory allocator which is a lot less tolerant regarding accesses to unallocated memory than others, people have started reporting failure to start the X server whithout a xorg.conf configuration file. Mark Kettenis provided a detailled analysis of the problem. Modules add references to symbols they need to a global list using the LoaderRefSymList() function. This function calls AppendSymbol(), which stores a pointer to the string in the module's address space into the list. When a module is unloaded (which happens when autoconfiguration is used), the strings in the module's address space are no longer available, but the refList list still references them. Any further iteration on this list will cause access to unallocated memory. Since the loader does not provide a mean to clean up list entries when unloading a module, I think that the best fix for this problem is to copy the strings into the list, as done in the attached patch.
Created attachment 2959 [details] [review] Proposed patch
Comment on attachment 2959 [details] [review] Proposed patch Oops I did not mean to edit this patch
The fact that symbols cannot get nuked when the module is unloaded is a design flaw. I can imagine that if a symbol has been marked required by a module that has then be nuked will generate bogus errors because it may be unresolved in another driver which however doesn't need it. The reason is that if one driver is unloaded all its submodules are nuked if their refcount is 0. The right fix would involve to add an argument to the LoaderRefSymbols() and LoaderReqSymbols() function and their SymList equivalents so that the module adding it could be identified. We at least should avoid copying the same string multiple times. This will add bw compatibility issues - but maybe that's what needs to be done.
of course, dlloader makes LoaderRefSymList redundant anyway. the only issue i have with this patch is that it leaks, the xnfalloc'd strings won't get free'd on server regeneration.
Comment on attachment 2959 [details] [review] Proposed patch nominating for 7. leaks, but not a lot, and prevents a crash.
Has this patch been merged into trunk? (Keywording as patch)
I just commited it to HEAD. Keeping the bug open if ajax wants to put it in 7.1.
Can be closed now.
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.