Re: [PATCH] kernel/signal: Signal-based pre-coredump notification

2018-10-22 Thread Enke Chen
Jann, Thanks for the feedback. I will post a revised patch shortly. On the related topic of "pdeath_signal", there are several inconsistencies by preserving the flag across execve(2). The flag is cleared under several conditions in different places. I will start a separate thread to see if it

Re: [PATCH] kernel/signal: Signal-based pre-coredump notification

2018-10-22 Thread Enke Chen
Jann, Thanks for the feedback. I will post a revised patch shortly. On the related topic of "pdeath_signal", there are several inconsistencies by preserving the flag across execve(2). The flag is cleared under several conditions in different places. I will start a separate thread to see if it

Re: [PATCH] kernel/signal: Signal-based pre-coredump notification

2018-10-22 Thread Jann Horn
On Sat, Oct 20, 2018 at 1:01 AM Enke Chen wrote: > Regarding the security considerations, it seems simpler and more secure to > just clear the "pre-coredump signal" cross execve(2), and let the new program > decide for itself. What do you think? I don't have a problem with these semantics. I

Re: [PATCH] kernel/signal: Signal-based pre-coredump notification

2018-10-22 Thread Jann Horn
On Sat, Oct 20, 2018 at 1:01 AM Enke Chen wrote: > Regarding the security considerations, it seems simpler and more secure to > just clear the "pre-coredump signal" cross execve(2), and let the new program > decide for itself. What do you think? I don't have a problem with these semantics. I

Re: [PATCH] kernel/signal: Signal-based pre-coredump notification

2018-10-19 Thread Enke Chen
Hi, Jann: Regarding the security considerations, it seems simpler and more secure to just clear the "pre-coredump signal" cross execve(2), and let the new program decide for itself. What do you think? --- Changes to prctl(2): DESCRIPTION PR_SET_PREDUMP_SIG (since Linux 4.20.x)

Re: [PATCH] kernel/signal: Signal-based pre-coredump notification

2018-10-19 Thread Enke Chen
Hi, Jann: Regarding the security considerations, it seems simpler and more secure to just clear the "pre-coredump signal" cross execve(2), and let the new program decide for itself. What do you think? --- Changes to prctl(2): DESCRIPTION PR_SET_PREDUMP_SIG (since Linux 4.20.x)

Re: [PATCH] kernel/signal: Signal-based pre-coredump notification

2018-10-16 Thread Enke Chen
Hi, Oleg: On 10/16/18 7:14 AM, Oleg Nesterov wrote: > On 10/15, Enke Chen wrote: >> >>> I don't understand why we need valid_predump_signal() at all. >> >> Most of the signals have well-defined semantics, and would not be appropriate >> for this purpose. > > you are going to change the rules

Re: [PATCH] kernel/signal: Signal-based pre-coredump notification

2018-10-16 Thread Enke Chen
Hi, Oleg: On 10/16/18 7:14 AM, Oleg Nesterov wrote: > On 10/15, Enke Chen wrote: >> >>> I don't understand why we need valid_predump_signal() at all. >> >> Most of the signals have well-defined semantics, and would not be appropriate >> for this purpose. > > you are going to change the rules

Re: [PATCH] kernel/signal: Signal-based pre-coredump notification

2018-10-16 Thread Eric W. Biederman
Enke Chen writes: > Hi, Eric: > > On 10/15/18 4:28 PM, Eric W. Biederman wrote: >> With that said I think the best solution would be to figure out how to >> allow the coredump to run in parallel with the usual exit signal, and >> exit code reaping of the process> >> That would solve the

Re: [PATCH] kernel/signal: Signal-based pre-coredump notification

2018-10-16 Thread Eric W. Biederman
Enke Chen writes: > Hi, Eric: > > On 10/15/18 4:28 PM, Eric W. Biederman wrote: >> With that said I think the best solution would be to figure out how to >> allow the coredump to run in parallel with the usual exit signal, and >> exit code reaping of the process> >> That would solve the

Re: [PATCH] kernel/signal: Signal-based pre-coredump notification

2018-10-16 Thread Eric W. Biederman
Oleg Nesterov writes: > On 10/15, Enke Chen wrote: >> >> > I don't understand why we need valid_predump_signal() at all. >> >> Most of the signals have well-defined semantics, and would not be appropriate >> for this purpose. > > you are going to change the rules anyway. I will just add that

Re: [PATCH] kernel/signal: Signal-based pre-coredump notification

2018-10-16 Thread Eric W. Biederman
Oleg Nesterov writes: > On 10/15, Enke Chen wrote: >> >> > I don't understand why we need valid_predump_signal() at all. >> >> Most of the signals have well-defined semantics, and would not be appropriate >> for this purpose. > > you are going to change the rules anyway. I will just add that

