| Message ID | 20190827115850.25731-3-mripard@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/4] drm/modes: Add a switch to differentiate free standing options | expand |
hi maxime, On Tue, Aug 27, 2019 at 1:59 PM Maxime Ripard <mripard@kernel.org> wrote: > > From: Maxime Ripard <maxime.ripard@bootlin.com> > > The named modes support has introduced a number of glitches that were in > part due to the fact that the parser will take any string as a named mode. > > Since we shouldn't have a lot of options there (and they should be pretty > standard), let's introduce a whitelist of the available named modes so that > the kernel can differentiate between a poorly formed command line and a > named mode. > > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com> Tested-by: thomas graichen <thomas.graichen@gmail.com> BEFORE (only "[PATCH v5 05/12] drm/modes: Rewrite the command line parser " applied): my eachlink h6 mini tv box (which requires the video=HDMI-A-1:e cmdline option to give any output on hdmi) did not show any hdmi output any more (in 5.2 it still worked) AFTER (the above patch plus this patch set here applied): my eachlink h6 mini tv box gives output on hdmi again. i also did check it the other way around: if i remove the video=HDMI-A-1:e option i no longer get any hdmi output as expected. as a result this patch series fixes the problem/regression for me. best wishes - thomas > --- > drivers/gpu/drm/drm_modes.c | 20 ++++++++++++++++++++ > 1 file changed, 20 insertions(+) > > diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c > index ea7e6c8c8318..988797d8080a 100644 > --- a/drivers/gpu/drm/drm_modes.c > +++ b/drivers/gpu/drm/drm_modes.c > @@ -1677,6 +1677,22 @@ static int drm_mode_parse_cmdline_options(char *str, size_t len, > return 0; > } > > +const char *drm_named_modes_whitelist[] = { > + "NTSC", > + "PAL", > +}; > + > +static bool drm_named_mode_is_in_whitelist(const char *mode, unsigned int size) > +{ > + int i; > + > + for (i = 0; i < ARRAY_SIZE(drm_named_modes_whitelist); i++) > + if (!strncmp(mode, drm_named_modes_whitelist[i], size)) > + return true; > + > + return false; > +} > + > /** > * drm_mode_parse_command_line_for_connector - parse command line modeline for connector > * @mode_option: optional per connector mode option > @@ -1794,6 +1810,10 @@ bool drm_mode_parse_command_line_for_connector(const char *mode_option, > if (named_mode) { > if (mode_end + 1 > DRM_DISPLAY_MODE_LEN) > return false; > + > + if (!drm_named_mode_is_in_whitelist(name, mode_end)) > + return false; > + > strscpy(mode->name, name, mode_end + 1); > } else { > ret = drm_mode_parse_cmdline_res_mode(name, mode_end, > -- > 2.21.0 >
Hi! Dne torek, 27. avgust 2019 ob 13:58:49 CEST je Maxime Ripard napisal(a): > From: Maxime Ripard <maxime.ripard@bootlin.com> > > The named modes support has introduced a number of glitches that were in > part due to the fact that the parser will take any string as a named mode. > > Since we shouldn't have a lot of options there (and they should be pretty > standard), let's introduce a whitelist of the available named modes so that > the kernel can differentiate between a poorly formed command line and a > named mode. > > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com> > --- > drivers/gpu/drm/drm_modes.c | 20 ++++++++++++++++++++ > 1 file changed, 20 insertions(+) > > diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c > index ea7e6c8c8318..988797d8080a 100644 > --- a/drivers/gpu/drm/drm_modes.c > +++ b/drivers/gpu/drm/drm_modes.c > @@ -1677,6 +1677,22 @@ static int drm_mode_parse_cmdline_options(char *str, > size_t len, return 0; > } > > +const char *drm_named_modes_whitelist[] = { > + "NTSC", > + "PAL", > +}; That array should be static. With that fixed: Reviewed-by: Jernej Skrabec <jernej.skrabec@siol.net> Best regards, Jernej > + > +static bool drm_named_mode_is_in_whitelist(const char *mode, unsigned int > size) +{ > + int i; > + > + for (i = 0; i < ARRAY_SIZE(drm_named_modes_whitelist); i++) > + if (!strncmp(mode, drm_named_modes_whitelist[i], size)) > + return true; > + > + return false; > +} > + > /** > * drm_mode_parse_command_line_for_connector - parse command line modeline > for connector * @mode_option: optional per connector mode option > @@ -1794,6 +1810,10 @@ bool drm_mode_parse_command_line_for_connector(const > char *mode_option, if (named_mode) { > if (mode_end + 1 > DRM_DISPLAY_MODE_LEN) > return false; > + > + if (!drm_named_mode_is_in_whitelist(name, mode_end)) > + return false; > + > strscpy(mode->name, name, mode_end + 1); > } else { > ret = drm_mode_parse_cmdline_res_mode(name, mode_end,
On Thu, 29 Aug 2019, Jernej Škrabec <jernej.skrabec@gmail.com> wrote: > Hi! > > Dne torek, 27. avgust 2019 ob 13:58:49 CEST je Maxime Ripard napisal(a): >> From: Maxime Ripard <maxime.ripard@bootlin.com> >> >> The named modes support has introduced a number of glitches that were in >> part due to the fact that the parser will take any string as a named mode. >> >> Since we shouldn't have a lot of options there (and they should be pretty >> standard), let's introduce a whitelist of the available named modes so that >> the kernel can differentiate between a poorly formed command line and a >> named mode. >> >> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com> >> --- >> drivers/gpu/drm/drm_modes.c | 20 ++++++++++++++++++++ >> 1 file changed, 20 insertions(+) >> >> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c >> index ea7e6c8c8318..988797d8080a 100644 >> --- a/drivers/gpu/drm/drm_modes.c >> +++ b/drivers/gpu/drm/drm_modes.c >> @@ -1677,6 +1677,22 @@ static int drm_mode_parse_cmdline_options(char *str, >> size_t len, return 0; >> } >> >> +const char *drm_named_modes_whitelist[] = { >> + "NTSC", >> + "PAL", >> +}; > > That array should be static. With that fixed: And add more const for good measure: static const char * const drm_named_modes_whitelist[] = { BR, Jani. > > Reviewed-by: Jernej Skrabec <jernej.skrabec@siol.net> > > Best regards, > Jernej > >> + >> +static bool drm_named_mode_is_in_whitelist(const char *mode, unsigned int >> size) +{ >> + int i; >> + >> + for (i = 0; i < ARRAY_SIZE(drm_named_modes_whitelist); i++) >> + if (!strncmp(mode, drm_named_modes_whitelist[i], size)) >> + return true; >> + >> + return false; >> +} >> + >> /** >> * drm_mode_parse_command_line_for_connector - parse command line modeline >> for connector * @mode_option: optional per connector mode option >> @@ -1794,6 +1810,10 @@ bool drm_mode_parse_command_line_for_connector(const >> char *mode_option, if (named_mode) { >> if (mode_end + 1 > DRM_DISPLAY_MODE_LEN) >> return false; >> + >> + if (!drm_named_mode_is_in_whitelist(name, mode_end)) >> + return false; >> + >> strscpy(mode->name, name, mode_end + 1); >> } else { >> ret = drm_mode_parse_cmdline_res_mode(name, mode_end, > > > > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c index ea7e6c8c8318..988797d8080a 100644 --- a/drivers/gpu/drm/drm_modes.c +++ b/drivers/gpu/drm/drm_modes.c @@ -1677,6 +1677,22 @@ static int drm_mode_parse_cmdline_options(char *str, size_t len, return 0; } +const char *drm_named_modes_whitelist[] = { + "NTSC", + "PAL", +}; + +static bool drm_named_mode_is_in_whitelist(const char *mode, unsigned int size) +{ + int i; + + for (i = 0; i < ARRAY_SIZE(drm_named_modes_whitelist); i++) + if (!strncmp(mode, drm_named_modes_whitelist[i], size)) + return true; + + return false; +} + /** * drm_mode_parse_command_line_for_connector - parse command line modeline for connector * @mode_option: optional per connector mode option @@ -1794,6 +1810,10 @@ bool drm_mode_parse_command_line_for_connector(const char *mode_option, if (named_mode) { if (mode_end + 1 > DRM_DISPLAY_MODE_LEN) return false; + + if (!drm_named_mode_is_in_whitelist(name, mode_end)) + return false; + strscpy(mode->name, name, mode_end + 1); } else { ret = drm_mode_parse_cmdline_res_mode(name, mode_end,