| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <1245745420.3477.16.camel@wing-commander>
Date: Tue, 23 Jun 2009 09:23:40 +0100
From: Scott James Remnant <scott@...ntu.com>
To: Andrew Morton <akpm@...ux-foundation.org>
Cc: linux-kernel@...r.kernel.org, Matt Helsley <matthltc@...ibm.com>
Subject: Re: [PATCH] proc connector: add event for process becoming session
leader
On Mon, 2009-06-22 at 16:19 -0700, Andrew Morton wrote:
> Let's cc the Process Events developer..
>
Ah great, thanks - I couldn't find an e-mail address.
> On Mon, 15 Jun 2009 13:03:08 +0100
> Scott James Remnant <scott@...ntu.com> wrote:
>
> > The act of a process becoming a session leader is a useful signal to a
> > supervising init daemon such as Upstart.
> >
> > While a daemon will normally do this as part of the process of becoming
> > a daemon, it is rare for its children to do so. When the children do,
> > it is nearly always a sign that the child should be considered detached
> > from the parent and not supervised along with it.
> >
> > The poster-child example is OpenSSH; the per-login children call setsid()
> > so that they may control the pty connected to them. If the primary daemon
> > dies or is restarted, we do not want to consider the per-login children
> > and want to respawn the primary daemon without killing the children.
> >
> > This patch adds a new PROC_SID_EVENT and associated structure to the
> > proc_event event_data union, it arranges for this to be emitted when
> > the special PIDTYPE_SID pid is set.
> >
>
> hm, well, I don't have much useful to say about the overall idea, but
> it seems to slot into the existing code simply enough.
>
> > ---
> > drivers/connector/cn_proc.c | 25 +++++++++++++++++++++++++
> > include/linux/cn_proc.h | 10 ++++++++++
> > kernel/exit.c | 4 +++-
> > 3 files changed, 38 insertions(+), 1 deletions(-)
>
> We seem to have forgotten to document this entire interface, so I can't
> ding you for forgetting to update the forgotten documentation.
>
> > diff --git a/drivers/connector/cn_proc.c b/drivers/connector/cn_proc.c
> > index c5afc98..7d48cd9 100644
> > --- a/drivers/connector/cn_proc.c
> > +++ b/drivers/connector/cn_proc.c
> > @@ -139,6 +139,31 @@ void proc_id_connector(struct task_struct *task, int which_id)
> > cn_netlink_send(msg, CN_IDX_PROC, GFP_KERNEL);
> > }
> >
> > +void proc_sid_connector(struct task_struct *task)
>
> It would be nice to have a nice comment explaining what this function
> does. Ditto all the others in there, really.
>
> > +{
> > + struct cn_msg *msg;
> > + struct proc_event *ev;
> > + struct timespec ts;
> > + __u8 buffer[CN_PROC_MSG_SIZE];
> > +
> > + if (atomic_read(&proc_event_num_listeners) < 1)
> > + return;
> > +
> > + msg = (struct cn_msg*)buffer;
> > + ev = (struct proc_event*)msg->data;
>
> Please pass all patches through scripts/checkpatch.pl.
>
> > + get_seq(&msg->seq, &ev->cpu);
> > + ktime_get_ts(&ts); /* get high res monotonic timestamp */
> > + put_unaligned(timespec_to_ns(&ts), (__u64 *)&ev->timestamp_ns);
> > + ev->what = PROC_EVENT_SID;
> > + ev->event_data.sid.process_pid = task->pid;
>
> This is a bit of a worry. In a containerised environment, pids are not
> unique. Now what do we do?
>
I must admit, that I just copy and pasted the existing functions - bugs
and all :-)
> > + ev->event_data.sid.process_tgid = task->tgid;
> > +
> > + memcpy(&msg->id, &cn_proc_event_id, sizeof(msg->id));
> > + msg->ack = 0; /* not used */
> > + msg->len = sizeof(*ev);
> > + cn_netlink_send(msg, CN_IDX_PROC, GFP_KERNEL);
> > +}
> > +
> > void proc_exit_connector(struct task_struct *task)
> > {
> > struct cn_msg *msg;
> > diff --git a/include/linux/cn_proc.h b/include/linux/cn_proc.h
> > index b8125b2..47dac5e 100644
> > --- a/include/linux/cn_proc.h
> > +++ b/include/linux/cn_proc.h
> > @@ -52,6 +52,7 @@ struct proc_event {
> > PROC_EVENT_EXEC = 0x00000002,
> > PROC_EVENT_UID = 0x00000004,
> > PROC_EVENT_GID = 0x00000040,
> > + PROC_EVENT_SID = 0x00000080,
> > /* "next" should be 0x00000400 */
> > /* "last" is the last process event: exit */
> > PROC_EVENT_EXIT = 0x80000000
> > @@ -89,6 +90,11 @@ struct proc_event {
> > } e;
> > } id;
> >
> > + struct sid_proc_event {
> > + __kernel_pid_t process_pid;
> > + __kernel_pid_t process_tgid;
> > + } sid;
> > +
> > struct exit_proc_event {
> > __kernel_pid_t process_pid;
> > __kernel_pid_t process_tgid;
> > @@ -102,6 +108,7 @@ struct proc_event {
> > void proc_fork_connector(struct task_struct *task);
> > void proc_exec_connector(struct task_struct *task);
> > void proc_id_connector(struct task_struct *task, int which_id);
> > +void proc_sid_connector(struct task_struct *task);
> > void proc_exit_connector(struct task_struct *task);
> > #else
> > static inline void proc_fork_connector(struct task_struct *task)
> > @@ -114,6 +121,9 @@ static inline void proc_id_connector(struct task_struct *task,
> > int which_id)
> > {}
> >
> > +static inline void proc_sid_connector(struct task_struct *task)
> > +{}
> > +
> > static inline void proc_exit_connector(struct task_struct *task)
> > {}
> > #endif /* CONFIG_PROC_EVENTS */
> > diff --git a/kernel/exit.c b/kernel/exit.c
> > index b6c90b5..5162589 100644
> > --- a/kernel/exit.c
> > +++ b/kernel/exit.c
> > @@ -360,8 +360,10 @@ void __set_special_pids(struct pid *pid)
> > {
> > struct task_struct *curr = current->group_leader;
> >
> > - if (task_session(curr) != pid)
> > + if (task_session(curr) != pid) {
> > change_pid(curr, PIDTYPE_SID, pid);
> > + proc_sid_connector(curr);
> > + }
> >
> > if (task_pgrp(curr) != pid)
> > change_pid(curr, PIDTYPE_PGID, pid);
>
Scott
--
Scott James Remnant
scott@...ntu.com
Download attachment "signature.asc" of type "application/pgp-signature" (198 bytes)
Powered by blists - more mailing lists