[Mesa-dev] [PATCH v2 6/6] i965: Don't write past the end of the application supplied buffer

[Jan Vesely]

On Wed, 2015-03-04 at 09:55 -0800, Ian Romanick wrote:
> From: Ian Romanick <ian.d.romanick at intel.com>
> 
> Both the AMD and Intel APIs provide a dataSize parameter, and this
> function would merrily ignore it.  Neither API specifies what to do when
> the buffer isn't big enough.  I take the easy route of writing all the
> complete bits of data that will fit.  With more complete specs, we could
> probably do something different.
> 
> I noticed this while looking into an unused parameter warning.  The
> warning was actually useful!
> 
> brw_performance_monitor.c: In function 'brw_get_perf_monitor_result':
> brw_performance_monitor.c:1261:37: warning: unused parameter 'data_size' [-Wunused-parameter]
>                              GLsizei data_size,
>                                      ^
> 
> v2: Fix checks to include offset in the calculation.  Noticed by Jan.
> 
> Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>
> Cc: Kenneth Graunke <kenneth at whitecape.org>
> Cc: Jan Vesely <jan.vesely at rutgers.edu>
> ---
>  src/mesa/drivers/dri/i965/brw_performance_monitor.c | 19 ++++++++++++-------
>  1 file changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_performance_monitor.c b/src/mesa/drivers/dri/i965/brw_performance_monitor.c
> index 6c31d4c..2c8cd49 100644
> --- a/src/mesa/drivers/dri/i965/brw_performance_monitor.c
> +++ b/src/mesa/drivers/dri/i965/brw_performance_monitor.c
> @@ -1264,6 +1264,7 @@ brw_get_perf_monitor_result(struct gl_context *ctx,
>  {
>     struct brw_context *brw = brw_context(ctx);
>     struct brw_perf_monitor_object *monitor = brw_perf_monitor(m);
> +   const GLuint *const data_end = (GLuint *)((uint8_t *) data + data_size);
>  
>     DBG("GetResult(%d)\n", m->Name);
>     brw_dump_perf_monitors(brw);
> @@ -1309,9 +1310,11 @@ brw_get_perf_monitor_result(struct gl_context *ctx,
>           if (counter < 0 || !BITSET_TEST(m->ActiveCounters[group], counter))
>              continue;
>  
> -         data[offset++] = group;
> -         data[offset++] = counter;
> -         data[offset++] = monitor->oa_results[i];
> +         if (data + offset + 3 <= data_end) {
> +            data[offset++] = group;
> +            data[offset++] = counter;
> +            data[offset++] = monitor->oa_results[i];
> +         }
>        }
>  
>        clean_bookend_bo(brw);
> @@ -1335,10 +1338,12 @@ brw_get_perf_monitor_result(struct gl_context *ctx,
>  
>        for (int i = 0; i < num_counters; i++) {
>           if (BITSET_TEST(m->ActiveCounters[PIPELINE_STATS_COUNTERS], i)) {
> -            data[offset++] = PIPELINE_STATS_COUNTERS;
> -            data[offset++] = i;
> -            *((uint64_t *) (&data[offset])) = monitor->pipeline_stats_results[i];
> -            offset += 2;
> +            if (data + offset + 4 <= data_end) {
> +               data[offset++] = PIPELINE_STATS_COUNTERS;
> +               data[offset++] = i;
> +               *((uint64_t *) (&data[offset])) = monitor->pipeline_stats_results[i];
> +               offset += 2;
> +            }
>           }
>        }
>     }

don't now anything about i965 driver, but the checks seem OK now
Reviewed-by : Jan Vesely <jan.vesely at rutgers.edu>
-- 
Jan Vesely <jan.vesely at rutgers.edu>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150304/208783fd/attachment-0001.sig>