linux-kernel - Re: [PATCH v5 08/22] drm/modes: Move named modes parsing to a separate function

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20221018075742.7g3sfay3o7wk54kv@houat>
Date:   Tue, 18 Oct 2022 09:57:42 +0200
From:   Maxime Ripard <maxime@...no.tech>
To:     Noralf Trønnes <noralf@...nnes.org>
Cc:     Karol Herbst <kherbst@...hat.com>,
        Jani Nikula <jani.nikula@...ux.intel.com>,
        Tvrtko Ursulin <tvrtko.ursulin@...ux.intel.com>,
        Daniel Vetter <daniel@...ll.ch>,
        Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
        David Airlie <airlied@...ux.ie>,
        Joonas Lahtinen <joonas.lahtinen@...ux.intel.com>,
        Lyude Paul <lyude@...hat.com>, Emma Anholt <emma@...olt.net>,
        Chen-Yu Tsai <wens@...e.org>,
        Samuel Holland <samuel@...lland.org>,
        Ben Skeggs <bskeggs@...hat.com>,
        Thomas Zimmermann <tzimmermann@...e.de>,
        Rodrigo Vivi <rodrigo.vivi@...el.com>,
        Jernej Skrabec <jernej.skrabec@...il.com>,
        Dom Cobley <dom@...pberrypi.com>, linux-sunxi@...ts.linux.dev,
        Dave Stevenson <dave.stevenson@...pberrypi.com>,
        intel-gfx@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
        nouveau@...ts.freedesktop.org,
        Geert Uytterhoeven <geert@...ux-m68k.org>,
        linux-arm-kernel@...ts.infradead.org,
        Mateusz Kwiatkowski <kfyatek+publicgit@...il.com>,
        dri-devel@...ts.freedesktop.org,
        Hans de Goede <hdegoede@...hat.com>,
        Phil Elwell <phil@...pberrypi.com>
Subject: Re: [PATCH v5 08/22] drm/modes: Move named modes parsing to a
 separate function

On Sun, Oct 16, 2022 at 06:11:21PM +0200, Noralf Trønnes wrote:
> Den 13.10.2022 15.18, skrev Maxime Ripard:
> > The current construction of the named mode parsing doesn't allow to extend
> > it easily. Let's move it to a separate function so we can add more
> > parameters and modes.
> > 
> > In order for the tests to still pass, some extra checks are needed, so
> > it's not a 1:1 move.
> > 
> > Signed-off-by: Maxime Ripard <maxime@...no.tech>
> > 
> 
> I was hoping that someone else would step up and review these parser
> patches since the parser code is rather difficult to read, for me at
> least. I have studied it now, so I'll give it a try.
> 
> > ---
> > Changes in v4:
> > - Fold down all the named mode patches that were split into a single
> >   patch again to maintain bisectability
> > ---
> >  drivers/gpu/drm/drm_modes.c | 73 ++++++++++++++++++++++++++++++++++++++-------
> >  1 file changed, 62 insertions(+), 11 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> > index c0dceff51cac..2f020ef2ddf2 100644
> > --- a/drivers/gpu/drm/drm_modes.c
> > +++ b/drivers/gpu/drm/drm_modes.c
> > @@ -2229,6 +2229,55 @@ static const char * const drm_named_modes_whitelist[] = {
> >  	"PAL",
> >  };
> >  
> > +static int drm_mode_parse_cmdline_named_mode(const char *name,
> > +					     unsigned int name_end,
> > +					     struct drm_cmdline_mode *cmdline_mode)
> > +{
> > +	unsigned int i;
> > +
> > +	if (!name_end)
> > +		return 0;
> 
> name_end can't be zero since the argument is checked before calling this
> function.

I'd really like to keep it in though. At least, we know by looking at
this small function that we're going to be safe all the time, no matter
what the caller does.

And if the caller wants to check it as well, fine, it's only a simple
comparison ran once or twice at boot, it's not like it's in a hot-path.

I've addressed your other comments, thanks!
Maxime

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)