All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net 1/4] umh: add exit routine for UMH process
@ 2018-12-30 16:31 Taehee Yoo
  2019-01-05 22:10 ` David Miller
  0 siblings, 1 reply; 5+ messages in thread
From: Taehee Yoo @ 2018-12-30 16:31 UTC (permalink / raw)
  To: davem, netdev, linux-kernel, daniel, ast, mcgrof; +Cc: ap420073

A UMH process which is created by the fork_usermode_blob() such as
bpfilter needs to release members of the umh_info when process is
terminated.
But the do_exit() does not release members of the umh_info. hence module
which uses UMH needs own code to detect whether UMH process is
terminated or not.
But this implementation needs extra code for checking the status of
UMH process. it eventually makes the code more complex.

The exit_umh() does not release members of the umh_info.
Hence umh_info->cleanup callback should release both members of the
umh_info and the private data.

Suggested-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Taehee Yoo <ap420073@gmail.com>
---
 include/linux/umh.h |  4 ++++
 kernel/exit.c       |  1 +
 kernel/umh.c        | 27 +++++++++++++++++++++++++++
 3 files changed, 32 insertions(+)

diff --git a/include/linux/umh.h b/include/linux/umh.h
index 235f51b62c71..c645f0a19103 100644
--- a/include/linux/umh.h
+++ b/include/linux/umh.h
@@ -47,6 +47,8 @@ struct umh_info {
 	const char *cmdline;
 	struct file *pipe_to_umh;
 	struct file *pipe_from_umh;
+	struct list_head list;
+	void (*cleanup)(struct umh_info *info);
 	pid_t pid;
 };
 int fork_usermode_blob(void *data, size_t len, struct umh_info *info);
@@ -75,6 +77,8 @@ static inline void usermodehelper_enable(void)
 	__usermodehelper_set_disable_depth(UMH_ENABLED);
 }
 
+void exit_umh(struct task_struct *tsk);
+
 extern int usermodehelper_read_trylock(void);
 extern long usermodehelper_read_lock_wait(long timeout);
 extern void usermodehelper_read_unlock(void);
diff --git a/kernel/exit.c b/kernel/exit.c
index 0e21e6d21f35..63ce4c958390 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -866,6 +866,7 @@ void __noreturn do_exit(long code)
 	exit_task_namespaces(tsk);
 	exit_task_work(tsk);
 	exit_thread(tsk);
+	exit_umh(tsk);
 
 	/*
 	 * Flush inherited counters to the parent - before the parent
diff --git a/kernel/umh.c b/kernel/umh.c
index 0baa672e023c..9b2238e440eb 100644
--- a/kernel/umh.c
+++ b/kernel/umh.c
@@ -37,6 +37,8 @@ static kernel_cap_t usermodehelper_bset = CAP_FULL_SET;
 static kernel_cap_t usermodehelper_inheritable = CAP_FULL_SET;
 static DEFINE_SPINLOCK(umh_sysctl_lock);
 static DECLARE_RWSEM(umhelper_sem);
+static LIST_HEAD(umh_list);
+static DEFINE_MUTEX(umh_list_lock);
 
 static void call_usermodehelper_freeinfo(struct subprocess_info *info)
 {
@@ -517,6 +519,11 @@ int fork_usermode_blob(void *data, size_t len, struct umh_info *info)
 		goto out;
 
 	err = call_usermodehelper_exec(sub_info, UMH_WAIT_EXEC);
+	if (!err) {
+		mutex_lock(&umh_list_lock);
+		list_add(&info->list, &umh_list);
+		mutex_unlock(&umh_list_lock);
+	}
 out:
 	fput(file);
 	return err;
@@ -679,6 +686,26 @@ static int proc_cap_handler(struct ctl_table *table, int write,
 	return 0;
 }
 
+void exit_umh(struct task_struct *tsk)
+{
+	struct umh_info *info;
+	pid_t pid = tsk->pid;
+
+	mutex_lock(&umh_list_lock);
+	list_for_each_entry(info, &umh_list, list) {
+		if (info->pid == pid) {
+			list_del(&info->list);
+			mutex_unlock(&umh_list_lock);
+			goto out;
+		}
+	}
+	mutex_unlock(&umh_list_lock);
+	return;
+out:
+	if (info->cleanup)
+		info->cleanup(info);
+}
+
 struct ctl_table usermodehelper_table[] = {
 	{
 		.procname	= "bset",
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH net 1/4] umh: add exit routine for UMH process
  2018-12-30 16:31 [PATCH net 1/4] umh: add exit routine for UMH process Taehee Yoo
@ 2019-01-05 22:10 ` David Miller
  2019-01-06  5:34   ` Taehee Yoo
  0 siblings, 1 reply; 5+ messages in thread
From: David Miller @ 2019-01-05 22:10 UTC (permalink / raw)
  To: ap420073; +Cc: netdev, linux-kernel, daniel, ast, mcgrof

From: Taehee Yoo <ap420073@gmail.com>
Date: Mon, 31 Dec 2018 01:31:43 +0900

> +void exit_umh(struct task_struct *tsk)
> +{
> +	struct umh_info *info;
> +	pid_t pid = tsk->pid;
> +
> +	mutex_lock(&umh_list_lock);
> +	list_for_each_entry(info, &umh_list, list) {

So this is probably too expensive of a cost for every process exit.
The problem is that the cost will be taken even if the process is
not a UMH.

I've taken my time to respond in hopes that I could come up with a
good alternative to suggest, but so far I don't have any better ideas.

I'll keep thinking about this some more, please let me know if you
have any ideas.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net 1/4] umh: add exit routine for UMH process
  2019-01-05 22:10 ` David Miller
