cn: w1: buffer size checks
Subsystems (2)
cover cn: w1: buffer size checks
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
PATCH 1/2 cn: verify msg->len before making callback
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.
--- drivers/connector/connector.c | 6 ++++++ 1 file changed, 6 insertions(+)
drivers/connector/connector.c +6 -0
@@ -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)) {
Signature · 1 line
1.7.10.4
PATCH 2/2 w1: avoid potential u16 overflow
In reply to David Fries--- drivers/w1/w1_netlink.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/w1/w1_netlink.c +1 -1
@@ -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) +
Signature · 1 line
1.7.10.4
Hi
10.11.2014, 01:37, "David Fries" <[email protected]>:
Quoted 5 lines
These issues were found by Dan Carpenter with a static checker. Evgeniy are you okay with these?
[PATCH 1/2] cn: verify
msg->lenbefore making callback [PATCH 2/2] w1: avoid potential u16 overflow
Both patches look good, thank you
cover cn: w1: buffer size checks
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
PATCH 1/2 cn: verify msg->len before making callback
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.
--- drivers/connector/connector.c | 6 ++++++ 1 file changed, 6 insertions(+)
drivers/connector/connector.c +6 -0
@@ -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)) {
Signature · 1 line
1.7.10.4
PATCH 2/2 w1: avoid potential u16 overflow
In reply to David Fries--- drivers/w1/w1_netlink.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/w1/w1_netlink.c +1 -1
@@ -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) +
Signature · 1 line
1.7.10.4