diff mbox series

[v3,8/9] kernel: export task_work_add

Message ID 20210818073824.1560124-9-desmondcheongzx@gmail.com (mailing list archive)
State New, archived
Headers show
Series drm, kernel: update locking for DRM | expand

Commit Message

Desmond Cheong Zhi Xi Aug. 18, 2021, 7:38 a.m. UTC
The task_work_add function is needed to prevent userspace races with
DRM modesetting rights.

Some DRM ioctls can change modesetting permissions while other
concurrent users are performing modesetting. To prevent races with
userspace, such functions should flush readers of old permissions
before returning to user mode. As the function that changes
permissions might itself be a reader of the old permissions, we intend
to schedule this flush using task_work_add.

However, when DRM is compiled as a loadable kernel module without
exporting task_work_add, we get the following compilation error:

ERROR: modpost: "task_work_add" [drivers/gpu/drm/drm.ko] undefined!

Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Desmond Cheong Zhi Xi <desmondcheongzx@gmail.com>
---
 kernel/task_work.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Daniel Vetter Aug. 18, 2021, 11:06 a.m. UTC | #1
On Wed, Aug 18, 2021 at 03:38:23PM +0800, Desmond Cheong Zhi Xi wrote:
> The task_work_add function is needed to prevent userspace races with
> DRM modesetting rights.
> 
> Some DRM ioctls can change modesetting permissions while other
> concurrent users are performing modesetting. To prevent races with
> userspace, such functions should flush readers of old permissions
> before returning to user mode. As the function that changes
> permissions might itself be a reader of the old permissions, we intend
> to schedule this flush using task_work_add.
> 
> However, when DRM is compiled as a loadable kernel module without
> exporting task_work_add, we get the following compilation error:
> 
> ERROR: modpost: "task_work_add" [drivers/gpu/drm/drm.ko] undefined!
> 
> Reported-by: kernel test robot <lkp@intel.com>
> Signed-off-by: Desmond Cheong Zhi Xi <desmondcheongzx@gmail.com>

Just realized another benefit of pushing the dev->master_rwsem write
locks down into ioctls that need them: We wouldn't need this function here
exported for use in drm. But also I'm not sure that works any better than
the design in your current patch set ...
-Daniel

> ---
>  kernel/task_work.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/kernel/task_work.c b/kernel/task_work.c
> index 1698fbe6f0e1..90000404af2b 100644
> --- a/kernel/task_work.c
> +++ b/kernel/task_work.c
> @@ -60,6 +60,7 @@ int task_work_add(struct task_struct *task, struct callback_head *work,
>  
>  	return 0;
>  }
> +EXPORT_SYMBOL(task_work_add);
>  
>  /**
>   * task_work_cancel_match - cancel a pending work added by task_work_add()
> -- 
> 2.25.1
>
Desmond Cheong Zhi Xi Aug. 19, 2021, 9:40 a.m. UTC | #2
On 19/8/21 5:26 pm, Christoph Hellwig wrote:
> On Wed, Aug 18, 2021 at 03:38:23PM +0800, Desmond Cheong Zhi Xi wrote:
>> +EXPORT_SYMBOL(task_work_add);
> 
> EXPORT_SYMBOL_GPL for this kinds of functionality, please.
> 

Thanks, I wasn't aware of the GPL-only export. I'll update this in a 
future series if we still need the export.
diff mbox series

Patch

diff --git a/kernel/task_work.c b/kernel/task_work.c
index 1698fbe6f0e1..90000404af2b 100644
--- a/kernel/task_work.c
+++ b/kernel/task_work.c
@@ -60,6 +60,7 @@  int task_work_add(struct task_struct *task, struct callback_head *work,
 
 	return 0;
 }
+EXPORT_SYMBOL(task_work_add);
 
 /**
  * task_work_cancel_match - cancel a pending work added by task_work_add()