Re: [ 030/143] proc connector: fix info leaks
From: Willy Tarreau Date: Mon, 12 May 2014 10:57:04 +0200 > Thank you guys, I'll check. I don't know why I didn't see them on an > allmodconfig build. Because you have to enable connector as "y", rather than "m", for the proc connector to be available in the config. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [ 030/143] proc connector: fix info leaks
On Mon, May 12, 2014 at 10:57:04AM +0200, Willy Tarreau wrote: > > > diff --git a/drivers/connector/cn_proc.c b/drivers/connector/cn_proc.c > > > index 6069790..3a2587a 100644 > > > --- a/drivers/connector/cn_proc.c > > > +++ b/drivers/connector/cn_proc.c > > > [...] > > > module_init(cn_proc_init); > > > + memset(&ev->event_data, 0, sizeof(ev->event_data)); > > > + msg->flags = 0; /* not used */ > > > > That last hunk looks bogus. Probably the source of Christoph's compile > > error. > > Thank you guys, I'll check. I don't know why I didn't see them on an > allmodconfig build. I found why: CONFIG_PROC_EVENTS is not enabled when CONNECTOR=m. Here's a fixed patch. Sorry for the trouble. Willy >From 8ab62fb31d8fb0f84686612437cfa69e32cd3626 Mon Sep 17 00:00:00 2001 From: Mathias Krause Date: Mon, 30 Sep 2013 22:03:06 +0200 Subject: proc connector: fix info leaks [ Upstream commit e727ca82e0e9616ab4844301e6bae60ca7327682 ] Initialize event_data for all possible message types to prevent leaking kernel stack contents to userland (up to 20 bytes). Also set the flags member of the connector message to 0 to prevent leaking two more stack bytes this way. Cc: sta...@vger.kernel.org # v2.6.15+ Signed-off-by: Mathias Krause Signed-off-by: David S. Miller Signed-off-by: Willy Tarreau --- drivers/connector/cn_proc.c | 13 + 1 file changed, 13 insertions(+) diff --git a/drivers/connector/cn_proc.c b/drivers/connector/cn_proc.c index 6069790..3603599 100644 --- a/drivers/connector/cn_proc.c +++ b/drivers/connector/cn_proc.c @@ -59,6 +59,7 @@ void proc_fork_connector(struct task_struct *task) msg = (struct cn_msg*)buffer; ev = (struct proc_event*)msg->data; + memset(&ev->event_data, 0, sizeof(ev->event_data)); 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); @@ -71,6 +72,7 @@ void proc_fork_connector(struct task_struct *task) memcpy(&msg->id, &cn_proc_event_id, sizeof(msg->id)); msg->ack = 0; /* not used */ msg->len = sizeof(*ev); + msg->flags = 0; /* not used */ /* If cn_netlink_send() failed, the data is not sent */ cn_netlink_send(msg, CN_IDX_PROC, GFP_KERNEL); } @@ -87,6 +89,7 @@ void proc_exec_connector(struct task_struct *task) msg = (struct cn_msg*)buffer; ev = (struct proc_event*)msg->data; + memset(&ev->event_data, 0, sizeof(ev->event_data)); 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); @@ -97,6 +100,7 @@ void proc_exec_connector(struct task_struct *task) memcpy(&msg->id, &cn_proc_event_id, sizeof(msg->id)); msg->ack = 0; /* not used */ msg->len = sizeof(*ev); + msg->flags = 0; /* not used */ cn_netlink_send(msg, CN_IDX_PROC, GFP_KERNEL); } @@ -113,6 +117,7 @@ void proc_id_connector(struct task_struct *task, int which_id) msg = (struct cn_msg*)buffer; ev = (struct proc_event*)msg->data; + memset(&ev->event_data, 0, sizeof(ev->event_data)); ev->what = which_id; ev->event_data.id.process_pid = task->pid; ev->event_data.id.process_tgid = task->tgid; @@ -136,6 +141,7 @@ void proc_id_connector(struct task_struct *task, int which_id) memcpy(&msg->id, &cn_proc_event_id, sizeof(msg->id)); msg->ack = 0; /* not used */ msg->len = sizeof(*ev); + msg->flags = 0; /* not used */ cn_netlink_send(msg, CN_IDX_PROC, GFP_KERNEL); } @@ -151,6 +157,7 @@ void proc_sid_connector(struct task_struct *task) msg = (struct cn_msg *)buffer; ev = (struct proc_event *)msg->data; + memset(&ev->event_data, 0, sizeof(ev->event_data)); 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); @@ -161,6 +168,7 @@ void proc_sid_connector(struct task_struct *task) memcpy(&msg->id, &cn_proc_event_id, sizeof(msg->id)); msg->ack = 0; /* not used */ msg->len = sizeof(*ev); + msg->flags = 0; /* not used */ cn_netlink_send(msg, CN_IDX_PROC, GFP_KERNEL); } @@ -176,8 +184,10 @@ void proc_exit_connector(struct task_struct *task) msg = (struct cn_msg*)buffer; ev = (struct proc_event*)msg->data; + memset(&ev->event_data, 0, sizeof(ev->event_data)); get_seq(&msg->seq, &ev->cpu); ktime_get_ts(&ts); /* get high res monotonic timestamp */ + memset(&ev->event_data, 0, sizeof(ev->event_data)); put_unaligned(timespec_to_ns(&ts), (__u64 *)&ev->timestamp_ns); ev->what = PROC_EVENT_EXIT; ev->event_data.exit.process_pid = task->pid; @@ -188,6 +198,7 @@ void proc_exit_connector(struct task_struct *task)
Re: [ 030/143] proc connector: fix info leaks
On Mon, May 12, 2014 at 10:51:31AM +0200, Mathias Krause wrote: > On 12 May 2014 02:32, Willy Tarreau wrote: > > 2.6.32-longterm review patch. If anyone has any objections, please let me > > know. > > > > -- > > > > From: Mathias Krause > > > > [ Upstream commit e727ca82e0e9616ab4844301e6bae60ca7327682 ] > > > > Initialize event_data for all possible message types to prevent leaking > > kernel stack contents to userland (up to 20 bytes). Also set the flags > > member of the connector message to 0 to prevent leaking two more stack > > bytes this way. > > > > Cc: sta...@vger.kernel.org > > Signed-off-by: Mathias Krause > > Signed-off-by: David S. Miller > > Signed-off-by: Willy Tarreau > > --- > > drivers/connector/cn_proc.c | 16 > > 1 file changed, 16 insertions(+) > > > > diff --git a/drivers/connector/cn_proc.c b/drivers/connector/cn_proc.c > > index 6069790..3a2587a 100644 > > --- a/drivers/connector/cn_proc.c > > +++ b/drivers/connector/cn_proc.c > > [...] > > module_init(cn_proc_init); > > + memset(&ev->event_data, 0, sizeof(ev->event_data)); > > + msg->flags = 0; /* not used */ > > That last hunk looks bogus. Probably the source of Christoph's compile error. Thank you guys, I'll check. I don't know why I didn't see them on an allmodconfig build. Willy -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [ 030/143] proc connector: fix info leaks
On 12 May 2014 02:32, Willy Tarreau wrote: > 2.6.32-longterm review patch. If anyone has any objections, please let me > know. > > -- > > From: Mathias Krause > > [ Upstream commit e727ca82e0e9616ab4844301e6bae60ca7327682 ] > > Initialize event_data for all possible message types to prevent leaking > kernel stack contents to userland (up to 20 bytes). Also set the flags > member of the connector message to 0 to prevent leaking two more stack > bytes this way. > > Cc: sta...@vger.kernel.org > Signed-off-by: Mathias Krause > Signed-off-by: David S. Miller > Signed-off-by: Willy Tarreau > --- > drivers/connector/cn_proc.c | 16 > 1 file changed, 16 insertions(+) > > diff --git a/drivers/connector/cn_proc.c b/drivers/connector/cn_proc.c > index 6069790..3a2587a 100644 > --- a/drivers/connector/cn_proc.c > +++ b/drivers/connector/cn_proc.c > [...] > module_init(cn_proc_init); > + memset(&ev->event_data, 0, sizeof(ev->event_data)); > + msg->flags = 0; /* not used */ That last hunk looks bogus. Probably the source of Christoph's compile error. Mathias -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [ 030/143] proc connector: fix info leaks
Willy Tarreau wrote... > Initialize event_data for all possible message types to prevent leaking > kernel stack contents to userland (up to 20 bytes). Also set the flags > member of the connector message to 0 to prevent leaking two more stack > bytes this way. There are build errors as shown below and I guess that one is the culprit. Can do detailled checks tonight, I'm a bit in a hurry right now. (Using gcc-4.7 as provided by Debian wheezy) Christoph drivers/connector/cn_proc.c:286:9: error: expected declaration specifiers or '...' before '&' token drivers/connector/cn_proc.c:286:26: error: expected declaration specifiers or '...' before numeric constant drivers/connector/cn_proc.c:286:29: error: expected declaration specifiers or '...' before 'sizeof' drivers/connector/cn_proc.c:287:5: error: expected '=', ',', ';', 'asm' or '__attribute__' before '->' token make[5]: *** [drivers/connector/cn_proc.o] Error 1 make[4]: *** [drivers/connector] Error 2 make[4]: *** Waiting for unfinished jobs -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[ 030/143] proc connector: fix info leaks
2.6.32-longterm review patch. If anyone has any objections, please let me know. -- From: Mathias Krause [ Upstream commit e727ca82e0e9616ab4844301e6bae60ca7327682 ] Initialize event_data for all possible message types to prevent leaking kernel stack contents to userland (up to 20 bytes). Also set the flags member of the connector message to 0 to prevent leaking two more stack bytes this way. Cc: sta...@vger.kernel.org Signed-off-by: Mathias Krause Signed-off-by: David S. Miller Signed-off-by: Willy Tarreau --- drivers/connector/cn_proc.c | 16 1 file changed, 16 insertions(+) diff --git a/drivers/connector/cn_proc.c b/drivers/connector/cn_proc.c index 6069790..3a2587a 100644 --- a/drivers/connector/cn_proc.c +++ b/drivers/connector/cn_proc.c @@ -59,6 +59,7 @@ void proc_fork_connector(struct task_struct *task) msg = (struct cn_msg*)buffer; ev = (struct proc_event*)msg->data; + memset(&ev->event_data, 0, sizeof(ev->event_data)); 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); @@ -71,6 +72,7 @@ void proc_fork_connector(struct task_struct *task) memcpy(&msg->id, &cn_proc_event_id, sizeof(msg->id)); msg->ack = 0; /* not used */ msg->len = sizeof(*ev); + msg->flags = 0; /* not used */ /* If cn_netlink_send() failed, the data is not sent */ cn_netlink_send(msg, CN_IDX_PROC, GFP_KERNEL); } @@ -87,6 +89,7 @@ void proc_exec_connector(struct task_struct *task) msg = (struct cn_msg*)buffer; ev = (struct proc_event*)msg->data; + memset(&ev->event_data, 0, sizeof(ev->event_data)); 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); @@ -97,6 +100,7 @@ void proc_exec_connector(struct task_struct *task) memcpy(&msg->id, &cn_proc_event_id, sizeof(msg->id)); msg->ack = 0; /* not used */ msg->len = sizeof(*ev); + msg->flags = 0; /* not used */ cn_netlink_send(msg, CN_IDX_PROC, GFP_KERNEL); } @@ -113,6 +117,7 @@ void proc_id_connector(struct task_struct *task, int which_id) msg = (struct cn_msg*)buffer; ev = (struct proc_event*)msg->data; + memset(&ev->event_data, 0, sizeof(ev->event_data)); ev->what = which_id; ev->event_data.id.process_pid = task->pid; ev->event_data.id.process_tgid = task->tgid; @@ -136,6 +141,7 @@ void proc_id_connector(struct task_struct *task, int which_id) memcpy(&msg->id, &cn_proc_event_id, sizeof(msg->id)); msg->ack = 0; /* not used */ msg->len = sizeof(*ev); + msg->flags = 0; /* not used */ cn_netlink_send(msg, CN_IDX_PROC, GFP_KERNEL); } @@ -151,6 +157,7 @@ void proc_sid_connector(struct task_struct *task) msg = (struct cn_msg *)buffer; ev = (struct proc_event *)msg->data; + memset(&ev->event_data, 0, sizeof(ev->event_data)); 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); @@ -161,6 +168,7 @@ void proc_sid_connector(struct task_struct *task) memcpy(&msg->id, &cn_proc_event_id, sizeof(msg->id)); msg->ack = 0; /* not used */ msg->len = sizeof(*ev); + msg->flags = 0; /* not used */ cn_netlink_send(msg, CN_IDX_PROC, GFP_KERNEL); } @@ -176,8 +184,10 @@ void proc_exit_connector(struct task_struct *task) msg = (struct cn_msg*)buffer; ev = (struct proc_event*)msg->data; + memset(&ev->event_data, 0, sizeof(ev->event_data)); get_seq(&msg->seq, &ev->cpu); ktime_get_ts(&ts); /* get high res monotonic timestamp */ + memset(&ev->event_data, 0, sizeof(ev->event_data)); put_unaligned(timespec_to_ns(&ts), (__u64 *)&ev->timestamp_ns); ev->what = PROC_EVENT_EXIT; ev->event_data.exit.process_pid = task->pid; @@ -188,6 +198,7 @@ void proc_exit_connector(struct task_struct *task) memcpy(&msg->id, &cn_proc_event_id, sizeof(msg->id)); msg->ack = 0; /* not used */ msg->len = sizeof(*ev); + msg->flags = 0; /* not used */ cn_netlink_send(msg, CN_IDX_PROC, GFP_KERNEL); } @@ -211,6 +222,7 @@ static void cn_proc_ack(int err, int rcvd_seq, int rcvd_ack) msg = (struct cn_msg*)buffer; ev = (struct proc_event*)msg->data; + memset(&ev->event_data, 0, sizeof(ev->event_data)); msg->seq = rcvd_seq; ktime_get_ts(&ts); /* get high res monotonic timestamp */ put_unaligned(timespec_to_ns(&ts), (__u64 *)&ev->timestamp_ns); @@ -220,6 +232,7 @@ static void cn_proc_ack(int err, int rcvd_seq, int rcvd_ack) memcpy(&msg->id, &cn_proc_event_id, sizeof(msg->id)); msg->ack
