linux-kernel - Re: [lvc-project] [PATCH v2] drm/mgag200: Apply upper limit for clock variable

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: <20241114-1e41c566600f3ef18943a36f-pchelkin@ispras.ru>
Date: Thu, 14 Nov 2024 21:05:42 +0300
From: Fedor Pchelkin <pchelkin@...ras.ru>
To: Jocelyn Falempe <jfalempe@...hat.com>,
	Murad Masimov <m.masimov@...ima.ru>
Cc: Dave Airlie <airlied@...hat.com>, Simona Vetter <simona@...ll.ch>,
	lvc-project@...uxtesting.org,
	Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
	linux-kernel@...r.kernel.org, dri-devel@...ts.freedesktop.org,
	Maxime Ripard <mripard@...nel.org>,
	Thomas Zimmermann <tzimmermann@...e.de>,
	David Airlie <airlied@...il.com>
Subject: Re: [lvc-project] [PATCH v2] drm/mgag200: Apply upper limit for
 clock variable

On Thu, 14. Nov 17:47, Jocelyn Falempe wrote:
> On 11/11/2024 17:33, Murad Masimov wrote:
> > If the value of the clock variable is higher than 800000, the value of the
> > variable m, which is used as a divisor, will remain zero, because
> > (clock * testp) will be higher than vcomax in every loop iteration, which
> > leads to skipping every iteration and leaving variable m unmodified.
> > 
> > Clamp value of the clock variable between the lower and the upper limits.
> > It should be correct, since there is already a similar lower limit check.
> 
> I don't think it is correct.
> 
> If the clock asked is > 800000, then delta > premitteddelta, and it will
> return -EINVAL.

In many cases when clock is > 800000, the check won't be reached as the
division by "m" variable containing a zero value will have occured just
before.

> With your patch it will instead configure the clock to 800000 which is too
> low for the mode asked and will result in corrupted output.

Worth moving the check just after the loop or e.g. explicitly denying
clocks > 800000 at the beginning of the function?