On Tue, Sep 10, 2013 at 12:50:25PM +0200, Daniel Vetter wrote: > On Tue, Sep 10, 2013 at 12:26 PM, Ville Syrjälä > <ville.syrjala@xxxxxxxxxxxxxxx> wrote: > >> static void intel_sdvo_get_mode_from_dtd(struct drm_display_mode * mode, > >> const struct intel_sdvo_dtd *dtd) > >> { > >> + memset(mode, 0, sizeof(*mode)); > > > > I have a theoretical worry that someone might end up calling this on a > > mode that sits on some list or was actually allocated and has a proper > > object id which we'd leak here. > > > > To make it totally safe you could populate a pristine mode struct and > > use drm_mode_copy() to overwrite adjusted_mode. Assuming we're not so > > short on stack space that our oversized mode struct would cause issues. > > Other options would be to add some WARNs to catch wrongdoers, or embed > > a temp mode for this purpose inside the intel_sdvo struct. > > We can't really check for this since list_empty on stack garbage won't > work too well, either. And e.g. ->get_config has the pipe config on > the stack. So I think we just need to do review here. I also think the > risk is pretty low, this is all used in internal structures around > pipe_config, where the mode is never linked. Well, another idea would be to add drm_mode_clear() what would do the memset() but preserve the id and list head. -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx