* [PATHC 0/2] cn: w1: buffer size checks
@ 2014-11-09 22:37 David Fries
2014-11-09 22:37 ` [PATCH 1/2] cn: verify msg->len before making callback David Fries
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: David Fries @ 2014-11-09 22:37 UTC (permalink / raw)
To: linux-kernel; +Cc: Evgeniy Polyakov, Dan Carpenter, Kees Cook
These issues were found by Dan Carpenter with a static checker.
Evgeniy are you okay with these?
[PATCH 1/2] cn: verify msg->len before making callback
[PATCH 2/2] w1: avoid potential u16 overflow
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] cn: verify msg->len before making callback
2014-11-09 22:37 [PATHC 0/2] cn: w1: buffer size checks David Fries
@ 2014-11-09 22:37 ` David Fries
2014-11-09 22:37 ` [PATCH 2/2] w1: avoid potential u16 overflow David Fries
2014-11-10 0:12 ` [PATHC 0/2] cn: w1: buffer size checks Евгений Поляков
2 siblings, 0 replies; 7+ messages in thread
From: David Fries @ 2014-11-09 22:37 UTC (permalink / raw)
To: linux-kernel; +Cc: Evgeniy Polyakov, Dan Carpenter, Kees Cook
The struct cn_msg len field comes from userspace and needs to be
validated. More logical to do so here where the cn_msg pointer is
pulled out of the sk_buff than the callback which is passed cn_msg *
and might assume no validation is needed.
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: David Fries <David@Fries.net>
---
drivers/connector/connector.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/connector/connector.c b/drivers/connector/connector.c
index f612d68..30f5228 100644
--- a/drivers/connector/connector.c
+++ b/drivers/connector/connector.c
@@ -141,12 +141,18 @@ EXPORT_SYMBOL_GPL(cn_netlink_send);
*/
static int cn_call_callback(struct sk_buff *skb)
{
+ struct nlmsghdr *nlh;
struct cn_callback_entry *i, *cbq = NULL;
struct cn_dev *dev = &cdev;
struct cn_msg *msg = nlmsg_data(nlmsg_hdr(skb));
struct netlink_skb_parms *nsp = &NETLINK_CB(skb);
int err = -ENODEV;
+ /* verify msg->len is within skb */
+ nlh = nlmsg_hdr(skb);
+ if (nlh->nlmsg_len < NLMSG_HDRLEN + sizeof(struct cn_msg) + msg->len)
+ return -EINVAL;
+
spin_lock_bh(&dev->cbdev->queue_lock);
list_for_each_entry(i, &dev->cbdev->queue_list, callback_entry) {
if (cn_cb_equal(&i->id.id, &msg->id)) {
--
1.7.10.4
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/2] w1: avoid potential u16 overflow
2014-11-09 22:37 [PATHC 0/2] cn: w1: buffer size checks David Fries
2014-11-09 22:37 ` [PATCH 1/2] cn: verify msg->len before making callback David Fries
@ 2014-11-09 22:37 ` David Fries
2014-11-10 0:12 ` [PATHC 0/2] cn: w1: buffer size checks Евгений Поляков
2 siblings, 0 replies; 7+ messages in thread
From: David Fries @ 2014-11-09 22:37 UTC (permalink / raw)
To: linux-kernel; +Cc: Evgeniy Polyakov, Dan Carpenter, Kees Cook
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: David Fries <David@Fries.net>
---
drivers/w1/w1_netlink.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/w1/w1_netlink.c b/drivers/w1/w1_netlink.c
index dd96562..881597a 100644
--- a/drivers/w1/w1_netlink.c
+++ b/drivers/w1/w1_netlink.c
@@ -598,7 +598,7 @@ static void w1_cn_callback(struct cn_msg *cn, struct netlink_skb_parms *nsp)
msg = (struct w1_netlink_msg *)(cn + 1);
if (node_count) {
int size;
- u16 reply_size = sizeof(*cn) + cn->len + slave_len;
+ int reply_size = sizeof(*cn) + cn->len + slave_len;
if (cn->flags & W1_CN_BUNDLE) {
/* bundling duplicats some of the messages */
reply_size += 2 * cmd_count * (sizeof(struct cn_msg) +
--
1.7.10.4
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATHC 0/2] cn: w1: buffer size checks
2014-11-09 22:37 [PATHC 0/2] cn: w1: buffer size checks David Fries
2014-11-09 22:37 ` [PATCH 1/2] cn: verify msg->len before making callback David Fries
2014-11-09 22:37 ` [PATCH 2/2] w1: avoid potential u16 overflow David Fries
@ 2014-11-10 0:12 ` Евгений Поляков
2014-11-11 2:19 ` David Fries
2 siblings, 1 reply; 7+ messages in thread
From: Евгений Поляков @ 2014-11-10 0:12 UTC (permalink / raw)
To: David Fries, linux-kernel; +Cc: Dan Carpenter, Kees Cook
Hi
10.11.2014, 01:37, "David Fries" <David@Fries.net>:
> These issues were found by Dan Carpenter with a static checker.
> Evgeniy are you okay with these?
>
> [PATCH 1/2] cn: verify msg->len before making callback
> [PATCH 2/2] w1: avoid potential u16 overflow
Both patches look good, thank you
Acked-by: Evgeniy Polyakov <zbr@ioremap.net>
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATHC 0/2] cn: w1: buffer size checks
2014-11-10 0:12 ` [PATHC 0/2] cn: w1: buffer size checks Евгений Поляков
@ 2014-11-11 2:19 ` David Fries
2014-11-11 2:19 ` [PATCH 1/2] cn: verify msg->len before making callback David Fries
2014-11-11 2:19 ` [PATCH 2/2] w1: avoid potential u16 overflow David Fries
0 siblings, 2 replies; 7+ messages in thread
From: David Fries @ 2014-11-11 2:19 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: linux-kernel, Evgeniy Polyakov, Dan Carpenter, Kees Cook
Greg Kroah-Hartman,
These issues were found by Dan Carpenter with a static checker and will check
message buffer lengths from userspace or avoid length overflows. Evgeniy
Polyakov has given his ack, and they can be applied to the stable branch as
well.
[PATCH 1/2] cn: verify msg->len before making callback
[PATCH 2/2] w1: avoid potential u16 overflow
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] cn: verify msg->len before making callback
2014-11-11 2:19 ` David Fries
@ 2014-11-11 2:19 ` David Fries
2014-11-11 2:19 ` [PATCH 2/2] w1: avoid potential u16 overflow David Fries
1 sibling, 0 replies; 7+ messages in thread
From: David Fries @ 2014-11-11 2:19 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: linux-kernel, Evgeniy Polyakov, Dan Carpenter, Kees Cook
The struct cn_msg len field comes from userspace and needs to be
validated. More logical to do so here where the cn_msg pointer is
pulled out of the sk_buff than the callback which is passed cn_msg *
and might assume no validation is needed.
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Acked-by: Evgeniy Polyakov <zbr@ioremap.net>
Signed-off-by: David Fries <David@Fries.net>
---
drivers/connector/connector.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/connector/connector.c b/drivers/connector/connector.c
index f612d68..30f5228 100644
--- a/drivers/connector/connector.c
+++ b/drivers/connector/connector.c
@@ -141,12 +141,18 @@ EXPORT_SYMBOL_GPL(cn_netlink_send);
*/
static int cn_call_callback(struct sk_buff *skb)
{
+ struct nlmsghdr *nlh;
struct cn_callback_entry *i, *cbq = NULL;
struct cn_dev *dev = &cdev;
struct cn_msg *msg = nlmsg_data(nlmsg_hdr(skb));
struct netlink_skb_parms *nsp = &NETLINK_CB(skb);
int err = -ENODEV;
+ /* verify msg->len is within skb */
+ nlh = nlmsg_hdr(skb);
+ if (nlh->nlmsg_len < NLMSG_HDRLEN + sizeof(struct cn_msg) + msg->len)
+ return -EINVAL;
+
spin_lock_bh(&dev->cbdev->queue_lock);
list_for_each_entry(i, &dev->cbdev->queue_list, callback_entry) {
if (cn_cb_equal(&i->id.id, &msg->id)) {
--
1.7.10.4
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/2] w1: avoid potential u16 overflow
2014-11-11 2:19 ` David Fries
2014-11-11 2:19 ` [PATCH 1/2] cn: verify msg->len before making callback David Fries
@ 2014-11-11 2:19 ` David Fries
1 sibling, 0 replies; 7+ messages in thread
From: David Fries @ 2014-11-11 2:19 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: linux-kernel, Evgeniy Polyakov, Dan Carpenter, Kees Cook
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Acked-by: Evgeniy Polyakov <zbr@ioremap.net>
Signed-off-by: David Fries <David@Fries.net>
---
drivers/w1/w1_netlink.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/w1/w1_netlink.c b/drivers/w1/w1_netlink.c
index dd96562..881597a 100644
--- a/drivers/w1/w1_netlink.c
+++ b/drivers/w1/w1_netlink.c
@@ -598,7 +598,7 @@ static void w1_cn_callback(struct cn_msg *cn, struct netlink_skb_parms *nsp)
msg = (struct w1_netlink_msg *)(cn + 1);
if (node_count) {
int size;
- u16 reply_size = sizeof(*cn) + cn->len + slave_len;
+ int reply_size = sizeof(*cn) + cn->len + slave_len;
if (cn->flags & W1_CN_BUNDLE) {
/* bundling duplicats some of the messages */
reply_size += 2 * cmd_count * (sizeof(struct cn_msg) +
--
1.7.10.4
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2014-11-11 2:21 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-11-09 22:37 [PATHC 0/2] cn: w1: buffer size checks David Fries
2014-11-09 22:37 ` [PATCH 1/2] cn: verify msg->len before making callback David Fries
2014-11-09 22:37 ` [PATCH 2/2] w1: avoid potential u16 overflow David Fries
2014-11-10 0:12 ` [PATHC 0/2] cn: w1: buffer size checks Евгений Поляков
2014-11-11 2:19 ` David Fries
2014-11-11 2:19 ` [PATCH 1/2] cn: verify msg->len before making callback David Fries
2014-11-11 2:19 ` [PATCH 2/2] w1: avoid potential u16 overflow David Fries
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Powered by JetHome