linux-kernel - Re: [PATCH 1/3] drm: Widen vblank count to 64 bits. Change vblank time precision to ns

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: <86o9sxd2sw.fsf@keithp.com>
Date:   Thu, 06 Jul 2017 08:04:47 -0700
From:   Keith Packard <keithp@...thp.com>
To:     Michel Dänzer <michel@...nzer.net>,
        Dave Airlie <airlied@...hat.com>,
        Daniel Vetter <daniel@...ll.ch>
Cc:     linux-kernel@...r.kernel.org, dri-devel@...ts.freedesktop.org
Subject: Re: [PATCH 1/3] drm: Widen vblank count to 64 bits. Change vblank time precision to ns

Michel Dänzer <michel@...nzer.net> writes:

> Subtle breakage here: vblwait->request.sequence must still get updated
> for _DRM_VBLANK_RELATIVE, in case we're interrupted by a signal.

Thanks for finding this.

I think it might be better to just not modify the request.type field
instead, so that on re-entry it gets recomputed? That would mean that a
signal might cause the value to be different if the application takes a
long time processing the signal, but I'm not sure that's wrong?

>> @@ -317,6 +317,9 @@ int via_driver_irq_postinstall(struct drm_device *dev)
>>  	if (!dev_priv)
>>  		return -EINVAL;
>>  
>> +	if (dev->driver->get_vblank_counter)
>> +		dev->max_vblank_count = 0xffffffff;
>
> What's the purpose of this? All drivers providing get_vblank_counter
> should already initialize max_vblank_count correctly.

Yeah, I couldn't prove that this driver did that, and as Daniel says, we
haven't ever audited the drivers to make sure they do.

We have a check to see that they don't set max_vblank_count if they
don't provide a get function, but I can't find the matching check for
drivers that do provide a function and aren't setting max_vblank_count.

Do you have any thoughts on the wisdom of changing this API before we
have a driver that needs it?

And, of course, thanks for your review!

-- 
-keith

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