[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Public WebGL] Need to spec range checking for WebGL ANGLE_instanced_arrays?



Thanks for submitting those pull requests. If you could add back in
the removed benchmark per comments on
https://github.com/KhronosGroup/WebGL/pull/501 , I'll merge it right
away. We should expose as many issues in as many implementations of
index validation as possible.

Florian, would you please take a look at the above pull request too?
I'd really appreciate your feedback on how to add more real-world
scenarios.

Thanks,

-Ken



On Mon, Mar 17, 2014 at 6:39 AM, Olli Etuaho <oetuaho@nvidia.com> wrote:
> The drawElements overhead benchmark is now here: https://github.com/KhronosGroup/WebGL/pull/501 and I added a pull request for the extension spec as well: https://github.com/KhronosGroup/WebGL/pull/502
>
> As Benoit pointed out in the other thread though, Firefox already handles the usual case where only small index buffer updates happen quite well, so we might want to avoid any spec changes before taking a more careful look at problematic cases. I feel that re-specifying drawElements could still be advantageous to get rid of the memory overhead and gain maximum performance even in obscure cases, but if the validation using a binary heap can perform well in all real-world cases, it could be an acceptable solution.
>
> I am investigating whether ARB_robust_buffer_access_behavior wording could be changed, but I don't have further information on that yet.
> ________________________________________
> From: Kenneth Russell [kbr@google.com]
> Sent: Monday, March 17, 2014 9:02 AM
> To: Olli Etuaho
> Cc: Florian Bösch; public_webgl@khronos.org
> Subject: Re: [Public WebGL] Need to spec range checking for WebGL ANGLE_instanced_arrays?
>
> On Fri, Mar 14, 2014 at 8:39 AM, Olli Etuaho <oetuaho@nvidia.com> wrote:
>> Sounds good, though I don't think your interpretation of GL_ARB_robust_buffer_access_behavior is entirely waterproof. The extension gives sufficient security guarantees, but the wording is vague enough that implementations can't be trusted to be identical. Jeff Bolz, who was one of the authors of the extension spec, agreed with me on this. At minimum we need to clarify the extension spec if we want to make this core behavior in WebGL. Right now, at least these two interpretations seem possible:
>>
>> 1. If an index is pointing past the storage of one particular bound buffer, set only that attribute to zero.
>> 2. If an index is pointing past the storage of one particular bound buffer, set all attributes to zero.
>>
>> Also the case where an indexed attribute is partially inside and partially outside a buffer might be problematic.
>
> That's unfortunate. I wish that the issues with the extension spec had
> been thought through and raised earlier.
>
> Do you have a feeling whether GL_ARB_robust_buffer_access can be
> clarified, and how? What about whether the WebGL conformance tests
> could be written with restricted enough cases to allow multiple
> implementations of the current GL_ARB_robust_buffer_access to pass it?
>
>
>> Based on this going with a WebGL extension seems like a better idea. Here's an extension proposal based on my discussion with Jeff Gilbert:
>> https://github.com/NVIDIA/WebGL/commit/bfc70090e17840213f431ea4f485b7f2d7486d0b
>
> The extension spec looks fine. Thanks for putting it together. Would
> you submit it as a pull request so that it can be more easily reviewed
> by the community?
>
>
>> I have a crude JS benchmark for just measuring the validation overhead, targeting the current implementations in Chrome and Firefox. The browsers seem to be able to validate around 80000 uint16 indices per millisecond on my Core i7 workstation. The current Firefox implementation is faster than that in Chrome for certain kinds of content, but it does have its corner cases as well. I can probably share my benchmark for what it's worth after a bit of clean-up.
>
> Please do clean it up and submit it also as a pull request into the
> WebGL repository -- perhaps under sdk/tests/extra/ .
>
> -Ken
>
>
>
>> -Olli
>> ________________________________________
>> From: Kenneth Russell [kbr@google.com]
>> Sent: Friday, March 14, 2014 4:24 AM
>> To: Olli Etuaho
>> Cc: Florian Bösch; public_webgl@khronos.org
>> Subject: Re: [Public WebGL] Need to spec range checking for WebGL ANGLE_instanced_arrays?
>>
>> Thanks for raising this issue and for filing pull request
>> https://github.com/KhronosGroup/WebGL/pull/495 .
>>
>> This extension must not be able to sidestep or otherwise subvert the
>> security guarantees provided in the core spec's drawArrays and
>> drawElements calls, so to provide consistent behavior, let's merge
>> this pull request.
>>
>> The WebGL WG needs to revisit the range checking behavior in
>> drawArrays and drawElements. If both GL_ARB_robustness and
>> GL_ARB_robust_buffer_access_behavior are available then it's possible
>> to have a secure and testable WebGL spec that doesn't generate
>> INVALID_OPERATION due to out of range indices or accesses during these
>> draw calls. This came up a while back. Mozilla stated that their
>> implementation of range checking is highly optimized and carries very
>> little overhead even for dynamically changing indices. I was supposed
>> to write a benchmark which dynamically updated the ELEMENT_ARRAY
>> buffer and did its rendering using drawElements (this is slow in
>> Chrome right now) and see whether Firefox yielded any speedup if its
>> range checking code were completely disabled. That would inform
>> whether we should preserve the INVALID_OPERATION generation in the
>> WebGL spec. Unfortunately I dropped the ball on this, but will try to
>> pick it up again. If anyone would like to write such a benchmark so
>> that we can see just how expensive the index validation in the various
>> WebGL implementations is, that would be very welcome.
>>
>> -Ken
>>
>>
>>
>> On Tue, Mar 11, 2014 at 4:42 AM, Olli Etuaho <oetuaho@nvidia.com> wrote:
>>>
>>> The definition of robust buffer access in ARB_robustness says that the implementation may return undefined values, and these could include data that the WebGL context is not supposed to be able to access. So relying on that is insecure.
>>>
>>> ARB_robust_buffer_access_behavior is better in terms of security, but not as widely supported. That's being looked at, but I'd prefer if this thread would just be about what to do with range checking for ANGLE_instanced_arrays. Since the underlying graphics APIs don't always give enough guarantees, some checking needs to be present on the CPU side, and so at least Chrome is currently producing errors that are not explicitly specified. I agree with your suggested minimum of specifying that INVALID_OPERATION is produced whenever an out-of-range attribute would be accessed regardless of the which draw call is made, but it might be easier to interpret the spec as an implementer if it spelled things out a bit more.
>>> ________________________________________
>>> From: Florian Bösch [pyalot@gmail.com]
>>> Sent: Tuesday, March 11, 2014 12:27 PM
>>> To: Olli Etuaho
>>> Cc: public_webgl@khronos.org
>>> Subject: Re: [Public WebGL] Need to spec range checking for WebGL ANGLE_instanced_arrays?
>>>
>>> On Tue, Mar 11, 2014 at 11:18 AM, Olli Etuaho <oetuaho@nvidia.com<mailto:oetuaho@nvidia.com>> wrote:
>>>> Well, ARB_robustness is not a part of the WebGL spec.
>>> I'm aware of that.
>>>
>>>> It's a part of how WebGL is typically implemented on OpenGL, and robust buffer access in ARB_robustness is tangentially related, but it doesn't give solid security guarantees
>>> So your answer is that ARB_robustness does not validate the vertexPointer validity on drawElements, drawArrays and neither on drawElementsInstanced and drawArraysInstanced? My reading of the ARB_robustness specification suggests that ARB_robustness does in fact do those things. Could you please clarify?
>>>
>>>> and beyond that I don't see how ARB_robustness would come into play here. The issue is that at least one browser vendor has thought it necessary to have these unspecified checks, so it seems like a good idea to me to correct the omission in the spec and tests.
>>>
>>> ARB_robustness comes into play these ways
>>>
>>>  1.   if ARB_robustness specifies the checks (and enforces them) than an implementation should use that because:
>>>  2.  Manual checking isn't free, it makes it practically infeasible to use drawElements except under very specific rarely encountered situations. Manual checking means that everytime drawElements is called, that all indices are validated on the CPU. So ARB_robustiness guarantees are of great relevance here.
>>>  3.  Since neither the extension specifications for OpenGL, nor for OpenGL ES extensions, nor the core specification of OpenGL nor the core specification of OpenGL ES defines these checks, and the only reference to checks is to be found in ARB_robustness in the standard specifications, and in two paragraphs in the WebGL specification, it is questionable if further specification is required at all. I think at a minimum the WebGL specification could be updated not to specifically mention drawElements and drawArrays, and make it clear that appropriate range checks would have to be performed regardless of how things are drawn, and regardless of how these checks are implemented underneath.
>>>
>>>
>>>
>>> -----------------------------------------------------------
>>> You are currently subscribed to public_webgl@khronos.org.
>>> To unsubscribe, send an email to majordomo@khronos.org with
>>> the following command in the body of your email:
>>> unsubscribe public_webgl
>>> -----------------------------------------------------------
>>>

-----------------------------------------------------------
You are currently subscribed to public_webgl@khronos.org.
To unsubscribe, send an email to majordomo@khronos.org with
the following command in the body of your email:
unsubscribe public_webgl
-----------------------------------------------------------