Re: [PATCH] kernel/signal: Signal-based pre-coredump notification

2018-10-16 Thread Oleg Nesterov
On 10/15, Enke Chen wrote: > > > I don't understand why we need valid_predump_signal() at all. > > Most of the signals have well-defined semantics, and would not be appropriate > for this purpose. you are going to change the rules anyway. > That is why it is limited to only SIGCHLD, SIGUSR1,

Re: [PATCH] kernel/signal: Signal-based pre-coredump notification

2018-10-16 Thread Oleg Nesterov
On 10/15, Enke Chen wrote: > > > I don't understand why we need valid_predump_signal() at all. > > Most of the signals have well-defined semantics, and would not be appropriate > for this purpose. you are going to change the rules anyway. > That is why it is limited to only SIGCHLD, SIGUSR1,

Re: [PATCH] kernel/signal: Signal-based pre-coredump notification

2018-10-15 Thread Enke Chen
Hi, Eric: On 10/15/18 4:28 PM, Eric W. Biederman wrote: > Enke Chen writes: > >> For simplicity and consistency, this patch provides an implementation >> for signal-based fault notification prior to the coredump of a child >> process. A new prctl command, PR_SET_PREDUMP_SIG, is defined that can

Re: [PATCH] kernel/signal: Signal-based pre-coredump notification

2018-10-15 Thread Enke Chen
Hi, Eric: On 10/15/18 4:28 PM, Eric W. Biederman wrote: > Enke Chen writes: > >> For simplicity and consistency, this patch provides an implementation >> for signal-based fault notification prior to the coredump of a child >> process. A new prctl command, PR_SET_PREDUMP_SIG, is defined that can

Re: [PATCH] kernel/signal: Signal-based pre-coredump notification

2018-10-15 Thread valdis . kletnieks
On Mon, 15 Oct 2018 18:28:03 -0500, Eric W. Biederman said: > Enke Chen writes: > > > For simplicity and consistency, this patch provides an implementation > > for signal-based fault notification prior to the coredump of a child > > process. A new prctl command, PR_SET_PREDUMP_SIG, is defined

Re: [PATCH] kernel/signal: Signal-based pre-coredump notification

2018-10-15 Thread valdis . kletnieks
On Mon, 15 Oct 2018 18:28:03 -0500, Eric W. Biederman said: > Enke Chen writes: > > > For simplicity and consistency, this patch provides an implementation > > for signal-based fault notification prior to the coredump of a child > > process. A new prctl command, PR_SET_PREDUMP_SIG, is defined

Re: [PATCH] kernel/signal: Signal-based pre-coredump notification

2018-10-15 Thread Eric W. Biederman
Enke Chen writes: > For simplicity and consistency, this patch provides an implementation > for signal-based fault notification prior to the coredump of a child > process. A new prctl command, PR_SET_PREDUMP_SIG, is defined that can > be used by an application to express its interest and to

Re: [PATCH] kernel/signal: Signal-based pre-coredump notification

2018-10-15 Thread Eric W. Biederman
Enke Chen writes: > For simplicity and consistency, this patch provides an implementation > for signal-based fault notification prior to the coredump of a child > process. A new prctl command, PR_SET_PREDUMP_SIG, is defined that can > be used by an application to express its interest and to

Re: [PATCH] kernel/signal: Signal-based pre-coredump notification

