linux-kernel - Re: [PATCH] drm/sun4i: dsi: Prevent underflow when computing packet sizes

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

Message-ID: <093fee4d-d0b9-c6f2-8dde-d50b514fbc69@sholland.org>
Date:   Sun, 14 Aug 2022 12:54:39 -0500
From:   Samuel Holland <samuel@...lland.org>
To:     Jernej Škrabec <jernej.skrabec@...il.com>,
        Chen-Yu Tsai <wens@...e.org>,
        Maxime Ripard <mripard@...nel.org>
Cc:     Daniel Vetter <daniel@...ll.ch>, David Airlie <airlied@...ux.ie>,
        dri-devel@...ts.freedesktop.org,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        linux-sunxi@...ts.linux.dev
Subject: Re: [PATCH] drm/sun4i: dsi: Prevent underflow when computing packet
 sizes

On 8/14/22 2:55 AM, Jernej Škrabec wrote:
> Dne petek, 12. avgust 2022 ob 05:16:23 CEST je Samuel Holland napisal(a):
>> Currently, the packet overhead is subtracted using unsigned arithmetic.
>> With a short sync pulse, this could underflow and wrap around to near
>> the maximal u16 value. Fix this by using signed subtraction. The call to
>> max() will correctly handle any negative numbers that are produced.
>>
>> Apply the same fix to the other timings, even though those subtractions
>> are less likely to underflow.
>>
>> Fixes: 133add5b5ad4 ("drm/sun4i: Add Allwinner A31 MIPI-DSI controller
>> support") Signed-off-by: Samuel Holland <samuel@...lland.org>
>> ---
>>
>>  drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 10 +++++-----
>>  1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
>> b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c index b4dfa166eccd..34234a144e87
>> 100644
>> --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
>> +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
>> @@ -522,77 +522,77 @@ static void sun6i_dsi_setup_format(struct sun6i_dsi
>> *dsi, SUN6I_DSI_PIXEL_PF1_CRC_INIT_LINE0(0xffff) |
>>  		     SUN6I_DSI_PIXEL_PF1_CRC_INIT_LINEN(0xffff));
>>
>>  	regmap_write(dsi->regs, SUN6I_DSI_PIXEL_CTL0_REG,
>>  		     SUN6I_DSI_PIXEL_CTL0_PD_PLUG_DISABLE |
>>  		     SUN6I_DSI_PIXEL_CTL0_FORMAT(fmt));
>>  }
>>
>>  static void sun6i_dsi_setup_timings(struct sun6i_dsi *dsi,
>>  				    struct drm_display_mode *mode)
>>  {
>>  	struct mipi_dsi_device *device = dsi->device;
>> -	unsigned int Bpp = mipi_dsi_pixel_format_to_bpp(device->format) / 8;
>> +	int Bpp = mipi_dsi_pixel_format_to_bpp(device->format) / 8;
> 
> Nit: mipi_dsi_pixel_format_to_bpp() can return -EINVAL in case of unsupported 
> format. Would it make sense to check it?

The switch statement in mipi_dsi_pixel_format_to_bpp() handles every value in
the enumeration, so I think the -EINVAL is just there to keep GCC from
complaining. If we do want to handle this case, it would need to be in
sun6i_dsi_attach(), since the other places we use mipi_dsi_pixel_format_to_bpp()
are way too late to handle any errors.

Regards,
Samuel