Messages in this thread | | | From | (Eric W. Biederman) | Date | Tue, 18 Sep 2012 21:46:35 -0700 | Subject | Re: [PATCH] poweroff: fix bug in orderly_poweroff |
| |
hongfeng <hongfeng@marvell.com> writes:
> orderly_poweroff is trying to poweroff platform by two steps: > step 1: Call userspace application to poweroff > step 2: If userspace poweroff fail, then do a force power off if force param is set. > > The bug here is, step 1 is always successful with param UMH_NO_WAIT,
This code has existed for 5 years. Is this a recent regression? Why has no one complained before?
It looks to me that step 2 is: step 2: If we can not launch the userspace poweroff fail.
> should change to UMH_WAIT_PROC which will monitor the return value > ofuserspace application.
Is it safe to block indefinitely in the callers waiting for userspace?
If the caller is not running in a kernel thread then we can easily get into a case where the userspace caller will block waiting for us when we are waiting for the userspace caller.
I don't want to impeded progress but I don't see the evidence that this change is good enough.
> Change-Id: I2f9ebbb90c0c2443780080ec9507c8d004e5da74 > Signed-off-by: Feng Hong <hongfeng@marvell.com> > --- > kernel/sys.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/kernel/sys.c b/kernel/sys.c > index 241507f..1b30b30 100644 > --- a/kernel/sys.c > +++ b/kernel/sys.c > @@ -2204,7 +2204,7 @@ static int __orderly_poweroff(void) > return -ENOMEM; > } > > - ret = call_usermodehelper_fns(argv[0], argv, envp, UMH_NO_WAIT, > + ret = call_usermodehelper_fns(argv[0], argv, envp, UMH_WAIT_PROC, > NULL, argv_cleanup, NULL); > if (ret == -ENOMEM) > argv_free(argv);
| |