[Intel-gfx] [PATCH 2/3] drm/i915/sdvo: Robustify the dtd<->drm_mode conversions

[Daniel Vetter]

On Tue, Sep 10, 2013 at 1:00 PM, Ville Syrjälä
<ville.syrjala at linux.intel.com> wrote:
> 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 at linux.intel.com> 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.

At least for the adjusted mode embedded into the pipe config that
won't work either since we want to memset the entire thing to not miss
any fields ...
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch