| Message ID | 20140328142914.GB25962@hansolo.jdub.homelinux.org |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | David Miller |
| Headers | show |
On Fri, 2014-03-28 at 10:29 -0400, Josh Boyer wrote: > Backport of upstream commit 36d5fe6a0007 to 3.13.y > > nfqnl_zcopy can copy elements of the frags array between skbs, but it doesn't > orphan them. Also, it doesn't handle errors, so this patch takes care of that > as well, and modify the caller accordingly. skb_tx_error() is also added to > the callers so they will signal the failed delivery towards the creator of the > skb. > > Fixes CVE-2014-2568. > > Signed-off-by: Zoltan Kiss <zoltan.kiss@citrix.com> > Signed-off-by: David S. Miller <davem@davemloft.net> > Signed-off-by: Josh Boyer <jwboyer@fedoraproject.org> [...] FWIW, I applied the same change to Debian's 3.13.7-1, except for leaving 'from' as pointer-to-const. Ben.
On Sun, Mar 30, 2014 at 7:56 PM, Ben Hutchings <ben@decadent.org.uk> wrote: > On Fri, 2014-03-28 at 10:29 -0400, Josh Boyer wrote: >> Backport of upstream commit 36d5fe6a0007 to 3.13.y >> >> nfqnl_zcopy can copy elements of the frags array between skbs, but it doesn't >> orphan them. Also, it doesn't handle errors, so this patch takes care of that >> as well, and modify the caller accordingly. skb_tx_error() is also added to >> the callers so they will signal the failed delivery towards the creator of the >> skb. >> >> Fixes CVE-2014-2568. >> >> Signed-off-by: Zoltan Kiss <zoltan.kiss@citrix.com> >> Signed-off-by: David S. Miller <davem@davemloft.net> >> Signed-off-by: Josh Boyer <jwboyer@fedoraproject.org> > [...] > > FWIW, I applied the same change to Debian's 3.13.7-1, except for leaving > 'from' as pointer-to-const. Thanks for the confirmation, Ben. I noticed Dave already has the original patch in his stable bundle on patchwork, so perhaps he's going to do his own backport. (Though he has v4 in the bundle and v5 is what went in-tree.) I figured I would just send mine along in case it was helpful. josh -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/netfilter/nfnetlink_queue_core.c b/net/netfilter/nfnetlink_queue_core.c index 21258cf..b241654 100644 --- a/net/netfilter/nfnetlink_queue_core.c +++ b/net/netfilter/nfnetlink_queue_core.c @@ -235,22 +235,23 @@ nfqnl_flush(struct nfqnl_instance *queue, nfqnl_cmpfn cmpfn, unsigned long data) spin_unlock_bh(&queue->lock); } -static void -nfqnl_zcopy(struct sk_buff *to, const struct sk_buff *from, int len, int hlen) +static int +nfqnl_zcopy(struct sk_buff *to, struct sk_buff *from, int len, int hlen) { int i, j = 0; int plen = 0; /* length of skb->head fragment */ + int ret; struct page *page; unsigned int offset; /* dont bother with small payloads */ - if (len <= skb_tailroom(to)) { - skb_copy_bits(from, 0, skb_put(to, len), len); - return; - } + if (len <= skb_tailroom(to)) + return skb_copy_bits(from, 0, skb_put(to, len), len); if (hlen) { - skb_copy_bits(from, 0, skb_put(to, hlen), hlen); + ret = skb_copy_bits(from, 0, skb_put(to, hlen), hlen); + if (unlikely(ret)) + return ret; len -= hlen; } else { plen = min_t(int, skb_headlen(from), len); @@ -268,6 +269,11 @@ nfqnl_zcopy(struct sk_buff *to, const struct sk_buff *from, int len, int hlen) to->len += len + plen; to->data_len += len + plen; + if (unlikely(skb_orphan_frags(from, GFP_ATOMIC))) { + skb_tx_error(from); + return -ENOMEM; + } + for (i = 0; i < skb_shinfo(from)->nr_frags; i++) { if (!len) break; @@ -278,6 +284,8 @@ nfqnl_zcopy(struct sk_buff *to, const struct sk_buff *from, int len, int hlen) j++; } skb_shinfo(to)->nr_frags = j; + + return 0; } static int @@ -374,13 +382,16 @@ nfqnl_build_packet_message(struct net *net, struct nfqnl_instance *queue, skb = nfnetlink_alloc_skb(net, size, queue->peer_portid, GFP_ATOMIC); - if (!skb) + if (!skb) { + skb_tx_error(entskb); return NULL; + } nlh = nlmsg_put(skb, 0, 0, NFNL_SUBSYS_QUEUE << 8 | NFQNL_MSG_PACKET, sizeof(struct nfgenmsg), 0); if (!nlh) { + skb_tx_error(entskb); kfree_skb(skb); return NULL; } @@ -504,13 +515,15 @@ nfqnl_build_packet_message(struct net *net, struct nfqnl_instance *queue, nla->nla_type = NFQA_PAYLOAD; nla->nla_len = nla_attr_size(data_len); - nfqnl_zcopy(skb, entskb, data_len, hlen); + if (nfqnl_zcopy(skb, entskb, data_len, hlen)) + goto nla_put_failure; } nlh->nlmsg_len = skb->len; return skb; nla_put_failure: + skb_tx_error(entskb); kfree_skb(skb); net_err_ratelimited("nf_queue: error creating packet message\n"); return NULL;