<div dir="auto"><div><br><div class="gmail_extra"><br><div class="gmail_quote">On Feb 28, 2017 5:03 AM, "Timothy Arceri" <<a href="mailto:tarceri@itsqueeze.com">tarceri@itsqueeze.com</a>> wrote:<br type="attribution"><blockquote class="quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="elided-text"><br> <br> On 27/02/17 20:57, Michel Dänzer wrote:<br> <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> On 25/02/17 01:56 PM, Timothy Arceri wrote:<br> <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> On 24/02/17 21:02, Marek Olšák wrote:<br> <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> On Fri, Feb 24, 2017 at 3:18 AM, Timothy Arceri<br> <<a href="mailto:tarceri@itsqueeze.com" target="_blank">tarceri@itsqueeze.com</a>> wrote:<br> <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> On 24/02/17 08:49, Timothy Arceri wrote:<br> <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> On 24/02/17 05:12, Marek Olšák wrote:<br> <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> On Thu, Feb 23, 2017 at 3:09 AM, Timothy Arceri<br> <<a href="mailto:tarceri@itsqueeze.com" target="_blank">tarceri@itsqueeze.com</a>> wrote:<br> <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> <br> From: kdj0c <<a href="mailto:kdj0c@djinvi.net" target="_blank">kdj0c@djinvi.net</a>><br> <br> V2 (Timothy Arceri):<br> - when loading from disk cache also binary insert into memory cache.<br> - check that the binary loaded from disk is the correct size. If not<br> delete the cache item and skip loading from cache.<br> ---<br> src/gallium/drivers/radeonsi/<wbr>si_state_shaders.c | 69<br> ++++++++++++++++++++++---<br> 1 file changed, 62 insertions(+), 7 deletions(-)<br> <br> diff --git a/src/gallium/drivers/radeonsi<wbr>/si_state_shaders.c<br> b/src/gallium/drivers/radeonsi<wbr>/si_state_shaders.c<br> index f615aa8..71556f9 100644<br> --- a/src/gallium/drivers/radeonsi<wbr>/si_state_shaders.c<br> +++ b/src/gallium/drivers/radeonsi<wbr>/si_state_shaders.c<br> @@ -36,6 +36,9 @@<br> #include "util/u_memory.h"<br> #include "util/u_prim.h"<br> <br> +#include "util/disk_cache.h"<br> +#include "util/mesa-sha1.h"<br> +<br> /* SHADER_CACHE */<br> <br> /**<br> @@ -182,10 +185,12 @@ static bool si_load_shader_binary(struct<br> si_shader *shader, void *binary)<br> */<br> static bool si_shader_cache_insert_shader(<wbr>struct si_screen *sscreen,<br> void *tgsi_binary,<br> - struct si_shader *shader)<br> + struct si_shader *shader,<br> + bool<br> insert_into_disk_cache)<br> {<br> void *hw_binary;<br> struct hash_entry *entry;<br> + uint8_t key[CACHE_KEY_SIZE];<br> <br> entry = _mesa_hash_table_search(sscree<wbr>n->shader_cache,<br> tgsi_binary);<br> if (entry)<br> @@ -201,6 +206,12 @@ static bool si_shader_cache_insert_shader(<wbr>struct<br> si_screen *sscreen,<br> return false;<br> }<br> <br> + if (sscreen->b.disk_shader_cache && insert_into_disk_cache) {<br> + _mesa_sha1_compute(tgsi_binar<wbr>y, *((uint32_t<br> *)tgsi_binary), key);<br> </blockquote> <br> <br> What happens if we randomly get a sha1 collision?<br> </blockquote> <br> <br> You should stop playing your game which will be rendering incorrectly<br> and by a lotto ticket.<br> <br> <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> Shouldn't we store the whole key as well?<br> </blockquote> <br> <br> Sure I can add that, its cheap to check here anyway. Although the other<br> cache stages rely on a collision being improbable.<br> </blockquote> <br> <br> <br> For some reason I thought the key was simpler than it is. It seems<br> excessive<br> to store and compare the tgsi again. I don't think git even worries<br> about<br> the possibility of a collision and we will be dealing with much smaller<br> amounts of cache items then commits in a git repository.<br> <br> Thoughts?<br> </blockquote> <br> I'll let others comment on this. If nobody comments, checking only the<br> key can stay.<br> </blockquote> <br> Seems SVN didn't used to worry about collisions either.<br> <br> <a href="https://arstechnica.com/security/2017/02/watershed-sha1-collision-just-broke-the-webkit-repository-others-may-follow/" rel="noreferrer" target="_blank">https://arstechnica.com/securi<wbr>ty/2017/02/watershed-sha1-<wbr>collision-just-broke-the-webki<wbr>t-repository-others-may-follow<wbr>/</a><br> </blockquote> <br> What scenario are we concerned about here? Getting a genuine SHA1<br> collision between two shader objects is still as unlikely as it ever was<br> (virtually impossible?), and someone with access to the shader cache<br> files can wreak havoc much more easily than via a hash collision.<br> </blockquote> <br></div> I guess in theory someone could craft some shaders for webgl to cause a collision, although if we want to avoid that we should probably use a different hash.<br></blockquote></div></div></div><div dir="auto"><br></div><div dir="auto">A shader with an infinite loop is an easier way to cause troubles.</div><div dir="auto"><br></div><div dir="auto">OK I guess the collision issue is closed.</div><div dir="auto"><br></div><div dir="auto">Marek</div><div dir="auto"><br></div><div dir="auto"><br></div><div dir="auto"><div class="gmail_extra"><div class="gmail_quote"><blockquote class="quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> <br> <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> <br> <br> </blockquote> </blockquote></div><br></div></div></div>