Menu
▾
▴
linux-fbdev-devel
|
From: Philipp R. <phi...@li...> - 2009-10-02 12:41:33
|
Affected: All code that uses connector, in kernel and out of mainline The connector, as it is today, does not allow the in kernel receiving parts to do any checks on privileges of a message's sender. I know, there are not many out there that like connector, but as long as it is in the kernel, we have to fix the security issues it has! Please either drop connector, or someone who feels a bit responsible and has our beloved dictator's blessing, PLEASE PLEASE PLEASE take this into your tree, and send the pull request to Linus. Patches 1 to 4 are already Acked-by Evgeny, the connector's maintainer. Patches 5 to 7 are the obvious fixes to the connector user's code. For convenience these patches are also available as git tree: git://git.drbd.org/linux-2.6-drbd.git connector-fix -Phil Philipp Reisner (8): connector: Keep the skb in cn_callback_data connector: Provide the sender's credentials to the callback connector/dm: Fixed a compilation warning connector: Removed the destruct_data callback since it is always kfree_skb() dm/connector: Only process connector packages from privileged processes dst/connector: Disallow unpliviged users to configure dst pohmelfs/connector: Disallow unpliviged users to configure pohmelfs uvesafb/connector: Disallow unpliviged users to send netlink packets Documentation/connector/cn_test.c | 2 +- Documentation/connector/connector.txt | 8 ++++---- drivers/connector/cn_queue.c | 12 +++++++----- drivers/connector/connector.c | 22 ++++++++-------------- drivers/md/dm-log-userspace-transfer.c | 6 ++++-- drivers/staging/dst/dcore.c | 7 ++++++- drivers/staging/pohmelfs/config.c | 5 ++++- drivers/video/uvesafb.c | 5 ++++- drivers/w1/w1_netlink.c | 2 +- include/linux/connector.h | 11 ++++------- 10 files changed, 43 insertions(+), 37 deletions(-) |
|
From: Greg KH <gr...@kr...> - 2009-10-02 14:12:27
|
On Fri, Oct 02, 2009 at 02:40:03PM +0200, Philipp Reisner wrote: > Affected: All code that uses connector, in kernel and out of mainline > > The connector, as it is today, does not allow the in kernel receiving > parts to do any checks on privileges of a message's sender. So, assume I know nothing about the connector architecture, what does this mean in a security context? > I know, there are not many out there that like connector, but as > long as it is in the kernel, we have to fix the security issues it has! And what specifically are the security issues? > Please either drop connector, or someone who feels a bit responsible > and has our beloved dictator's blessing, PLEASE PLEASE PLEASE take > this into your tree, and send the pull request to Linus. > > Patches 1 to 4 are already Acked-by Evgeny, the connector's maintainer. > Patches 5 to 7 are the obvious fixes to the connector user's code. Obvious in what way? thanks, greg k-h |
|
From: Philipp R. <phi...@li...> - 2009-10-02 18:21:07
|
> On Fri, Oct 02, 2009 at 02:40:03PM +0200, Philipp Reisner wrote: > > Affected: All code that uses connector, in kernel and out of mainline > > > > The connector, as it is today, does not allow the in kernel receiving > > parts to do any checks on privileges of a message's sender. > > So, assume I know nothing about the connector architecture, what does > this mean in a security context? > Think of the connector as a layer on top of netlink that allows more than a hard coded number of subsystems to use netlink. Netlink is used e.g. to modify routing tables in the kernel. As it is today, subsystem utilising the connector can not examine the capabilities of the user/program that sent the netlink message. If the same would be true for netlink, than every unprivileged user could change the routing tables on your box. > > I know, there are not many out there that like connector, but as > > long as it is in the kernel, we have to fix the security issues it has! > > And what specifically are the security issues? > unprivileged users can trigger operations that are supposed to be only accessible to users having CAP_SYS_ADMIN (or some other CAP_XXX) > > Please either drop connector, or someone who feels a bit responsible > > and has our beloved dictator's blessing, PLEASE PLEASE PLEASE take > > this into your tree, and send the pull request to Linus. > > > > Patches 1 to 4 are already Acked-by Evgeny, the connector's maintainer. > > Patches 5 to 7 are the obvious fixes to the connector user's code. > > Obvious in what way? > They limit processing of connector/netlink messages in these subsystems to messages sent from root (or some user having CAP_SYS_ADMIN). That is obvious for dst, because device setup and destruction is done by connector messages. This is obvious for pohmelfs becuase these connector messages are used there to change some configuration. This is obvious for uvesafb because the connector messages are used there to delegate some video bios emulation to userspace. Last not least dm's dirty logging in user space, should be immune to some crafted netlink packets sent by some unprivileged user. Patches 1 to 4 fix the framework, should be merged as soon as possible. Patches 5 to 8 (not 7) should probably be blessed by the affected subsystem's maintainers. I think I have put all on CC. HTH. -phil |
|
From: David M. <da...@da...> - 2009-10-02 18:13:29
|
From: Philipp Reisner <phi...@li...> Date: Fri, 2 Oct 2009 17:54:12 +0200 > Think of the connector as a layer on top of netlink that allows more > than a hard coded number of subsystems to use netlink. There are no such limits in netlink, we have 'genetlink' which allows an arbitrary number of subsystems to use netlink. What connector provides over netlink/genetlink is something different altogether. |
|
From: Greg KH <gr...@kr...> - 2009-10-02 19:50:42
|
On Fri, Oct 02, 2009 at 05:54:12PM +0200, Philipp Reisner wrote: > > On Fri, Oct 02, 2009 at 02:40:03PM +0200, Philipp Reisner wrote: > > > Affected: All code that uses connector, in kernel and out of mainline > > > > > > The connector, as it is today, does not allow the in kernel receiving > > > parts to do any checks on privileges of a message's sender. > > > > So, assume I know nothing about the connector architecture, what does > > this mean in a security context? > > > > Think of the connector as a layer on top of netlink that allows more > than a hard coded number of subsystems to use netlink. > > Netlink is used e.g. to modify routing tables in the kernel. > > As it is today, subsystem utilising the connector can not examine > the capabilities of the user/program that sent the netlink message. > > If the same would be true for netlink, than every unprivileged user > could change the routing tables on your box. > > > > I know, there are not many out there that like connector, but as > > > long as it is in the kernel, we have to fix the security issues it has! > > > > And what specifically are the security issues? > > > > unprivileged users can trigger operations that are supposed to be only > accessible to users having CAP_SYS_ADMIN (or some other CAP_XXX) Ok, but it doesn't look like there are that many connector operations right now, right? Anyway, I have no objection to the patches, and figure they should go through David's network tree. thanks, greg k-h |
|
From: Lars E. <lar...@li...> - 2009-10-02 18:51:08
|
On Fri, Oct 02, 2009 at 06:58:59AM -0700, Greg KH wrote:
> On Fri, Oct 02, 2009 at 02:40:03PM +0200, Philipp Reisner wrote:
> > Affected: All code that uses connector, in kernel and out of mainline
> >
> > The connector, as it is today, does not allow the in kernel receiving
> > parts to do any checks on privileges of a message's sender.
>
> So, assume I know nothing about the connector architecture, what does
> this mean in a security context?
Arbitrary unprivileged users may craft a netlink message, which gets delivered
through connector to callbacks (registered in kernel with cn_add_callback).
These callbacks will then act on the message, as if it originated from an
"expected" source. But currently there is no mechanism to verify the origin,
even if the callbacks would try to.
> > I know, there are not many out there that like connector, but as
> > long as it is in the kernel, we have to fix the security issues it has!
>
> And what specifically are the security issues?
For the cn_ulog_callback (dm-log-userspace-transfer.c),
someone would be able to fake completion (with or without error code)
of ulog entries, copying arbitrary data into receiving_pkg entries.
/*
* This is the connector callback that delivers data
* that was sent from userspace.
*/
static void cn_ulog_callback(void *data)
{
struct cn_msg *msg = (struct cn_msg *)data;
struct dm_ulog_request *tfr = (struct dm_ulog_request *)(msg + 1);
spin_lock(&receiving_list_lock);
if (msg->len == 0)
fill_pkg(msg, NULL);
else if (msg->len < sizeof(*tfr))
DMERR("Incomplete message received (expected %u, got %u): [%u]",
(unsigned)sizeof(*tfr), msg->len, msg->seq);
else
fill_pkg(NULL, tfr);
spin_unlock(&receiving_list_lock);
}
static int fill_pkg(struct cn_msg *msg, struct dm_ulog_request *tfr)
{
uint32_t rtn_seq = (msg) ? msg->seq : (tfr) ? tfr->seq : 0;
...
} else {
pkg->error = tfr->error;
memcpy(pkg->data, tfr->data, tfr->data_size);
*(pkg->data_size) = tfr->data_size;
}
complete(&pkg->complete);
should make that obvious: if an unprivileged user can deliver arbitrary msg to
cn_ulog_callback, that should at least be disruptive to services that use it.
fix: check origin of message for proper credentials (e.g. CAP_SYS_ADMIN).
what or how much damage a crafted message can do in uvesafb_cn_callback,
I'm not sure. But, if I get the msg->seq right, and get by the first
sanity check, again, arbitrary input is copied into some
kernel object, which will likely at least confuse that subsystem,
maybe do damage, or result in some sort of denial of service.
I just don't know what these uvesafb_ktask do, but I doubt that anyone but root
should be able to manipulate them.
in the case of dst and pohemlfs, it is (re|de) configuration of respective in
kernel objects, possibly exposing arbitrary data content
@Evgeniy - is that statement correct? Does something prevent an
unprivileged user to export arbitrary things via dst?
At least some sort of denial of service should be possible there.
for DRBD, we have of course similar problems as long as we use the connector
in its current form as our configuration choice.
I'm not sure what actual harm can be done by arbitrary calling
w1_reset_select_slave(), or w1_process_command_io(),
but allowing unprivileged users to meddle with arbitrary devices is most likely
not the intended behaviour there, either.
The "obvious" way was to first make the credentials and capabilities of the
message origin available to these callbacks, and then test on "CAP_SYS_ADMIN".
Note that the suggested usage of the connector for _userspace_ tools
is to bind() to some netlink socket, subscribing to apropriate mutlicast
groups, which will usually fail for unprivileged users in netlink_bind()
because of
/* Only superuser is allowed to listen multicasts */
if (nladdr->nl_groups) {
if (!netlink_capable(sock, NL_NONROOT_RECV))
return -EPERM;
err = netlink_realloc_groups(sk);
if (err)
return err;
}
So typical userspace tools will fail when used as non-root.
But if you leave out the bind, you are perfectly able to _send_ arbitrary
messages on that socket, even if you are not able to receive any replies from
connector kernel space in that case.
Cheers,
Lars
|
|
From: Greg KH <gr...@kr...> - 2009-10-02 18:03:03
|
On Fri, Oct 02, 2009 at 10:56:59AM -0700, David Miller wrote: > From: Philipp Reisner <phi...@li...> > Date: Fri, 2 Oct 2009 14:40:03 +0200 > > > Affected: All code that uses connector, in kernel and out of mainline > > > > The connector, as it is today, does not allow the in kernel receiving > > parts to do any checks on privileges of a message's sender. > > > > I know, there are not many out there that like connector, but as > > long as it is in the kernel, we have to fix the security issues it has! > > > > Please either drop connector, or someone who feels a bit responsible > > and has our beloved dictator's blessing, PLEASE PLEASE PLEASE take > > this into your tree, and send the pull request to Linus. > > > > Patches 1 to 4 are already Acked-by Evgeny, the connector's maintainer. > > Patches 5 to 7 are the obvious fixes to the connector user's code. > > > > For convenience these patches are also available as git tree: > > git://git.drbd.org/linux-2.6-drbd.git connector-fix > > All applied to net-2.6, I'll push this out to Linus later > today. Should it also go to -stable? If so, I can pick it up once it hits Linus's tree. thanks, greg k-h |
|
From: David M. <da...@da...> - 2009-10-02 18:03:29
|
From: Philipp Reisner <phi...@li...> Date: Fri, 2 Oct 2009 14:40:03 +0200 > Affected: All code that uses connector, in kernel and out of mainline > > The connector, as it is today, does not allow the in kernel receiving > parts to do any checks on privileges of a message's sender. > > I know, there are not many out there that like connector, but as > long as it is in the kernel, we have to fix the security issues it has! > > Please either drop connector, or someone who feels a bit responsible > and has our beloved dictator's blessing, PLEASE PLEASE PLEASE take > this into your tree, and send the pull request to Linus. > > Patches 1 to 4 are already Acked-by Evgeny, the connector's maintainer. > Patches 5 to 7 are the obvious fixes to the connector user's code. > > For convenience these patches are also available as git tree: > git://git.drbd.org/linux-2.6-drbd.git connector-fix All applied to net-2.6, I'll push this out to Linus later today. |
|
From: David M. <da...@da...> - 2009-10-02 18:18:26
|
From: Greg KH <gr...@kr...> Date: Fri, 2 Oct 2009 11:00:22 -0700 > On Fri, Oct 02, 2009 at 10:56:59AM -0700, David Miller wrote: >> All applied to net-2.6, I'll push this out to Linus later >> today. > > Should it also go to -stable? If so, I can pick it up once it hits > Linus's tree. Yes, please take it into -stable. Greg, I'll also send you a batch of other networking bits for -stable later this afternoon as well, just FYI... |
|
From: Greg KH <gr...@kr...> - 2009-10-02 18:39:50
|
On Fri, Oct 02, 2009 at 11:05:04AM -0700, David Miller wrote: > From: Greg KH <gr...@kr...> > Date: Fri, 2 Oct 2009 11:00:22 -0700 > > > On Fri, Oct 02, 2009 at 10:56:59AM -0700, David Miller wrote: > >> All applied to net-2.6, I'll push this out to Linus later > >> today. > > > > Should it also go to -stable? If so, I can pick it up once it hits > > Linus's tree. > > Yes, please take it into -stable. Will do. > Greg, I'll also send you a batch of other networking bits > for -stable later this afternoon as well, just FYI... Great, I'll queue it up for the next -stable releases, these are full enough as it is already :) thanks, greg k-h |
|
From: Philipp R. <phi...@li...> - 2009-10-02 12:41:33
|
Signed-off-by: Philipp Reisner <phi...@li...>
Acked-by: Lars Ellenberg <lar...@li...>
Acked-by: Evgeniy Polyakov <zb...@io...>
---
drivers/connector/cn_queue.c | 3 ++-
drivers/connector/connector.c | 11 +++++------
include/linux/connector.h | 4 ++--
3 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/connector/cn_queue.c b/drivers/connector/cn_queue.c
index 4a1dfe1..b4cfac9 100644
--- a/drivers/connector/cn_queue.c
+++ b/drivers/connector/cn_queue.c
@@ -78,8 +78,9 @@ void cn_queue_wrapper(struct work_struct *work)
struct cn_callback_entry *cbq =
container_of(work, struct cn_callback_entry, work);
struct cn_callback_data *d = &cbq->data;
+ struct cn_msg *msg = NLMSG_DATA(nlmsg_hdr(d->skb));
- d->callback(d->callback_priv);
+ d->callback(msg);
d->destruct_data(d->ddata);
d->ddata = NULL;
diff --git a/drivers/connector/connector.c b/drivers/connector/connector.c
index 74f52af..fc9887f 100644
--- a/drivers/connector/connector.c
+++ b/drivers/connector/connector.c
@@ -129,10 +129,11 @@ EXPORT_SYMBOL_GPL(cn_netlink_send);
/*
* Callback helper - queues work and setup destructor for given data.
*/
-static int cn_call_callback(struct cn_msg *msg, void (*destruct_data)(void *), void *data)
+static int cn_call_callback(struct sk_buff *skb, void (*destruct_data)(void *), void *data)
{
struct cn_callback_entry *__cbq, *__new_cbq;
struct cn_dev *dev = &cdev;
+ struct cn_msg *msg = NLMSG_DATA(nlmsg_hdr(skb));
int err = -ENODEV;
spin_lock_bh(&dev->cbdev->queue_lock);
@@ -140,7 +141,7 @@ static int cn_call_callback(struct cn_msg *msg, void (*destruct_data)(void *), v
if (cn_cb_equal(&__cbq->id.id, &msg->id)) {
if (likely(!work_pending(&__cbq->work) &&
__cbq->data.ddata == NULL)) {
- __cbq->data.callback_priv = msg;
+ __cbq->data.skb = skb;
__cbq->data.ddata = data;
__cbq->data.destruct_data = destruct_data;
@@ -156,7 +157,7 @@ static int cn_call_callback(struct cn_msg *msg, void (*destruct_data)(void *), v
__new_cbq = kzalloc(sizeof(struct cn_callback_entry), GFP_ATOMIC);
if (__new_cbq) {
d = &__new_cbq->data;
- d->callback_priv = msg;
+ d->skb = skb;
d->callback = __cbq->data.callback;
d->ddata = data;
d->destruct_data = destruct_data;
@@ -191,7 +192,6 @@ static int cn_call_callback(struct cn_msg *msg, void (*destruct_data)(void *), v
*/
static void cn_rx_skb(struct sk_buff *__skb)
{
- struct cn_msg *msg;
struct nlmsghdr *nlh;
int err;
struct sk_buff *skb;
@@ -208,8 +208,7 @@ static void cn_rx_skb(struct sk_buff *__skb)
return;
}
- msg = NLMSG_DATA(nlh);
- err = cn_call_callback(msg, (void (*)(void *))kfree_skb, skb);
+ err = cn_call_callback(skb, (void (*)(void *))kfree_skb, skb);
if (err < 0)
kfree_skb(skb);
}
diff --git a/include/linux/connector.h b/include/linux/connector.h
index 47ebf41..05a7a14 100644
--- a/include/linux/connector.h
+++ b/include/linux/connector.h
@@ -134,8 +134,8 @@ struct cn_callback_id {
struct cn_callback_data {
void (*destruct_data) (void *);
void *ddata;
-
- void *callback_priv;
+
+ struct sk_buff *skb;
void (*callback) (struct cn_msg *);
void *free;
--
1.6.0.4
|
|
From: Philipp R. <phi...@li...> - 2009-10-02 12:41:33
|
Signed-off-by: Philipp Reisner <phi...@li...>
Acked-by: Lars Ellenberg <lar...@li...>
Acked-by: Evgeniy Polyakov <zb...@io...>
---
Documentation/connector/cn_test.c | 2 +-
Documentation/connector/connector.txt | 8 ++++----
drivers/connector/cn_queue.c | 7 ++++---
drivers/connector/connector.c | 4 ++--
drivers/md/dm-log-userspace-transfer.c | 2 +-
drivers/staging/dst/dcore.c | 2 +-
drivers/staging/pohmelfs/config.c | 2 +-
drivers/video/uvesafb.c | 2 +-
drivers/w1/w1_netlink.c | 2 +-
include/linux/connector.h | 6 +++---
10 files changed, 19 insertions(+), 18 deletions(-)
diff --git a/Documentation/connector/cn_test.c b/Documentation/connector/cn_test.c
index 1711adc..b07add3 100644
--- a/Documentation/connector/cn_test.c
+++ b/Documentation/connector/cn_test.c
@@ -34,7 +34,7 @@ static char cn_test_name[] = "cn_test";
static struct sock *nls;
static struct timer_list cn_test_timer;
-static void cn_test_callback(struct cn_msg *msg)
+static void cn_test_callback(struct cn_msg *msg, struct netlink_skb_parms *nsp)
{
pr_info("%s: %lu: idx=%x, val=%x, seq=%u, ack=%u, len=%d: %s.\n",
__func__, jiffies, msg->id.idx, msg->id.val,
diff --git a/Documentation/connector/connector.txt b/Documentation/connector/connector.txt
index 81e6bf6..78c9466 100644
--- a/Documentation/connector/connector.txt
+++ b/Documentation/connector/connector.txt
@@ -23,7 +23,7 @@ handling, etc... The Connector driver allows any kernelspace agents to use
netlink based networking for inter-process communication in a significantly
easier way:
-int cn_add_callback(struct cb_id *id, char *name, void (*callback) (void *));
+int cn_add_callback(struct cb_id *id, char *name, void (*callback) (struct cn_msg *, struct netlink_skb_parms *));
void cn_netlink_send(struct cn_msg *msg, u32 __group, int gfp_mask);
struct cb_id
@@ -53,15 +53,15 @@ struct cn_msg
Connector interfaces.
/*****************************************/
-int cn_add_callback(struct cb_id *id, char *name, void (*callback) (void *));
+int cn_add_callback(struct cb_id *id, char *name, void (*callback) (struct cn_msg *, struct netlink_skb_parms *));
Registers new callback with connector core.
struct cb_id *id - unique connector's user identifier.
It must be registered in connector.h for legal in-kernel users.
char *name - connector's callback symbolic name.
- void (*callback) (void *) - connector's callback.
- Argument must be dereferenced to struct cn_msg *.
+ void (*callback) (struct cn..) - connector's callback.
+ cn_msg and the sender's credentials
void cn_del_callback(struct cb_id *id);
diff --git a/drivers/connector/cn_queue.c b/drivers/connector/cn_queue.c
index b4cfac9..163c3e3 100644
--- a/drivers/connector/cn_queue.c
+++ b/drivers/connector/cn_queue.c
@@ -79,8 +79,9 @@ void cn_queue_wrapper(struct work_struct *work)
container_of(work, struct cn_callback_entry, work);
struct cn_callback_data *d = &cbq->data;
struct cn_msg *msg = NLMSG_DATA(nlmsg_hdr(d->skb));
+ struct netlink_skb_parms *nsp = &NETLINK_CB(d->skb);
- d->callback(msg);
+ d->callback(msg, nsp);
d->destruct_data(d->ddata);
d->ddata = NULL;
@@ -90,7 +91,7 @@ void cn_queue_wrapper(struct work_struct *work)
static struct cn_callback_entry *
cn_queue_alloc_callback_entry(char *name, struct cb_id *id,
- void (*callback)(struct cn_msg *))
+ void (*callback)(struct cn_msg *, struct netlink_skb_parms *))
{
struct cn_callback_entry *cbq;
@@ -124,7 +125,7 @@ int cn_cb_equal(struct cb_id *i1, struct cb_id *i2)
}
int cn_queue_add_callback(struct cn_queue_dev *dev, char *name, struct cb_id *id,
- void (*callback)(struct cn_msg *))
+ void (*callback)(struct cn_msg *, struct netlink_skb_parms *))
{
struct cn_callback_entry *cbq, *__cbq;
int found = 0;
diff --git a/drivers/connector/connector.c b/drivers/connector/connector.c
index fc9887f..e59f0ab 100644
--- a/drivers/connector/connector.c
+++ b/drivers/connector/connector.c
@@ -269,7 +269,7 @@ static void cn_notify(struct cb_id *id, u32 notify_event)
* May sleep.
*/
int cn_add_callback(struct cb_id *id, char *name,
- void (*callback)(struct cn_msg *))
+ void (*callback)(struct cn_msg *, struct netlink_skb_parms *))
{
int err;
struct cn_dev *dev = &cdev;
@@ -351,7 +351,7 @@ static int cn_ctl_msg_equals(struct cn_ctl_msg *m1, struct cn_ctl_msg *m2)
*
* Used for notification of a request's processing.
*/
-static void cn_callback(struct cn_msg *msg)
+static void cn_callback(struct cn_msg *msg, struct netlink_skb_parms *nsp)
{
struct cn_ctl_msg *ctl;
struct cn_ctl_entry *ent;
diff --git a/drivers/md/dm-log-userspace-transfer.c b/drivers/md/dm-log-userspace-transfer.c
index ba0edad..556131f 100644
--- a/drivers/md/dm-log-userspace-transfer.c
+++ b/drivers/md/dm-log-userspace-transfer.c
@@ -129,7 +129,7 @@ static int fill_pkg(struct cn_msg *msg, struct dm_ulog_request *tfr)
* This is the connector callback that delivers data
* that was sent from userspace.
*/
-static void cn_ulog_callback(void *data)
+static void cn_ulog_callback(void *data, struct netlink_skb_parms *nsp)
{
struct cn_msg *msg = (struct cn_msg *)data;
struct dm_ulog_request *tfr = (struct dm_ulog_request *)(msg + 1);
diff --git a/drivers/staging/dst/dcore.c b/drivers/staging/dst/dcore.c
index ac85773..3943c91 100644
--- a/drivers/staging/dst/dcore.c
+++ b/drivers/staging/dst/dcore.c
@@ -847,7 +847,7 @@ static dst_command_func dst_commands[] = {
/*
* Configuration parser.
*/
-static void cn_dst_callback(struct cn_msg *msg)
+static void cn_dst_callback(struct cn_msg *msg, struct netlink_skb_parms *nsp)
{
struct dst_ctl *ctl;
int err;
diff --git a/drivers/staging/pohmelfs/config.c b/drivers/staging/pohmelfs/config.c
index 90f962e..c9162b3 100644
--- a/drivers/staging/pohmelfs/config.c
+++ b/drivers/staging/pohmelfs/config.c
@@ -527,7 +527,7 @@ out_unlock:
return err;
}
-static void pohmelfs_cn_callback(struct cn_msg *msg)
+static void pohmelfs_cn_callback(struct cn_msg *msg, struct netlink_skb_parms *nsp)
{
int err;
diff --git a/drivers/video/uvesafb.c b/drivers/video/uvesafb.c
index e98baf6..aa7cd95 100644
--- a/drivers/video/uvesafb.c
+++ b/drivers/video/uvesafb.c
@@ -67,7 +67,7 @@ static DEFINE_MUTEX(uvfb_lock);
* find the kernel part of the task struct, copy the registers and
* the buffer contents and then complete the task.
*/
-static void uvesafb_cn_callback(struct cn_msg *msg)
+static void uvesafb_cn_callback(struct cn_msg *msg, struct netlink_skb_parms *nsp)
{
struct uvesafb_task *utask;
struct uvesafb_ktask *task;
diff --git a/drivers/w1/w1_netlink.c b/drivers/w1/w1_netlink.c
index 52ccb3d..45c126f 100644
--- a/drivers/w1/w1_netlink.c
+++ b/drivers/w1/w1_netlink.c
@@ -306,7 +306,7 @@ static int w1_netlink_send_error(struct cn_msg *rcmsg, struct w1_netlink_msg *rm
return error;
}
-static void w1_cn_callback(struct cn_msg *msg)
+static void w1_cn_callback(struct cn_msg *msg, struct netlink_skb_parms *nsp)
{
struct w1_netlink_msg *m = (struct w1_netlink_msg *)(msg + 1);
struct w1_netlink_cmd *cmd;
diff --git a/include/linux/connector.h b/include/linux/connector.h
index 05a7a14..545728e 100644
--- a/include/linux/connector.h
+++ b/include/linux/connector.h
@@ -136,7 +136,7 @@ struct cn_callback_data {
void *ddata;
struct sk_buff *skb;
- void (*callback) (struct cn_msg *);
+ void (*callback) (struct cn_msg *, struct netlink_skb_parms *);
void *free;
};
@@ -167,11 +167,11 @@ struct cn_dev {
struct cn_queue_dev *cbdev;
};
-int cn_add_callback(struct cb_id *, char *, void (*callback) (struct cn_msg *));
+int cn_add_callback(struct cb_id *, char *, void (*callback) (struct cn_msg *, struct netlink_skb_parms *));
void cn_del_callback(struct cb_id *);
int cn_netlink_send(struct cn_msg *, u32, gfp_t);
-int cn_queue_add_callback(struct cn_queue_dev *dev, char *name, struct cb_id *id, void (*callback)(struct cn_msg *));
+int cn_queue_add_callback(struct cn_queue_dev *dev, char *name, struct cb_id *id, void (*callback)(struct cn_msg *, struct netlink_skb_parms *));
void cn_queue_del_callback(struct cn_queue_dev *dev, struct cb_id *id);
int queue_cn_work(struct cn_callback_entry *cbq, struct work_struct *work);
--
1.6.0.4
|
|
From: Philipp R. <phi...@li...> - 2009-10-02 12:41:33
|
Signed-off-by: Philipp Reisner <phi...@li...>
Acked-by: Lars Ellenberg <lar...@li...>
Acked-by: Evgeniy Polyakov <zb...@io...>
---
drivers/md/dm-log-userspace-transfer.c | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/drivers/md/dm-log-userspace-transfer.c b/drivers/md/dm-log-userspace-transfer.c
index 556131f..1327e1a 100644
--- a/drivers/md/dm-log-userspace-transfer.c
+++ b/drivers/md/dm-log-userspace-transfer.c
@@ -129,9 +129,8 @@ static int fill_pkg(struct cn_msg *msg, struct dm_ulog_request *tfr)
* This is the connector callback that delivers data
* that was sent from userspace.
*/
-static void cn_ulog_callback(void *data, struct netlink_skb_parms *nsp)
+static void cn_ulog_callback(struct cn_msg *msg, struct netlink_skb_parms *nsp)
{
- struct cn_msg *msg = (struct cn_msg *)data;
struct dm_ulog_request *tfr = (struct dm_ulog_request *)(msg + 1);
spin_lock(&receiving_list_lock);
--
1.6.0.4
|
|
From: Philipp R. <phi...@li...> - 2009-10-02 12:42:51
|
Signed-off-by: Philipp Reisner <phi...@li...>
---
drivers/staging/dst/dcore.c | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/drivers/staging/dst/dcore.c b/drivers/staging/dst/dcore.c
index 3943c91..ee16010 100644
--- a/drivers/staging/dst/dcore.c
+++ b/drivers/staging/dst/dcore.c
@@ -855,6 +855,11 @@ static void cn_dst_callback(struct cn_msg *msg, struct netlink_skb_parms *nsp)
struct dst_node *n = NULL, *tmp;
unsigned int hash;
+ if (!cap_raised(nsp->eff_cap, CAP_SYS_ADMIN)) {
+ err = -EPERM;
+ goto out;
+ }
+
if (msg->len < sizeof(struct dst_ctl)) {
err = -EBADMSG;
goto out;
--
1.6.0.4
|
|
From: Philipp R. <phi...@li...> - 2009-10-02 12:51:28
|
Signed-off-by: Philipp Reisner <phi...@li...>
---
drivers/staging/pohmelfs/config.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/drivers/staging/pohmelfs/config.c b/drivers/staging/pohmelfs/config.c
index c9162b3..5d04bf5 100644
--- a/drivers/staging/pohmelfs/config.c
+++ b/drivers/staging/pohmelfs/config.c
@@ -531,6 +531,9 @@ static void pohmelfs_cn_callback(struct cn_msg *msg, struct netlink_skb_parms *n
{
int err;
+ if (!cap_raised(nsp->eff_cap, CAP_SYS_ADMIN))
+ return;
+
switch (msg->flags) {
case POHMELFS_FLAGS_ADD:
case POHMELFS_FLAGS_DEL:
--
1.6.0.4
|
|
[Linux-fbdev-devel] [PATCH 8/8] uvesafb/connector: Disallow
unpliviged users to send netlink packets
From: Philipp R. <phi...@li...> - 2009-10-02 12:51:29
|
Signed-off-by: Philipp Reisner <phi...@li...> --- drivers/video/uvesafb.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/drivers/video/uvesafb.c b/drivers/video/uvesafb.c index aa7cd95..e35232a 100644 --- a/drivers/video/uvesafb.c +++ b/drivers/video/uvesafb.c @@ -72,6 +72,9 @@ static void uvesafb_cn_callback(struct cn_msg *msg, struct netlink_skb_parms *ns struct uvesafb_task *utask; struct uvesafb_ktask *task; + if (!cap_raised(nsp->eff_cap, CAP_SYS_ADMIN)) + return; + if (msg->seq >= UVESAFB_TASKS_MAX) return; -- 1.6.0.4 |
|
From: Philipp R. <phi...@li...> - 2009-10-02 12:56:19
|
Signed-off-by: Philipp Reisner <phi...@li...>
Acked-by: Lars Ellenberg <lar...@li...>
Acked-by: Evgeniy Polyakov <zb...@io...>
---
drivers/connector/cn_queue.c | 4 ++--
drivers/connector/connector.c | 11 +++--------
include/linux/connector.h | 3 ---
3 files changed, 5 insertions(+), 13 deletions(-)
diff --git a/drivers/connector/cn_queue.c b/drivers/connector/cn_queue.c
index 163c3e3..210338e 100644
--- a/drivers/connector/cn_queue.c
+++ b/drivers/connector/cn_queue.c
@@ -83,8 +83,8 @@ void cn_queue_wrapper(struct work_struct *work)
d->callback(msg, nsp);
- d->destruct_data(d->ddata);
- d->ddata = NULL;
+ kfree_skb(d->skb);
+ d->skb = NULL;
kfree(d->free);
}
diff --git a/drivers/connector/connector.c b/drivers/connector/connector.c
index e59f0ab..f060246 100644
--- a/drivers/connector/connector.c
+++ b/drivers/connector/connector.c
@@ -129,7 +129,7 @@ EXPORT_SYMBOL_GPL(cn_netlink_send);
/*
* Callback helper - queues work and setup destructor for given data.
*/
-static int cn_call_callback(struct sk_buff *skb, void (*destruct_data)(void *), void *data)
+static int cn_call_callback(struct sk_buff *skb)
{
struct cn_callback_entry *__cbq, *__new_cbq;
struct cn_dev *dev = &cdev;
@@ -140,12 +140,9 @@ static int cn_call_callback(struct sk_buff *skb, void (*destruct_data)(void *),
list_for_each_entry(__cbq, &dev->cbdev->queue_list, callback_entry) {
if (cn_cb_equal(&__cbq->id.id, &msg->id)) {
if (likely(!work_pending(&__cbq->work) &&
- __cbq->data.ddata == NULL)) {
+ __cbq->data.skb == NULL)) {
__cbq->data.skb = skb;
- __cbq->data.ddata = data;
- __cbq->data.destruct_data = destruct_data;
-
if (queue_cn_work(__cbq, &__cbq->work))
err = 0;
else
@@ -159,8 +156,6 @@ static int cn_call_callback(struct sk_buff *skb, void (*destruct_data)(void *),
d = &__new_cbq->data;
d->skb = skb;
d->callback = __cbq->data.callback;
- d->ddata = data;
- d->destruct_data = destruct_data;
d->free = __new_cbq;
__new_cbq->pdev = __cbq->pdev;
@@ -208,7 +203,7 @@ static void cn_rx_skb(struct sk_buff *__skb)
return;
}
- err = cn_call_callback(skb, (void (*)(void *))kfree_skb, skb);
+ err = cn_call_callback(skb);
if (err < 0)
kfree_skb(skb);
}
diff --git a/include/linux/connector.h b/include/linux/connector.h
index 545728e..3a14615 100644
--- a/include/linux/connector.h
+++ b/include/linux/connector.h
@@ -132,9 +132,6 @@ struct cn_callback_id {
};
struct cn_callback_data {
- void (*destruct_data) (void *);
- void *ddata;
-
struct sk_buff *skb;
void (*callback) (struct cn_msg *, struct netlink_skb_parms *);
--
1.6.0.4
|
|
From: Philipp R. <phi...@li...> - 2009-10-02 22:12:27
|
Signed-off-by: Philipp Reisner <phi...@li...>
---
drivers/md/dm-log-userspace-transfer.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/drivers/md/dm-log-userspace-transfer.c b/drivers/md/dm-log-userspace-transfer.c
index 1327e1a..54abf9e 100644
--- a/drivers/md/dm-log-userspace-transfer.c
+++ b/drivers/md/dm-log-userspace-transfer.c
@@ -133,6 +133,9 @@ static void cn_ulog_callback(struct cn_msg *msg, struct netlink_skb_parms *nsp)
{
struct dm_ulog_request *tfr = (struct dm_ulog_request *)(msg + 1);
+ if (!cap_raised(nsp->eff_cap, CAP_SYS_ADMIN))
+ return;
+
spin_lock(&receiving_list_lock);
if (msg->len == 0)
fill_pkg(msg, NULL);
--
1.6.0.4
|
|
From: Jonathan B. <jbr...@re...> - 2009-10-02 20:47:15
|
This patch (and "[dm-devel] [PATCH 3/8] connector/dm: Fixed a compilation warning") will likely collide with an earlier patch (which agk is pushing) to fix the compilation warning (https://www.redhat.com/archives/dm-devel/2009-September/msg00218.html ), but the fix-up will be trivial. The dm-log-userspace code checks that incoming messages correspond to requests that were sent to userspace by way of a sequence number. If they don't correspond, they are dropped. So, you must be able to receive the messages from this kernel module (be root) in order to be able respond with a message that will be accepted. I can't completely rule out the ability to guess a sequence number, and be able to beat the log daemon in responding while the window of that sequence number's validity is open though... If someone could manage to pull this off with accuracy, they could disrupt the creation of a device, mimic a log device failure, or cause mirror resynchronization to occur to a different area that may simultaneously be performing a write (potential data corruption of a mirror). It would be an impressive feat to accomplish this, but I very much welcome the patch rather than test fate. Reviewed-by: Jonathan Brassow <jbr...@re...> brassow On Oct 2, 2009, at 7:40 AM, Philipp Reisner wrote: > Signed-off-by: Philipp Reisner <phi...@li...> > --- > drivers/md/dm-log-userspace-transfer.c | 3 +++ > 1 files changed, 3 insertions(+), 0 deletions(-) > > diff --git a/drivers/md/dm-log-userspace-transfer.c b/drivers/md/dm- > log-userspace-transfer.c > index 1327e1a..54abf9e 100644 > --- a/drivers/md/dm-log-userspace-transfer.c > +++ b/drivers/md/dm-log-userspace-transfer.c > @@ -133,6 +133,9 @@ static void cn_ulog_callback(struct cn_msg *msg, > struct netlink_skb_parms *nsp) > { > struct dm_ulog_request *tfr = (struct dm_ulog_request *)(msg + 1); > > + if (!cap_raised(nsp->eff_cap, CAP_SYS_ADMIN)) > + return; > + > spin_lock(&receiving_list_lock); > if (msg->len == 0) > fill_pkg(msg, NULL); > -- > 1.6.0.4 > > -- > dm-devel mailing list > dm-...@re... > https://www.redhat.com/mailman/listinfo/dm-devel |
|
From: Greg KH <gr...@kr...> - 2009-10-09 22:38:16
|
On Fri, Oct 02, 2009 at 02:40:03PM +0200, Philipp Reisner wrote: > Affected: All code that uses connector, in kernel and out of mainline > > The connector, as it is today, does not allow the in kernel receiving > parts to do any checks on privileges of a message's sender. > > I know, there are not many out there that like connector, but as > long as it is in the kernel, we have to fix the security issues it has! > > Please either drop connector, or someone who feels a bit responsible > and has our beloved dictator's blessing, PLEASE PLEASE PLEASE take > this into your tree, and send the pull request to Linus. > > Patches 1 to 4 are already Acked-by Evgeny, the connector's maintainer. > Patches 5 to 7 are the obvious fixes to the connector user's code. These don't apply to the 2.6.31-stable tree at all. Could you provide them backported to that tree if you want to see them go into a .31-stable release? thanks, greg k-h |
|
From: Evgeniy P. <zb...@io...> - 2009-10-04 10:55:02
|
On Fri, Oct 02, 2009 at 02:40:03PM +0200, Philipp Reisner (phi...@li...) wrote: > Affected: All code that uses connector, in kernel and out of mainline > > The connector, as it is today, does not allow the in kernel receiving > parts to do any checks on privileges of a message's sender. > > I know, there are not many out there that like connector, but as > long as it is in the kernel, we have to fix the security issues it has! > > Please either drop connector, or someone who feels a bit responsible > and has our beloved dictator's blessing, PLEASE PLEASE PLEASE take > this into your tree, and send the pull request to Linus. How expressive! :) > Patches 1 to 4 are already Acked-by Evgeny, the connector's maintainer. > Patches 5 to 7 are the obvious fixes to the connector user's code. I ack those changes either since they do not affect logic of the user. -- Evgeniy Polyakov |
×
Want the latest updates on software, tech news, and AI?
Get latest updates about software, tech news, and AI from SourceForge directly in your inbox once a month.
Thanks for helping keep SourceForge clean.
X