linux-kernel - Re: [PATCH] drm/mipi-dsi: Add a mipi_dual_dsi_dcs_write

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] [day] [month] [year] [list]

Message-ID: <CACRpkdYwu5XKDcowggDMM0pSjuKdAJnZ8F92OdDyEWP7HJUC-A@mail.gmail.com>
Date:   Sat, 11 Mar 2023 00:05:44 +0100
From:   Linus Walleij <linus.walleij@...aro.org>
To:     Jani Nikula <jani.nikula@...ux.intel.com>,
        Javier Martinez Canillas <javierm@...hat.com>
Cc:     Jianhua Lu <lujianhua000@...il.com>,
        Thomas Zimmermann <tzimmermann@...e.de>,
        linux-kernel@...r.kernel.org, dri-devel@...ts.freedesktop.org,
        phone-devel@...r.kernel.org, ~postmarketos/upstreaming@...ts.sr.ht
Subject: Re: [PATCH] drm/mipi-dsi: Add a mipi_dual_dsi_dcs_write_seq() macro

(CC Javier)

On Fri, Mar 10, 2023 at 2:52 PM Jani Nikula <jani.nikula@...ux.intel.com> wrote:

> >> > +          for (i = 0; i < ARRAY_SIZE(dsi); i++)                    \
> >> > +                  mipi_dsi_dcs_write_seq(dsi[i], cmd, seq);        \
> >>
> >> This ignores errors.
> > mipi_dsi_dcs_write_seq is also a macro, contains error checks in the body block.
>
> Ugh, I think it's pretty scary to hide control flow like return
> statements in macros like this.

The macros are written like this because:

#define mipi_dsi_generic_write_seq(dsi, seq...)
(...)
                static const u8 d[] = { seq };

Array of bytes

                ret = mipi_dsi_generic_write(dsi, d, ARRAY_SIZE(d));

So we can use use ARRAY_SIZE() in the macro and pass in any
arbitrary sequence, e.g.

mipi_dsi_generic_write_seq(dsi, 0xfb, 0x01);

Any function-esque definitions will (as in your example) require a
length to be passed in so it would become:

mipi_dsi_generic_write_seq(dsi, 0xfb, 0x01, 2);

And if you grep mipi_dsi_generic_write_seq | wc -l you find all the
245 opportunities to get that last len wrong and cause an out-of-bounds
bug.

I think this macro is the lesser evil for this reason, also it saves code
that you otherwise have to do manually, and one should never put
a person to do a machine's job.

Any suggestions to rewrite the macro using varargs welcome.
I think it is not very easy to do without the preprocessor.

Yours,
Linus Walleij