@ 2019-01-06  5:34   ` Taehee Yoo
  2019-01-06 16:55     ` David Miller
  0 siblings, 1 reply; 5+ messages in thread
From: Taehee Yoo @ 2019-01-06  5:34 UTC (permalink / raw)
  To: David Miller; +Cc: Netdev, linux-kernel, Daniel Borkmann, ast, mcgrof

On Sun, 6 Jan 2019 at 07:10, David Miller <davem@davemloft.net> wrote:
>
> From: Taehee Yoo <ap420073@gmail.com>
> Date: Mon, 31 Dec 2018 01:31:43 +0900
>
> > +void exit_umh(struct task_struct *tsk)
> > +{
> > +     struct umh_info *info;
> > +     pid_t pid = tsk->pid;
> > +
> > +     mutex_lock(&umh_list_lock);
> > +     list_for_each_entry(info, &umh_list, list) {
>

Thank you for review!

> So this is probably too expensive of a cost for every process exit.
> The problem is that the cost will be taken even if the process is
> not a UMH.
>

Yes, I agree with you.

> I've taken my time to respond in hopes that I could come up with a
> good alternative to suggest, but so far I don't have any better ideas.
>
> I'll keep thinking about this some more, please let me know if you
> have any ideas.

Thanks a lot for spending time to think about better ideas!
How about adding a new PF_UMH flag for task_struct->flags to identify
UMH process?
By using this flag, the exit_umh() can avoid unnecessary lookups.

Thanks again.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net 1/4] umh: add exit routine for UMH process
  2019-01-06  5:34   ` Taehee Yoo
@ 2019-01-06 16:55     ` David Miller
  2019-01-07  3:44       ` Taehee Yoo
  0 siblings, 1 reply; 5+ messages in thread
From: David Miller @ 2019-01-06 16:55 UTC (permalink / raw)
  To: ap420073; +Cc: netdev, linux-kernel, daniel, ast, mcgrof

From: Taehee Yoo <ap420073@gmail.com>
Date: Sun, 6 Jan 2019 14:34:52 +0900

> How about adding a new PF_UMH flag for task_struct->flags to identify
> UMH process?
> By using this flag, the exit_umh() can avoid unnecessary lookups.

Yes, that might be more efficient and eliminate the high cost for
non-UMH tasks.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net 1/4] umh: add exit routine for UMH process
  2019-01-06 16:55     ` David Miller
@ 2019-01-07  3:44       ` Taehee Yoo
  0 siblings, 0 replies; 5+ messages in thread
From: Taehee Yoo @ 2019-01-07  3:44 UTC (permalink / raw)
  To: David Miller; +Cc: Netdev, linux-kernel, Daniel Borkmann, ast, mcgrof

On Mon, 7 Jan 2019 at 01:55, David Miller <davem@davemloft.net> wrote:
>
> From: Taehee Yoo <ap420073@gmail.com>
> Date: Sun, 6 Jan 2019 14:34:52 +0900
>
> > How about adding a new PF_UMH flag for task_struct->flags to identify
> > UMH process?
> > By using this flag, the exit_umh() can avoid unnecessary lookups.
>
> Yes, that might be more efficient and eliminate the high cost for
> non-UMH tasks.

I will send a v3 patch.
Thank you!

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2019-01-07  3:44 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-30 16:31 [PATCH net 1/4] umh: add exit routine for UMH process Taehee Yoo
2019-01-05 22:10 ` David Miller
2019-01-06  5:34   ` Taehee Yoo
2019-01-06 16:55     ` David Miller
2019-01-07  3:44       ` Taehee Yoo

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.