2018-10-15 Thread Enke Chen
Hi, Alan: As I replied earlier, I will remove the logic that allows setting on others. This function "set_predump_signal_perm()" will be gone too. Thanks. -- Enke On 10/15/18 2:21 PM, Alan Cox wrote: >> +/* >> + * Returns true if current's euid is same as p's uid or euid, >> + * or has

Re: [PATCH] kernel/signal: Signal-based pre-coredump notification

2018-10-15 Thread Enke Chen
Hi, Alan: As I replied earlier, I will remove the logic that allows setting on others. This function "set_predump_signal_perm()" will be gone too. Thanks. -- Enke On 10/15/18 2:21 PM, Alan Cox wrote: >> +/* >> + * Returns true if current's euid is same as p's uid or euid, >> + * or has

Re: [PATCH] kernel/signal: Signal-based pre-coredump notification

2018-10-15 Thread Alan Cox
> +/* > + * Returns true if current's euid is same as p's uid or euid, > + * or has CAP_SYS_ADMIN. > + * > + * Called with rcu_read_lock, creds are safe. > + * > + * Adapted from set_one_prio_perm(). > + */ > +static bool set_predump_signal_perm(struct task_struct *p) > +{ > + const struct

Re: [PATCH] kernel/signal: Signal-based pre-coredump notification

2018-10-15 Thread Alan Cox
> +/* > + * Returns true if current's euid is same as p's uid or euid, > + * or has CAP_SYS_ADMIN. > + * > + * Called with rcu_read_lock, creds are safe. > + * > + * Adapted from set_one_prio_perm(). > + */ > +static bool set_predump_signal_perm(struct task_struct *p) > +{ > + const struct

Re: [PATCH] kernel/signal: Signal-based pre-coredump notification

2018-10-15 Thread Enke Chen
Hi, Olge: >> probably ->predump_signal should be cleared on exec? As I replied to Jann, will do. Thanks. -- Enke On 10/15/18 12:17 PM, Enke Chen wrote: > Hi, Oleg: > > I missed some of your comments in my previous reply. > > On 10/15/18 5:05 AM, Oleg Nesterov wrote: >> On 10/12, Enke Chen

Re: [PATCH] kernel/signal: Signal-based pre-coredump notification

2018-10-15 Thread Enke Chen
Hi, Olge: >> probably ->predump_signal should be cleared on exec? As I replied to Jann, will do. Thanks. -- Enke On 10/15/18 12:17 PM, Enke Chen wrote: > Hi, Oleg: > > I missed some of your comments in my previous reply. > > On 10/15/18 5:05 AM, Oleg Nesterov wrote: >> On 10/12, Enke Chen

Re: [PATCH] kernel/signal: Signal-based pre-coredump notification

2018-10-15 Thread Enke Chen
Hi, Jann: Thanks for your detail explanation. Will take care of it. -- Enke On 10/15/18 11:54 AM, Jann Horn wrote: > On Mon, Oct 15, 2018 at 8:36 PM Enke Chen wrote: >> On 10/13/18 11:27 AM, Jann Horn wrote: >>> On Sat, Oct 13, 2018 at 2:33 AM Enke Chen wrote: For simplicity and

Re: [PATCH] kernel/signal: Signal-based pre-coredump notification

2018-10-15 Thread Enke Chen
Hi, Jann: Thanks for your detail explanation. Will take care of it. -- Enke On 10/15/18 11:54 AM, Jann Horn wrote: > On Mon, Oct 15, 2018 at 8:36 PM Enke Chen wrote: >> On 10/13/18 11:27 AM, Jann Horn wrote: >>> On Sat, Oct 13, 2018 at 2:33 AM Enke Chen wrote: For simplicity and

Re: [PATCH] kernel/signal: Signal-based pre-coredump notification

2018-10-15 Thread Enke Chen
Hi, Oleg: I missed some of your comments in my previous reply. On 10/15/18 5:05 AM, Oleg Nesterov wrote: > On 10/12, Enke Chen wrote: >> >> For simplicity and consistency, this patch provides an implementation >> for signal-based fault notification prior to the coredump of a child >> process. A

Re: [PATCH] kernel/signal: Signal-based pre-coredump notification

2018-10-15 Thread Enke Chen
Hi, Oleg: I missed some of your comments in my previous reply. On 10/15/18 5:05 AM, Oleg Nesterov wrote: > On 10/12, Enke Chen wrote: >> >> For simplicity and consistency, this patch provides an implementation >> for signal-based fault notification prior to the coredump of a child >> process. A

Re: [PATCH] kernel/signal: Signal-based pre-coredump notification

2018-10-15 Thread Enke Chen
Hi, Oleg: On 10/15/18 5:05 AM, Oleg Nesterov wrote: > On 10/12, Enke Chen wrote: >> >> For simplicity and consistency, this patch provides an implementation >> for signal-based fault notification prior to the coredump of a child >> process. A new prctl command, PR_SET_PREDUMP_SIG, is defined that

Re: [PATCH] kernel/signal: Signal-based pre-coredump notification

2018-10-15 Thread Enke Chen
Hi, Oleg: On 10/15/18 5:05 AM, Oleg Nesterov wrote: > On 10/12, Enke Chen wrote: >> >> For simplicity and consistency, this patch provides an implementation >> for signal-based fault notification prior to the coredump of a child >> process. A new prctl command, PR_SET_PREDUMP_SIG, is defined that

Re: [PATCH] kernel/signal: Signal-based pre-coredump notification

2018-10-15 Thread Greg Kroah-Hartman
On Mon, Oct 15, 2018 at 11:49:11AM -0700, Enke Chen wrote: > Hi, Greg: > > On 10/15/18 11:43 AM, Greg Kroah-Hartman wrote: > > On Mon, Oct 15, 2018 at 11:16:36AM -0700, Enke Chen wrote: > >> Hi, Greg: > >> > >>> Shouldn't there also be a manpage update, and a kselftest added for this > >>> new

Re: [PATCH] kernel/signal: Signal-based pre-coredump notification

2018-10-15 Thread Greg Kroah-Hartman
On Mon, Oct 15, 2018 at 11:49:11AM -0700, Enke Chen wrote: > Hi, Greg: > > On 10/15/18 11:43 AM, Greg Kroah-Hartman wrote: > > On Mon, Oct 15, 2018 at 11:16:36AM -0700, Enke Chen wrote: > >> Hi, Greg: > >> > >>> Shouldn't there also be a manpage update, and a kselftest added for this > >>> new

Re: [PATCH] kernel/signal: Signal-based pre-coredump notification

2018-10-15 Thread Jann Horn
On Mon, Oct 15, 2018 at 8:36 PM Enke Chen wrote: > On 10/13/18 11:27 AM, Jann Horn wrote: > > On Sat, Oct 13, 2018 at 2:33 AM Enke Chen wrote: > >> For simplicity and consistency, this patch provides an implementation > >> for signal-based fault notification prior to the coredump of a child > >>

Re: [PATCH] kernel/signal: Signal-based pre-coredump notification

2018-10-15 Thread Jann Horn
On Mon, Oct 15, 2018 at 8:36 PM Enke Chen wrote: > On 10/13/18 11:27 AM, Jann Horn wrote: > > On Sat, Oct 13, 2018 at 2:33 AM Enke Chen wrote: > >> For simplicity and consistency, this patch provides an implementation > >> for signal-based fault notification prior to the coredump of a child > >>

Re: [PATCH] kernel/signal: Signal-based pre-coredump notification

2018-10-15 Thread Enke Chen
Hi, Greg: On 10/15/18 11:43 AM, Greg Kroah-Hartman wrote: > On Mon, Oct 15, 2018 at 11:16:36AM -0700, Enke Chen wrote: >> Hi, Greg: >> >>> Shouldn't there also be a manpage update, and a kselftest added for this >>> new user/kernel api that is being created? >>> >> >> I will submit a patch for

Re: [PATCH] kernel/signal: Signal-based pre-coredump notification

2018-10-15 Thread Enke Chen
Hi, Greg: On 10/15/18 11:43 AM, Greg Kroah-Hartman wrote: > On Mon, Oct 15, 2018 at 11:16:36AM -0700, Enke Chen wrote: >> Hi, Greg: >> >>> Shouldn't there also be a manpage update, and a kselftest added for this >>> new user/kernel api that is being created? >>> >> >> I will submit a patch for

Re: [PATCH] kernel/signal: Signal-based pre-coredump notification

2018-10-15 Thread Enke Chen
Hi, Jann: Thanks a lot for you detailed review. Please see my replied/comments inline. On 10/13/18 11:27 AM, Jann Horn wrote: > On Sat, Oct 13, 2018 at 2:33 AM Enke Chen wrote: >> For simplicity and consistency, this patch provides an implementation >> for signal-based fault notification prior

Re: [PATCH] kernel/signal: Signal-based pre-coredump notification

2018-10-15 Thread Enke Chen
Hi, Jann: Thanks a lot for you detailed review. Please see my replied/comments inline. On 10/13/18 11:27 AM, Jann Horn wrote: > On Sat, Oct 13, 2018 at 2:33 AM Enke Chen wrote: >> For simplicity and consistency, this patch provides an implementation >> for signal-based fault notification prior

Re: [PATCH] kernel/signal: Signal-based pre-coredump notification

2018-10-15 Thread Greg Kroah-Hartman
On Mon, Oct 15, 2018 at 11:16:36AM -0700, Enke Chen wrote: > Hi, Greg: > > > Shouldn't there also be a manpage update, and a kselftest added for this > > new user/kernel api that is being created? > > > > I will submit a patch for manpage update once the code is accepted. Writing a manpage

Re: [PATCH] kernel/signal: Signal-based pre-coredump notification

2018-10-15 Thread Greg Kroah-Hartman
On Mon, Oct 15, 2018 at 11:16:36AM -0700, Enke Chen wrote: > Hi, Greg: > > > Shouldn't there also be a manpage update, and a kselftest added for this > > new user/kernel api that is being created? > > > > I will submit a patch for manpage update once the code is accepted. Writing a manpage

Re: [PATCH] kernel/signal: Signal-based pre-coredump notification

2018-10-15 Thread Enke Chen
Hi, Christian: As I replied to Jann, I will remove the code that does the setting on others to make the code simpler and more secure. Thanks. -- Enke >> +static bool set_predump_signal_perm(struct task_struct *p) >> +{ >> +const struct cred *cred = current_cred(), *pcred = __task_cred(p);

Re: [PATCH] kernel/signal: Signal-based pre-coredump notification

2018-10-15 Thread Enke Chen
Hi, Christian: As I replied to Jann, I will remove the code that does the setting on others to make the code simpler and more secure. Thanks. -- Enke >> +static bool set_predump_signal_perm(struct task_struct *p) >> +{ >> +const struct cred *cred = current_cred(), *pcred = __task_cred(p);

Re: [PATCH] kernel/signal: Signal-based pre-coredump notification

2018-10-15 Thread Enke Chen
Hi, Greg: > Shouldn't there also be a manpage update, and a kselftest added for this > new user/kernel api that is being created? > I will submit a patch for manpage update once the code is accepted. Regarding the kselftest, I am not sure. Once the prctl() is limited to self (which I will

Re: [PATCH] kernel/signal: Signal-based pre-coredump notification

2018-10-15 Thread Enke Chen
Hi, Greg: > Shouldn't there also be a manpage update, and a kselftest added for this > new user/kernel api that is being created? > I will submit a patch for manpage update once the code is accepted. Regarding the kselftest, I am not sure. Once the prctl() is limited to self (which I will

Re: [PATCH] kernel/signal: Signal-based pre-coredump notification

2018-10-15 Thread Oleg Nesterov
On 10/12, Enke Chen wrote: > > For simplicity and consistency, this patch provides an implementation > for signal-based fault notification prior to the coredump of a child > process. A new prctl command, PR_SET_PREDUMP_SIG, is defined that can > be used by an application to express its interest

Re: [PATCH] kernel/signal: Signal-based pre-coredump notification

2018-10-15 Thread Oleg Nesterov
On 10/12, Enke Chen wrote: > > For simplicity and consistency, this patch provides an implementation > for signal-based fault notification prior to the coredump of a child > process. A new prctl command, PR_SET_PREDUMP_SIG, is defined that can > be used by an application to express its interest

Re: [PATCH] kernel/signal: Signal-based pre-coredump notification

2018-10-13 Thread Jann Horn
On Sat, Oct 13, 2018 at 2:33 AM Enke Chen wrote: > For simplicity and consistency, this patch provides an implementation > for signal-based fault notification prior to the coredump of a child > process. A new prctl command, PR_SET_PREDUMP_SIG, is defined that can > be used by an application to

Re: [PATCH] kernel/signal: Signal-based pre-coredump notification

2018-10-13 Thread Jann Horn
On Sat, Oct 13, 2018 at 2:33 AM Enke Chen wrote: > For simplicity and consistency, this patch provides an implementation > for signal-based fault notification prior to the coredump of a child > process. A new prctl command, PR_SET_PREDUMP_SIG, is defined that can > be used by an application to

Re: [PATCH] kernel/signal: Signal-based pre-coredump notification

2018-10-13 Thread Christian Brauner
On Fri, Oct 12, 2018 at 05:33:35PM -0700, Enke Chen wrote: > For simplicity and consistency, this patch provides an implementation > for signal-based fault notification prior to the coredump of a child > process. A new prctl command, PR_SET_PREDUMP_SIG, is defined that can > be used by an

Re: [PATCH] kernel/signal: Signal-based pre-coredump notification

2018-10-13 Thread Christian Brauner
On Fri, Oct 12, 2018 at 05:33:35PM -0700, Enke Chen wrote: > For simplicity and consistency, this patch provides an implementation > for signal-based fault notification prior to the coredump of a child > process. A new prctl command, PR_SET_PREDUMP_SIG, is defined that can > be used by an

Re: [PATCH] kernel/signal: Signal-based pre-coredump notification

2018-10-13 Thread Greg Kroah-Hartman
On Fri, Oct 12, 2018 at 05:33:35PM -0700, Enke Chen wrote: > For simplicity and consistency, this patch provides an implementation > for signal-based fault notification prior to the coredump of a child > process. A new prctl command, PR_SET_PREDUMP_SIG, is defined that can > be used by an

Re: [PATCH] kernel/signal: Signal-based pre-coredump notification

2018-10-13 Thread Greg Kroah-Hartman
On Fri, Oct 12, 2018 at 05:33:35PM -0700, Enke Chen wrote: > For simplicity and consistency, this patch provides an implementation > for signal-based fault notification prior to the coredump of a child > process. A new prctl command, PR_SET_PREDUMP_SIG, is defined that can > be used by an