| Message ID | 20170907130233.30902-1-kleber.souza@canonical.com |
|---|---|
| State | New |
| Headers | show |
| Series | [Trusty,SRU,CVE-2016-8633] firewire: net: guard against rx buffer overflows | expand |
On 07.09.2017 15:02, Kleber Sacilotto de Souza wrote: > From: Stefan Richter <stefanr@s5r6.in-berlin.de> > > CVE-2016-8633 > > The IP-over-1394 driver firewire-net lacked input validation when > handling incoming fragmented datagrams. A maliciously formed fragment > with a respectively large datagram_offset would cause a memcpy past the > datagram buffer. > > So, drop any packets carrying a fragment with offset + length larger > than datagram_size. > > In addition, ensure that > - GASP header, unfragmented encapsulation header, or fragment > encapsulation header actually exists before we access it, > - the encapsulated datagram or fragment is of nonzero size. > > Reported-by: Eyal Itkin <eyal.itkin@gmail.com> > Reviewed-by: Eyal Itkin <eyal.itkin@gmail.com> > Fixes: CVE 2016-8633 > Cc: stable@vger.kernel.org > Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> > (cherry picked from commit 667121ace9dbafb368618dbabcf07901c962ddac) > Signed-off-by: Kleber Sacilotto de Souza <kleber.souza@canonical.com> Acked-by: Stefan Bader <stefan.bader@canonical.com> > --- > > Notes: > Only Trusty still needs the fix for this CVE. Cherry pick applies cleanly, > compile tested. > > Kleber > > drivers/firewire/net.c | 51 ++++++++++++++++++++++++++++++++++---------------- > 1 file changed, 35 insertions(+), 16 deletions(-) > > diff --git a/drivers/firewire/net.c b/drivers/firewire/net.c > index 4af0a7bad7f2..641eeab43c57 100644 > --- a/drivers/firewire/net.c > +++ b/drivers/firewire/net.c > @@ -591,6 +591,9 @@ static int fwnet_incoming_packet(struct fwnet_device *dev, __be32 *buf, int len, > int retval; > u16 ether_type; > > + if (len <= RFC2374_UNFRAG_HDR_SIZE) > + return 0; > + > hdr.w0 = be32_to_cpu(buf[0]); > lf = fwnet_get_hdr_lf(&hdr); > if (lf == RFC2374_HDR_UNFRAG) { > @@ -615,7 +618,12 @@ static int fwnet_incoming_packet(struct fwnet_device *dev, __be32 *buf, int len, > return fwnet_finish_incoming_packet(net, skb, source_node_id, > is_broadcast, ether_type); > } > + > /* A datagram fragment has been received, now the fun begins. */ > + > + if (len <= RFC2374_FRAG_HDR_SIZE) > + return 0; > + > hdr.w1 = ntohl(buf[1]); > buf += 2; > len -= RFC2374_FRAG_HDR_SIZE; > @@ -629,6 +637,9 @@ static int fwnet_incoming_packet(struct fwnet_device *dev, __be32 *buf, int len, > datagram_label = fwnet_get_hdr_dgl(&hdr); > dg_size = fwnet_get_hdr_dg_size(&hdr); /* ??? + 1 */ > > + if (fg_off + len > dg_size) > + return 0; > + > spin_lock_irqsave(&dev->lock, flags); > > peer = fwnet_peer_find_by_node_id(dev, source_node_id, generation); > @@ -735,6 +746,22 @@ static void fwnet_receive_packet(struct fw_card *card, struct fw_request *r, > fw_send_response(card, r, rcode); > } > > +static int gasp_source_id(__be32 *p) > +{ > + return be32_to_cpu(p[0]) >> 16; > +} > + > +static u32 gasp_specifier_id(__be32 *p) > +{ > + return (be32_to_cpu(p[0]) & 0xffff) << 8 | > + (be32_to_cpu(p[1]) & 0xff000000) >> 24; > +} > + > +static u32 gasp_version(__be32 *p) > +{ > + return be32_to_cpu(p[1]) & 0xffffff; > +} > + > static void fwnet_receive_broadcast(struct fw_iso_context *context, > u32 cycle, size_t header_length, void *header, void *data) > { > @@ -744,9 +771,6 @@ static void fwnet_receive_broadcast(struct fw_iso_context *context, > __be32 *buf_ptr; > int retval; > u32 length; > - u16 source_node_id; > - u32 specifier_id; > - u32 ver; > unsigned long offset; > unsigned long flags; > > @@ -763,22 +787,17 @@ static void fwnet_receive_broadcast(struct fw_iso_context *context, > > spin_unlock_irqrestore(&dev->lock, flags); > > - specifier_id = (be32_to_cpu(buf_ptr[0]) & 0xffff) << 8 > - | (be32_to_cpu(buf_ptr[1]) & 0xff000000) >> 24; > - ver = be32_to_cpu(buf_ptr[1]) & 0xffffff; > - source_node_id = be32_to_cpu(buf_ptr[0]) >> 16; > - > - if (specifier_id == IANA_SPECIFIER_ID && > - (ver == RFC2734_SW_VERSION > + if (length > IEEE1394_GASP_HDR_SIZE && > + gasp_specifier_id(buf_ptr) == IANA_SPECIFIER_ID && > + (gasp_version(buf_ptr) == RFC2734_SW_VERSION > #if IS_ENABLED(CONFIG_IPV6) > - || ver == RFC3146_SW_VERSION > + || gasp_version(buf_ptr) == RFC3146_SW_VERSION > #endif > - )) { > - buf_ptr += 2; > - length -= IEEE1394_GASP_HDR_SIZE; > - fwnet_incoming_packet(dev, buf_ptr, length, source_node_id, > + )) > + fwnet_incoming_packet(dev, buf_ptr + 2, > + length - IEEE1394_GASP_HDR_SIZE, > + gasp_source_id(buf_ptr), > context->card->generation, true); > - } > > packet.payload_length = dev->rcv_buffer_size; > packet.interrupt = 1; >
On 07/09/17 14:02, Kleber Sacilotto de Souza wrote: > From: Stefan Richter <stefanr@s5r6.in-berlin.de> > > CVE-2016-8633 > > The IP-over-1394 driver firewire-net lacked input validation when > handling incoming fragmented datagrams. A maliciously formed fragment > with a respectively large datagram_offset would cause a memcpy past the > datagram buffer. > > So, drop any packets carrying a fragment with offset + length larger > than datagram_size. > > In addition, ensure that > - GASP header, unfragmented encapsulation header, or fragment > encapsulation header actually exists before we access it, > - the encapsulated datagram or fragment is of nonzero size. > > Reported-by: Eyal Itkin <eyal.itkin@gmail.com> > Reviewed-by: Eyal Itkin <eyal.itkin@gmail.com> > Fixes: CVE 2016-8633 > Cc: stable@vger.kernel.org > Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> > (cherry picked from commit 667121ace9dbafb368618dbabcf07901c962ddac) > Signed-off-by: Kleber Sacilotto de Souza <kleber.souza@canonical.com> > --- > > Notes: > Only Trusty still needs the fix for this CVE. Cherry pick applies cleanly, > compile tested. > > Kleber > > drivers/firewire/net.c | 51 ++++++++++++++++++++++++++++++++++---------------- > 1 file changed, 35 insertions(+), 16 deletions(-) > > diff --git a/drivers/firewire/net.c b/drivers/firewire/net.c > index 4af0a7bad7f2..641eeab43c57 100644 > --- a/drivers/firewire/net.c > +++ b/drivers/firewire/net.c > @@ -591,6 +591,9 @@ static int fwnet_incoming_packet(struct fwnet_device *dev, __be32 *buf, int len, > int retval; > u16 ether_type; > > + if (len <= RFC2374_UNFRAG_HDR_SIZE) > + return 0; > + > hdr.w0 = be32_to_cpu(buf[0]); > lf = fwnet_get_hdr_lf(&hdr); > if (lf == RFC2374_HDR_UNFRAG) { > @@ -615,7 +618,12 @@ static int fwnet_incoming_packet(struct fwnet_device *dev, __be32 *buf, int len, > return fwnet_finish_incoming_packet(net, skb, source_node_id, > is_broadcast, ether_type); > } > + > /* A datagram fragment has been received, now the fun begins. */ > + > + if (len <= RFC2374_FRAG_HDR_SIZE) > + return 0; > + > hdr.w1 = ntohl(buf[1]); > buf += 2; > len -= RFC2374_FRAG_HDR_SIZE; > @@ -629,6 +637,9 @@ static int fwnet_incoming_packet(struct fwnet_device *dev, __be32 *buf, int len, > datagram_label = fwnet_get_hdr_dgl(&hdr); > dg_size = fwnet_get_hdr_dg_size(&hdr); /* ??? + 1 */ > > + if (fg_off + len > dg_size) > + return 0; > + > spin_lock_irqsave(&dev->lock, flags); > > peer = fwnet_peer_find_by_node_id(dev, source_node_id, generation); > @@ -735,6 +746,22 @@ static void fwnet_receive_packet(struct fw_card *card, struct fw_request *r, > fw_send_response(card, r, rcode); > } > > +static int gasp_source_id(__be32 *p) > +{ > + return be32_to_cpu(p[0]) >> 16; > +} > + > +static u32 gasp_specifier_id(__be32 *p) > +{ > + return (be32_to_cpu(p[0]) & 0xffff) << 8 | > + (be32_to_cpu(p[1]) & 0xff000000) >> 24; > +} > + > +static u32 gasp_version(__be32 *p) > +{ > + return be32_to_cpu(p[1]) & 0xffffff; > +} > + > static void fwnet_receive_broadcast(struct fw_iso_context *context, > u32 cycle, size_t header_length, void *header, void *data) > { > @@ -744,9 +771,6 @@ static void fwnet_receive_broadcast(struct fw_iso_context *context, > __be32 *buf_ptr; > int retval; > u32 length; > - u16 source_node_id; > - u32 specifier_id; > - u32 ver; > unsigned long offset; > unsigned long flags; > > @@ -763,22 +787,17 @@ static void fwnet_receive_broadcast(struct fw_iso_context *context, > > spin_unlock_irqrestore(&dev->lock, flags); > > - specifier_id = (be32_to_cpu(buf_ptr[0]) & 0xffff) << 8 > - | (be32_to_cpu(buf_ptr[1]) & 0xff000000) >> 24; > - ver = be32_to_cpu(buf_ptr[1]) & 0xffffff; > - source_node_id = be32_to_cpu(buf_ptr[0]) >> 16; > - > - if (specifier_id == IANA_SPECIFIER_ID && > - (ver == RFC2734_SW_VERSION > + if (length > IEEE1394_GASP_HDR_SIZE && > + gasp_specifier_id(buf_ptr) == IANA_SPECIFIER_ID && > + (gasp_version(buf_ptr) == RFC2734_SW_VERSION > #if IS_ENABLED(CONFIG_IPV6) > - || ver == RFC3146_SW_VERSION > + || gasp_version(buf_ptr) == RFC3146_SW_VERSION > #endif > - )) { > - buf_ptr += 2; > - length -= IEEE1394_GASP_HDR_SIZE; > - fwnet_incoming_packet(dev, buf_ptr, length, source_node_id, > + )) > + fwnet_incoming_packet(dev, buf_ptr + 2, > + length - IEEE1394_GASP_HDR_SIZE, > + gasp_source_id(buf_ptr), > context->card->generation, true); > - } > > packet.payload_length = dev->rcv_buffer_size; > packet.interrupt = 1; > Clean cherry pick Acked-by: Colin Ian King <colin.king@canonical.com>
On 07.09.2017 15:02, Kleber Sacilotto de Souza wrote: > From: Stefan Richter <stefanr@s5r6.in-berlin.de> > > CVE-2016-8633 > > The IP-over-1394 driver firewire-net lacked input validation when > handling incoming fragmented datagrams. A maliciously formed fragment > with a respectively large datagram_offset would cause a memcpy past the > datagram buffer. > > So, drop any packets carrying a fragment with offset + length larger > than datagram_size. > > In addition, ensure that > - GASP header, unfragmented encapsulation header, or fragment > encapsulation header actually exists before we access it, > - the encapsulated datagram or fragment is of nonzero size. > > Reported-by: Eyal Itkin <eyal.itkin@gmail.com> > Reviewed-by: Eyal Itkin <eyal.itkin@gmail.com> > Fixes: CVE 2016-8633 > Cc: stable@vger.kernel.org > Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> > (cherry picked from commit 667121ace9dbafb368618dbabcf07901c962ddac) > Signed-off-by: Kleber Sacilotto de Souza <kleber.souza@canonical.com> > --- > > Notes: > Only Trusty still needs the fix for this CVE. Cherry pick applies cleanly, > compile tested. > > Kleber > > drivers/firewire/net.c | 51 ++++++++++++++++++++++++++++++++++---------------- > 1 file changed, 35 insertions(+), 16 deletions(-) > > diff --git a/drivers/firewire/net.c b/drivers/firewire/net.c > index 4af0a7bad7f2..641eeab43c57 100644 > --- a/drivers/firewire/net.c > +++ b/drivers/firewire/net.c > @@ -591,6 +591,9 @@ static int fwnet_incoming_packet(struct fwnet_device *dev, __be32 *buf, int len, > int retval; > u16 ether_type; > > + if (len <= RFC2374_UNFRAG_HDR_SIZE) > + return 0; > + > hdr.w0 = be32_to_cpu(buf[0]); > lf = fwnet_get_hdr_lf(&hdr); > if (lf == RFC2374_HDR_UNFRAG) { > @@ -615,7 +618,12 @@ static int fwnet_incoming_packet(struct fwnet_device *dev, __be32 *buf, int len, > return fwnet_finish_incoming_packet(net, skb, source_node_id, > is_broadcast, ether_type); > } > + > /* A datagram fragment has been received, now the fun begins. */ > + > + if (len <= RFC2374_FRAG_HDR_SIZE) > + return 0; > + > hdr.w1 = ntohl(buf[1]); > buf += 2; > len -= RFC2374_FRAG_HDR_SIZE; > @@ -629,6 +637,9 @@ static int fwnet_incoming_packet(struct fwnet_device *dev, __be32 *buf, int len, > datagram_label = fwnet_get_hdr_dgl(&hdr); > dg_size = fwnet_get_hdr_dg_size(&hdr); /* ??? + 1 */ > > + if (fg_off + len > dg_size) > + return 0; > + > spin_lock_irqsave(&dev->lock, flags); > > peer = fwnet_peer_find_by_node_id(dev, source_node_id, generation); > @@ -735,6 +746,22 @@ static void fwnet_receive_packet(struct fw_card *card, struct fw_request *r, > fw_send_response(card, r, rcode); > } > > +static int gasp_source_id(__be32 *p) > +{ > + return be32_to_cpu(p[0]) >> 16; > +} > + > +static u32 gasp_specifier_id(__be32 *p) > +{ > + return (be32_to_cpu(p[0]) & 0xffff) << 8 | > + (be32_to_cpu(p[1]) & 0xff000000) >> 24; > +} > + > +static u32 gasp_version(__be32 *p) > +{ > + return be32_to_cpu(p[1]) & 0xffffff; > +} > + > static void fwnet_receive_broadcast(struct fw_iso_context *context, > u32 cycle, size_t header_length, void *header, void *data) > { > @@ -744,9 +771,6 @@ static void fwnet_receive_broadcast(struct fw_iso_context *context, > __be32 *buf_ptr; > int retval; > u32 length; > - u16 source_node_id; > - u32 specifier_id; > - u32 ver; > unsigned long offset; > unsigned long flags; > > @@ -763,22 +787,17 @@ static void fwnet_receive_broadcast(struct fw_iso_context *context, > > spin_unlock_irqrestore(&dev->lock, flags); > > - specifier_id = (be32_to_cpu(buf_ptr[0]) & 0xffff) << 8 > - | (be32_to_cpu(buf_ptr[1]) & 0xff000000) >> 24; > - ver = be32_to_cpu(buf_ptr[1]) & 0xffffff; > - source_node_id = be32_to_cpu(buf_ptr[0]) >> 16; > - > - if (specifier_id == IANA_SPECIFIER_ID && > - (ver == RFC2734_SW_VERSION > + if (length > IEEE1394_GASP_HDR_SIZE && > + gasp_specifier_id(buf_ptr) == IANA_SPECIFIER_ID && > + (gasp_version(buf_ptr) == RFC2734_SW_VERSION > #if IS_ENABLED(CONFIG_IPV6) > - || ver == RFC3146_SW_VERSION > + || gasp_version(buf_ptr) == RFC3146_SW_VERSION > #endif > - )) { > - buf_ptr += 2; > - length -= IEEE1394_GASP_HDR_SIZE; > - fwnet_incoming_packet(dev, buf_ptr, length, source_node_id, > + )) > + fwnet_incoming_packet(dev, buf_ptr + 2, > + length - IEEE1394_GASP_HDR_SIZE, > + gasp_source_id(buf_ptr), > context->card->generation, true); > - } > > packet.payload_length = dev->rcv_buffer_size; > packet.interrupt = 1; > Applied to Trusty master-next
diff --git a/drivers/firewire/net.c b/drivers/firewire/net.c index 4af0a7bad7f2..641eeab43c57 100644 --- a/drivers/firewire/net.c +++ b/drivers/firewire/net.c @@ -591,6 +591,9 @@ static int fwnet_incoming_packet(struct fwnet_device *dev, __be32 *buf, int len, int retval; u16 ether_type; + if (len <= RFC2374_UNFRAG_HDR_SIZE) + return 0; + hdr.w0 = be32_to_cpu(buf[0]); lf = fwnet_get_hdr_lf(&hdr); if (lf == RFC2374_HDR_UNFRAG) { @@ -615,7 +618,12 @@ static int fwnet_incoming_packet(struct fwnet_device *dev, __be32 *buf, int len, return fwnet_finish_incoming_packet(net, skb, source_node_id, is_broadcast, ether_type); } + /* A datagram fragment has been received, now the fun begins. */ + + if (len <= RFC2374_FRAG_HDR_SIZE) + return 0; + hdr.w1 = ntohl(buf[1]); buf += 2; len -= RFC2374_FRAG_HDR_SIZE; @@ -629,6 +637,9 @@ static int fwnet_incoming_packet(struct fwnet_device *dev, __be32 *buf, int len, datagram_label = fwnet_get_hdr_dgl(&hdr); dg_size = fwnet_get_hdr_dg_size(&hdr); /* ??? + 1 */ + if (fg_off + len > dg_size) + return 0; + spin_lock_irqsave(&dev->lock, flags); peer = fwnet_peer_find_by_node_id(dev, source_node_id, generation); @@ -735,6 +746,22 @@ static void fwnet_receive_packet(struct fw_card *card, struct fw_request *r, fw_send_response(card, r, rcode); } +static int gasp_source_id(__be32 *p) +{ + return be32_to_cpu(p[0]) >> 16; +} + +static u32 gasp_specifier_id(__be32 *p) +{ + return (be32_to_cpu(p[0]) & 0xffff) << 8 | + (be32_to_cpu(p[1]) & 0xff000000) >> 24; +} + +static u32 gasp_version(__be32 *p) +{ + return be32_to_cpu(p[1]) & 0xffffff; +} + static void fwnet_receive_broadcast(struct fw_iso_context *context, u32 cycle, size_t header_length, void *header, void *data) { @@ -744,9 +771,6 @@ static void fwnet_receive_broadcast(struct fw_iso_context *context, __be32 *buf_ptr; int retval; u32 length; - u16 source_node_id; - u32 specifier_id; - u32 ver; unsigned long offset; unsigned long flags; @@ -763,22 +787,17 @@ static void fwnet_receive_broadcast(struct fw_iso_context *context, spin_unlock_irqrestore(&dev->lock, flags); - specifier_id = (be32_to_cpu(buf_ptr[0]) & 0xffff) << 8 - | (be32_to_cpu(buf_ptr[1]) & 0xff000000) >> 24; - ver = be32_to_cpu(buf_ptr[1]) & 0xffffff; - source_node_id = be32_to_cpu(buf_ptr[0]) >> 16; - - if (specifier_id == IANA_SPECIFIER_ID && - (ver == RFC2734_SW_VERSION + if (length > IEEE1394_GASP_HDR_SIZE && + gasp_specifier_id(buf_ptr) == IANA_SPECIFIER_ID && + (gasp_version(buf_ptr) == RFC2734_SW_VERSION #if IS_ENABLED(CONFIG_IPV6) - || ver == RFC3146_SW_VERSION + || gasp_version(buf_ptr) == RFC3146_SW_VERSION #endif - )) { - buf_ptr += 2; - length -= IEEE1394_GASP_HDR_SIZE; - fwnet_incoming_packet(dev, buf_ptr, length, source_node_id, + )) + fwnet_incoming_packet(dev, buf_ptr + 2, + length - IEEE1394_GASP_HDR_SIZE, + gasp_source_id(buf_ptr), context->card->generation, true); - } packet.payload_length = dev->rcv_buffer_size; packet.interrupt = 1;