Re: [Mesa-dev] [PATCH] st/mesa: bound the sampler count before calling into the driver.
On Sat, Mar 9, 2013 at 9:43 PM, Stéphane Marchesin wrote: > On Sat, Mar 9, 2013 at 12:30 PM, Jose Fonseca wrote: >> Looks a sensible thing to do. >> >> Reviewed-by: Jose Fonseca >> > > Thanks for the review. > >> Any insight how the caller can be fixed so that this doesn't happen? > > It happens to me when draw stages add more samplers on top of the max > samplers from the application. A couple of things: 1) The GLSL compiler should reject shaders which use more textures than the driver supports. 2) I think this patch won't help you with Draw. If you wanna fix Draw so that it doesn't exceed the max texture count limit, search for driver_set_sampler_views. Marek ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] st/mesa: bound the sampler count before calling into the driver.
On Sat, Mar 9, 2013 at 1:35 PM, Jose Fonseca wrote: > > > - Original Message - >> On Sat, Mar 9, 2013 at 12:30 PM, Jose Fonseca wrote: >> > Looks a sensible thing to do. >> > >> > Reviewed-by: Jose Fonseca >> > >> >> Thanks for the review. >> >> > Any insight how the caller can be fixed so that this doesn't happen? >> >> It happens to me when draw stages add more samplers on top of the max >> samplers from the application. > > I see. Maybe it would be safer if draw module just passed things through (and > warn) on those circumstances. I'm really trying to fix a possible security problem here, so a warning won't do it. All the gallium drivers I looked at will get an overflow in some way if the state tracker gives you > PIPE_MAX_SAMPLERS samplers. > Do real apps stress this, or just tests? > Real apps definitely exercise this, but I couldn't tell you which; I got it in a Chrome OS crash report, and I found it because subsequent members of the struct get nullified by the aaline draw stage which leads to crashes. > Another alternative would be for drivers that always depend on draw to > advertise one less stage.. Maybe, but that sounds much less flexible. Stéphane ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] st/mesa: bound the sampler count before calling into the driver.
- Original Message - > On Sat, Mar 9, 2013 at 12:30 PM, Jose Fonseca wrote: > > Looks a sensible thing to do. > > > > Reviewed-by: Jose Fonseca > > > > Thanks for the review. > > > Any insight how the caller can be fixed so that this doesn't happen? > > It happens to me when draw stages add more samplers on top of the max > samplers from the application. I see. Maybe it would be safer if draw module just passed things through (and warn) on those circumstances. Do real apps stress this, or just tests? Another alternative would be for drivers that always depend on draw to advertise one less stage.. Jose ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] st/mesa: bound the sampler count before calling into the driver.
On Sat, Mar 9, 2013 at 12:30 PM, Jose Fonseca wrote: > Looks a sensible thing to do. > > Reviewed-by: Jose Fonseca > Thanks for the review. > Any insight how the caller can be fixed so that this doesn't happen? It happens to me when draw stages add more samplers on top of the max samplers from the application. Stéphane > > Jose > > - Original Message - >> With the current code, the sampler count can become higher than >> PIPE_MAX_SAMPLERS and once it gets to the driver this can lead to >> miscellaneous crashes and memory corruptions. >> >> Although this is taken care of in debug mode with an assert, >> there is still a way to cause a crash/overflow in release mode. >> >> So instead, we bound the number of samplers in the state tracker. >> --- >> src/mesa/state_tracker/st_atom_texture.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/src/mesa/state_tracker/st_atom_texture.c >> b/src/mesa/state_tracker/st_atom_texture.c >> index fc2d690..bcd5856 100644 >> --- a/src/mesa/state_tracker/st_atom_texture.c >> +++ b/src/mesa/state_tracker/st_atom_texture.c >> @@ -311,6 +311,7 @@ update_textures(struct st_context *st, >> */ >> new_count = MAX2(*num_textures, old_max); >> assert(new_count <= max_units); >> + new_count = MIN2(new_count, max_units); >> >> cso_set_sampler_views(st->cso_context, >> shader_stage, >> -- >> 1.8.1.3 >> >> ___ >> mesa-dev mailing list >> mesa-dev@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/mesa-dev >> > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] st/mesa: bound the sampler count before calling into the driver.
Looks a sensible thing to do. Reviewed-by: Jose Fonseca Any insight how the caller can be fixed so that this doesn't happen? Jose - Original Message - > With the current code, the sampler count can become higher than > PIPE_MAX_SAMPLERS and once it gets to the driver this can lead to > miscellaneous crashes and memory corruptions. > > Although this is taken care of in debug mode with an assert, > there is still a way to cause a crash/overflow in release mode. > > So instead, we bound the number of samplers in the state tracker. > --- > src/mesa/state_tracker/st_atom_texture.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/src/mesa/state_tracker/st_atom_texture.c > b/src/mesa/state_tracker/st_atom_texture.c > index fc2d690..bcd5856 100644 > --- a/src/mesa/state_tracker/st_atom_texture.c > +++ b/src/mesa/state_tracker/st_atom_texture.c > @@ -311,6 +311,7 @@ update_textures(struct st_context *st, > */ > new_count = MAX2(*num_textures, old_max); > assert(new_count <= max_units); > + new_count = MIN2(new_count, max_units); > > cso_set_sampler_views(st->cso_context, > shader_stage, > -- > 1.8.1.3 > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/mesa-dev > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
