Commit 9ad6d99f authored by Ville Syrjälä's avatar Ville Syrjälä Committed by Daniel Vetter

drm/i915: Make i915_pipe_crc_read() oops proof

Currently i915_pipe_crc_read() will drop pipe_crc->lock for the entire
duration of the copy_to_user() loop, which means it'll access
pipe_crc->entries without any protection. If another thread sneaks in
and frees pipe_crc->entries the code will oops.

Reorganize the code to hold the lock around everything except
copy_to_user(). After the copy the lock is reacquired and the the number
of available entries is rechecked.

Since this is a debug feature simplify the error handling a bit by
consuming the crc entry even if copy_to_user() would fail.
Signed-off-by: default avatarVille Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
parent 3cf54b34
...@@ -2858,7 +2858,7 @@ i915_pipe_crc_read(struct file *filep, char __user *user_buf, size_t count, ...@@ -2858,7 +2858,7 @@ i915_pipe_crc_read(struct file *filep, char __user *user_buf, size_t count,
struct drm_i915_private *dev_priv = dev->dev_private; struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[info->pipe]; struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[info->pipe];
char buf[PIPE_CRC_BUFFER_LEN]; char buf[PIPE_CRC_BUFFER_LEN];
int head, tail, n_entries, n; int n_entries;
ssize_t bytes_read; ssize_t bytes_read;
/* /*
...@@ -2890,36 +2890,39 @@ i915_pipe_crc_read(struct file *filep, char __user *user_buf, size_t count, ...@@ -2890,36 +2890,39 @@ i915_pipe_crc_read(struct file *filep, char __user *user_buf, size_t count,
} }
/* We now have one or more entries to read */ /* We now have one or more entries to read */
head = pipe_crc->head; n_entries = count / PIPE_CRC_LINE_LEN;
tail = pipe_crc->tail;
n_entries = min((size_t)CIRC_CNT(head, tail, INTEL_PIPE_CRC_ENTRIES_NR),
count / PIPE_CRC_LINE_LEN);
spin_unlock_irq(&pipe_crc->lock);
bytes_read = 0; bytes_read = 0;
n = 0; while (n_entries > 0) {
do { struct intel_pipe_crc_entry *entry =
struct intel_pipe_crc_entry *entry = &pipe_crc->entries[tail]; &pipe_crc->entries[pipe_crc->tail];
int ret; int ret;
if (CIRC_CNT(pipe_crc->head, pipe_crc->tail,
INTEL_PIPE_CRC_ENTRIES_NR) < 1)
break;
BUILD_BUG_ON_NOT_POWER_OF_2(INTEL_PIPE_CRC_ENTRIES_NR);
pipe_crc->tail = (pipe_crc->tail + 1) & (INTEL_PIPE_CRC_ENTRIES_NR - 1);
bytes_read += snprintf(buf, PIPE_CRC_BUFFER_LEN, bytes_read += snprintf(buf, PIPE_CRC_BUFFER_LEN,
"%8u %8x %8x %8x %8x %8x\n", "%8u %8x %8x %8x %8x %8x\n",
entry->frame, entry->crc[0], entry->frame, entry->crc[0],
entry->crc[1], entry->crc[2], entry->crc[1], entry->crc[2],
entry->crc[3], entry->crc[4]); entry->crc[3], entry->crc[4]);
ret = copy_to_user(user_buf + n * PIPE_CRC_LINE_LEN, spin_unlock_irq(&pipe_crc->lock);
buf, PIPE_CRC_LINE_LEN);
ret = copy_to_user(user_buf, buf, PIPE_CRC_LINE_LEN);
if (ret == PIPE_CRC_LINE_LEN) if (ret == PIPE_CRC_LINE_LEN)
return -EFAULT; return -EFAULT;
BUILD_BUG_ON_NOT_POWER_OF_2(INTEL_PIPE_CRC_ENTRIES_NR); user_buf += PIPE_CRC_LINE_LEN;
tail = (tail + 1) & (INTEL_PIPE_CRC_ENTRIES_NR - 1); n_entries--;
n++;
} while (--n_entries);
spin_lock_irq(&pipe_crc->lock); spin_lock_irq(&pipe_crc->lock);
pipe_crc->tail = tail; }
spin_unlock_irq(&pipe_crc->lock); spin_unlock_irq(&pipe_crc->lock);
return bytes_read; return bytes_read;
...@@ -3458,6 +3461,8 @@ static int pipe_crc_set_source(struct drm_device *dev, enum pipe pipe, ...@@ -3458,6 +3461,8 @@ static int pipe_crc_set_source(struct drm_device *dev, enum pipe pipe,
spin_lock_irq(&pipe_crc->lock); spin_lock_irq(&pipe_crc->lock);
entries = pipe_crc->entries; entries = pipe_crc->entries;
pipe_crc->entries = NULL; pipe_crc->entries = NULL;
pipe_crc->head = 0;
pipe_crc->tail = 0;
spin_unlock_irq(&pipe_crc->lock); spin_unlock_irq(&pipe_crc->lock);
kfree(entries); kfree(entries);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment