On Wed, 09 Dec 2015, LABBE Corentin <clabbe.montjoie@xxxxxxxxx> wrote:
> My latest commit introduce some case where a valid mode, could be
> rejected.
> simple_strtox functions stop at first non-digit character, but kstrtox not.
> So args like "video=HDMI-A-1:720x480-16@60" will be reject when checking 16@.
> The proper solution is to store digits in a specific buffer.
Or to revert regressing commit...? Your original commit complicated the
already complicated function, and this one makes it more so. What is the
benefit?
> Fixes: 52157a4ca396 ("drm: modes: replace simple_strtoul by kstrtouint")
For me the commit id is cc344980c76748e57c9c03100c2a14d36ab00334.
BR,
Jani.
> Reported-by: Kuninori Morimoto <kuninori.morimoto.gx@xxxxxxxxxxx>
> Signed-off-by: LABBE Corentin <clabbe.montjoie@xxxxxxxxx>
> ---
> drivers/gpu/drm/drm_modes.c | 28 +++++++++++++++++++++-------
> 1 file changed, 21 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> index bde9b29..da1e80d 100644
> --- a/drivers/gpu/drm/drm_modes.c
> +++ b/drivers/gpu/drm/drm_modes.c
> @@ -1225,13 +1225,14 @@ bool drm_mode_parse_command_line_for_connector(const char *mode_option,
> struct drm_cmdline_mode *mode)
> {
> const char *name;
> - unsigned int namelen;
> + unsigned int namelen, digit_i;
> bool res_specified = false, bpp_specified = false, refresh_specified = false;
> unsigned int xres = 0, yres = 0, bpp = 32, refresh = 0;
> bool yres_specified = false, cvt = false, rb = false;
> bool interlace = false, margins = false, was_digit = false;
> int i, err;
> enum drm_connector_force force = DRM_FORCE_UNSPECIFIED;
> + char *digits;
>
> #ifdef CONFIG_FB
> if (!mode_option)
> @@ -1245,42 +1246,53 @@ bool drm_mode_parse_command_line_for_connector(const char *mode_option,
>
> name = mode_option;
> namelen = strlen(name);
> +
> + digits = kzalloc(namelen, GFP_KERNEL);
> + if (!digits)
> + return false;
> + /* The last character must be the last 0 */
> + digit_i = namelen;
> +
> for (i = namelen-1; i >= 0; i--) {
> switch (name[i]) {
> case '@':
> if (!refresh_specified && !bpp_specified &&
> !yres_specified && !cvt && !rb && was_digit) {
> - err = kstrtouint(&name[i + 1], 10, &refresh);
> + err = kstrtouint(&digits[digit_i], 10, &refresh);
> if (err)
> - return false;
> + goto done;
> refresh_specified = true;
> was_digit = false;
> + digit_i = namelen;
> } else
> goto done;
> break;
> case '-':
> if (!bpp_specified && !yres_specified && !cvt &&
> !rb && was_digit) {
> - err = kstrtouint(&name[i + 1], 10, &bpp);
> + err = kstrtouint(&digits[digit_i], 10, &bpp);
> if (err)
> - return false;
> + goto done;
> bpp_specified = true;
> was_digit = false;
> + digit_i = namelen;
> } else
> goto done;
> break;
> case 'x':
> if (!yres_specified && was_digit) {
> - err = kstrtouint(&name[i + 1], 10, &yres);
> + err = kstrtouint(&digits[digit_i], 10, &yres);
> if (err)
> - return false;
> + goto done;
> yres_specified = true;
> was_digit = false;
> + digit_i = namelen;
> } else
> goto done;
> break;
> case '0' ... '9':
> was_digit = true;
> + digits[--digit_i] = name[i];
> break;
> case 'M':
> if (yres_specified || cvt || was_digit)
> @@ -1349,6 +1361,7 @@ done:
> "parse error at position %i in video mode '%s'\n",
> i, name);
> mode->specified = false;
> + kfree(digits);
> return false;
> }
>
> @@ -1373,6 +1386,7 @@ done:
> mode->margins = margins;
> mode->force = force;
>
> + kfree(digits);
> return true;
> }
> EXPORT_SYMBOL(drm_mode_parse_command_line_for_connector);
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
