drm_read() to paged-out memory area

[Chris Wilson]

On Tue, Nov 24, 2015 at 09:49:58PM +0100, Thomas Hellstrom wrote:
> Hi, Chris,
> 
> With your new (well sort of) implementation of drm_read() it looks to me
> like a drm_read() to a paged out
> memory area will always fail with -EFAULT. From what I can tell, there's
> nothing there to generate a page-fault to get the destination paged back
> into memory?

True. Whoops.

If we take the first event (remove it from the list from a second reader
cannot see it), fail to copy it and replace it at the head, the second
reader may then get an out-of-order event.

First we need something like:

diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c
index c59ce4d0ef75..b9ca549e0e99 100644
--- a/drivers/gpu/drm/drm_fops.c
+++ b/drivers/gpu/drm/drm_fops.c
@@ -509,14 +509,21 @@ ssize_t drm_read(struct file *filp, char __user *buffer,
                        ret = 0;
                } else {
                        struct drm_pending_event *e;
+                       int unwritten;
 
                        e = list_first_entry(&file_priv->event_list,
                                             struct drm_pending_event, link);
                        if (e->event->length + ret > count)
                                break;
 
-                       if (__copy_to_user_inatomic(buffer + ret,
-                                                   e->event, e->event->length)) {
+                       list_del(&e->link);
+
+                       spin_unlock_irq(&dev->event_lock);
+                       unwritten = copy_to_user(buffer + ret,
+                                                e->event, e->event->length);
+                       spin_lock_irq(&dev->event_lock);
+                       if (unwritten) {
+                               list_add(&e->link, &file_priv->event_list);
                                if (ret == 0)
                                        ret = -EFAULT;
                                break;
@@ -524,7 +531,6 @@ ssize_t drm_read(struct file *filp, char __user *buffer,
 
                        file_priv->event_space += e->event->length;
                        ret += e->event->length;
-                       list_del(&e->link);
                        e->destroy(e);
                }
        }


A task for tomorrow (along with a suitable testcase).
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre