[PATCH v2 11/13] drm/modes: parse_cmdline: Explicitly memset the passed in drm_cmdline_mode struct

Hans de Goede hdegoede at redhat.com
Mon Nov 18 14:26:15 UTC 2019


Hi,

On 18-11-2019 13:28, Maxime Ripard wrote:
> On Wed, Nov 13, 2019 at 05:44:32PM +0100, Hans de Goede wrote:
>> Instead of only setting mode->specified on false on an early exit and
>> leaving e.g. mode->bpp_specified and mode->refresh_specified as is,
>> lets be consistent and just zero out the entire passed in struct at
>> the top of drm_mode_parse_command_line_for_connector()
>>
>> Signed-off-by: Hans de Goede <hdegoede at redhat.com>
>> ---
>>   drivers/gpu/drm/drm_modes.c | 5 ++---
>>   1 file changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
>> index beb764efe6b3..1fee4a71eff7 100644
>> --- a/drivers/gpu/drm/drm_modes.c
>> +++ b/drivers/gpu/drm/drm_modes.c
>> @@ -1745,12 +1745,11 @@ bool drm_mode_parse_command_line_for_connector(const char *mode_option,
>>   	char *bpp_end_ptr = NULL, *refresh_end_ptr = NULL;
>>   	int i, len, ret;
>>
>> +	memset(mode, 0, sizeof(*mode));
>>   	mode->panel_orientation = DRM_MODE_PANEL_ORIENTATION_UNKNOWN;
> 
> The reported error by kbuild rings a bell. I think I tried to do this,
> saw that error, and then forgot about it.
> 
> Looking more at the code now, I don't see any in bochs that looks
> really wrong. Either way, we should either fix bochs, or add a
> unit-test to have a test for the bochs case so that we don't have that
> issue sneaking around.

Ok, for those reading along, for some reason Red Hat's mails server
has eaten the kbuild report. in case this has happened to more people
we are talking about this report:

https://lists.freedesktop.org/archives/dri-devel/2019-November/244541.html

Maxime, looking at this closer, the bochs driver indeed does not do
anything wrong, my patch does. Instead of that I deleted the:

		mode->specified = false;

line, I deleted the:

		return false;

line instead, so the null-ptr deref is on the mode_option. No idea
why this is not crashing things everywhere, as I've run a kernel with
these patches, also without a video= option on several devices...

Anyways let me send out a v3 of the set with this patcch fixed and
thank you for the reviews.

Regards,

Hans



More information about the dri-devel mailing list