linux-kernel - Re: [PATCH v5 3/6] drm/edid: Add a function to match EDID with identity

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 for Android: free password hash cracker in your pocket

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

Date: Wed, 6 Mar 2024 16:37:43 -0800
From: Doug Anderson <dianders@...omium.org>
To: Hsin-Yi Wang <hsinyi@...omium.org>
Cc: Jani Nikula <jani.nikula@...ux.intel.com>, 
	Dmitry Baryshkov <dmitry.baryshkov@...aro.org>, Neil Armstrong <neil.armstrong@...aro.org>, 
	Jessica Zhang <quic_jesszhan@...cinc.com>, Sam Ravnborg <sam@...nborg.org>, 
	Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>, Maxime Ripard <mripard@...nel.org>, 
	Thomas Zimmermann <tzimmermann@...e.de>, David Airlie <airlied@...il.com>, Daniel Vetter <daniel@...ll.ch>, 
	dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5 3/6] drm/edid: Add a function to match EDID with identity

Hi,

On Wed, Mar 6, 2024 at 4:20 PM Hsin-Yi Wang <hsinyi@...omium.org> wrote:
>
> On Wed, Mar 6, 2024 at 3:30 PM Doug Anderson <dianders@...omium.org> wrote:
> >
> > Hi,
> >
> > On Wed, Mar 6, 2024 at 12:04 PM Hsin-Yi Wang <hsinyi@...omium.org> wrote:
> > >
> > > +static void
> > > +match_identity(const struct detailed_timing *timing, void *data)
> > > +{
> > > +       struct drm_edid_match_closure *closure = data;
> > > +       unsigned int i;
> > > +       const char *name = closure->ident->name;
> > > +       unsigned int name_len = strlen(name);
> > > +       const char *desc = timing->data.other_data.data.str.str;
> > > +       unsigned int desc_len = ARRAY_SIZE(timing->data.other_data.data.str.str);
> > > +
> > > +       if (name_len > desc_len ||
> > > +           !(is_display_descriptor(timing, EDID_DETAIL_MONITOR_NAME) ||
> > > +             is_display_descriptor(timing, EDID_DETAIL_MONITOR_STRING)))
> > > +               return;
> > > +
> > > +       if (strncmp(name, desc, name_len))
> > > +               return;
> > > +
> > > +       /* Allow trailing white spaces and \0. */
> > > +       for (i = name_len; i < desc_len; i++) {
> > > +               if (desc[i] == '\n')
> > > +                       break;
> > > +               if (!isspace(desc[i]) && !desc[i])
> > > +                       return;
> > > +       }
> >
> > If my code analysis is correct, I think you'll reject the case where:
> >
> > name = "foo"
> > desc[13] = "foo \0zzzzzzzz"
> >
> > ...but you'll accept these cases:
> >
> > desc[13] = "foo \nzzzzzzzz"
> > desc[13] = "foo \0\0\0\0\0\0\0\0\0"
> >
> > It somehow seems weird to me that a '\n' terminates the string but not a '\0'.
>
> I'm also not sure about \0... based on
> https://git.linuxtv.org/edid-decode.git/tree/parse-base-block.cpp#n493,
> they use \n as terminator. Maybe we should also reject \0 before\n?
> Since it's not printable.

Ah, OK. I guess the EDID spec simply doesn't allow for '\0' in there.
I guess in that case I'd prefer simply removing the code to handle
'\0' instead of treating it like space until we see some actual need
for it. So just get rid of the "!desc[i]" case?

-Doug