On Mon, Aug 15, 2016 at 08:19:47PM +0530, akash.goel@xxxxxxxxx wrote:
> +static void guc_read_update_log_buffer(struct intel_guc *guc)
> +{
> + struct guc_log_buffer_state *log_buffer_state, *log_buffer_snapshot_state;
> + struct guc_log_buffer_state log_buffer_state_local;
> + void *src_data_ptr, *dst_data_ptr;
> + unsigned int buffer_size, expected_size;
> + enum guc_log_buffer_type type;
> +
> + if (WARN_ON(!guc->log.buf_addr))
> + return;
> +
> + /* Get the pointer to shared GuC log buffer */
> + log_buffer_state = src_data_ptr = guc->log.buf_addr;
> +
> + /* Get the pointer to local buffer to store the logs */
> + dst_data_ptr = log_buffer_snapshot_state = guc_get_write_buffer(guc);
> +
> + /* Actual logs are present from the 2nd page */
> + src_data_ptr += PAGE_SIZE;
> + dst_data_ptr += PAGE_SIZE;
> +
> + for (type = GUC_ISR_LOG_BUFFER; type < GUC_MAX_LOG_BUFFER; type++) {
> + /* Make a copy of the state structure in GuC log buffer (which
> + * is uncached mapped) on the stack to avoid reading from it
> + * multiple times.
> + */
> + memcpy(&log_buffer_state_local, log_buffer_state,
> + sizeof(struct guc_log_buffer_state));
> + buffer_size = log_buffer_state_local.size;
> +
> + if (log_buffer_snapshot_state) {
> + /* First copy the state structure in snapshot buffer */
> + memcpy(log_buffer_snapshot_state, &log_buffer_state_local,
> + sizeof(struct guc_log_buffer_state));
> +
> + /* The write pointer could have been updated by the GuC
> + * firmware, after sending the flush interrupt to Host,
> + * for consistency set the write pointer value to same
> + * value of sampled_write_ptr in the snapshot buffer.
> + */
> + log_buffer_snapshot_state->write_ptr =
> + log_buffer_snapshot_state->sampled_write_ptr;
> +
> + log_buffer_snapshot_state++;
> +
> + /* Now copy the actual logs, but before that validate
> + * the buffer size value retrieved from state structure.
> + */
> + if (type == GUC_ISR_LOG_BUFFER)
> + expected_size = (GUC_LOG_ISR_PAGES+1)*PAGE_SIZE;
> + else if (type == GUC_DPC_LOG_BUFFER)
> + expected_size = (GUC_LOG_DPC_PAGES+1)*PAGE_SIZE;
> + else
> + expected_size = (GUC_LOG_CRASH_PAGES+1)*PAGE_SIZE;
> +
> + if (unlikely(buffer_size != expected_size)) {
> + DRM_ERROR("unexpected log buffer size\n");
> + /* Continue with further copying, already state
> + * structure has been copied which is enough to
> + * let Userspace know about the anomaly.
> + */
> + buffer_size = expected_size;
Urm, no.
You tell userspace one thing and then do another. This code should just
be a conduit and not apply its own outdated interpretation.
> + }
> +
> + memcpy(dst_data_ptr, src_data_ptr, buffer_size);
Where do you validate that buffer_size is sane before copying?
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
