[PATCH 0/2] [SRU] [T] Fix for CVE-2017-15649

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH 0/2] [SRU] [T] Fix for CVE-2017-15649

Paolo Pisati-5
https://people.canonical.com/~ubuntu-security/cve/2017/CVE-2017-15649.html

Trusty only, some manual modifications were due.

Willem de Bruijn (2):
  packet: hold bind lock when rebinding to fanout hook
  packet: in packet_do_bind, test fanout with bind_lock held

 net/packet/af_packet.c | 26 +++++++++++++++++---------
 1 file changed, 17 insertions(+), 9 deletions(-)

--
2.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[PATCH 1/2] packet: hold bind lock when rebinding to fanout hook

Paolo Pisati-5
From: Willem de Bruijn <[hidden email]>

Packet socket bind operations must hold the po->bind_lock. This keeps
po->running consistent with whether the socket is actually on a ptype
list to receive packets.

fanout_add unbinds a socket and its packet_rcv/tpacket_rcv call, then
binds the fanout object to receive through packet_rcv_fanout.

Make it hold the po->bind_lock when testing po->running and rebinding.
Else, it can race with other rebind operations, such as that in
packet_set_ring from packet_rcv to tpacket_rcv. Concurrent updates
can result in a socket being added to a fanout group twice, causing
use-after-free KASAN bug reports, among others.

Reported independently by both trinity and syzkaller.
Verified that the syzkaller reproducer passes after this patch.

Fixes: dc99f600698d ("packet: Add fanout support.")
Reported-by: nixioaming <[hidden email]>
Signed-off-by: Willem de Bruijn <[hidden email]>
Signed-off-by: David S. Miller <[hidden email]>
(cherry picked from commit 008ba2a13f2d04c947adc536d19debb8fe66f110)
Signed-off-by: Paolo Pisati <[hidden email]>
---
 net/packet/af_packet.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index 59a1558..2a07eb2 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -1317,10 +1317,6 @@ static int fanout_add(struct sock *sk, u16 id, u16 type_flags)
 
  mutex_lock(&fanout_mutex);
 
- err = -EINVAL;
- if (!po->running)
- goto out;
-
  err = -EALREADY;
  if (po->fanout)
  goto out;
@@ -1358,7 +1354,10 @@ static int fanout_add(struct sock *sk, u16 id, u16 type_flags)
  list_add(&match->list, &fanout_list);
  }
  err = -EINVAL;
- if (match->type == type &&
+
+ spin_lock(&po->bind_lock);
+ if (po->running &&
+    match->type == type &&
     match->prot_hook.type == po->prot_hook.type &&
     match->prot_hook.dev == po->prot_hook.dev) {
  err = -ENOSPC;
@@ -1370,6 +1369,13 @@ static int fanout_add(struct sock *sk, u16 id, u16 type_flags)
  err = 0;
  }
  }
+ spin_unlock(&po->bind_lock);
+
+ if (err && !atomic_read(&match->sk_ref)) {
+ list_del(&match->list);
+ kfree(match);
+ }
+
 out:
  mutex_unlock(&fanout_mutex);
  return err;
--
2.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[PATCH 2/2] packet: in packet_do_bind, test fanout with bind_lock held

Paolo Pisati-5
In reply to this post by Paolo Pisati-5
From: Willem de Bruijn <[hidden email]>

Once a socket has po->fanout set, it remains a member of the group
until it is destroyed. The prot_hook must be constant and identical
across sockets in the group.

If fanout_add races with packet_do_bind between the test of po->fanout
and taking the lock, the bind call may make type or dev inconsistent
with that of the fanout group.

Hold po->bind_lock when testing po->fanout to avoid this race.

I had to introduce artificial delay (local_bh_enable) to actually
observe the race.

Fixes: dc99f600698d ("packet: Add fanout support.")
Signed-off-by: Willem de Bruijn <[hidden email]>
Reviewed-by: Eric Dumazet <[hidden email]>
Signed-off-by: David S. Miller <[hidden email]>
(cherry picked from commit 4971613c1639d8e5f102c4e797c3bf8f83a5a69e)
Signed-off-by: Paolo Pisati <[hidden email]>
---
 net/packet/af_packet.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index 2a07eb2..c0230c7 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -2514,16 +2514,18 @@ static int packet_do_bind(struct sock *sk, struct net_device *dev, __be16 protoc
 {
  struct packet_sock *po = pkt_sk(sk);
 
+ lock_sock(sk);
+
+ spin_lock(&po->bind_lock);
+
  if (po->fanout) {
+ spin_unlock(&po->bind_lock);
+ release_sock(sk);
  if (dev)
  dev_put(dev);
-
  return -EINVAL;
  }
 
- lock_sock(sk);
-
- spin_lock(&po->bind_lock);
  unregister_prot_hook(sk, true);
 
  po->num = protocol;
--
2.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

ACK: [PATCH 0/2] [SRU] [T] Fix for CVE-2017-15649

Stefan Bader-2
In reply to this post by Paolo Pisati-5
On 10.07.2018 13:18, Paolo Pisati wrote:

> https://people.canonical.com/~ubuntu-security/cve/2017/CVE-2017-15649.html
>
> Trusty only, some manual modifications were due.
>
> Willem de Bruijn (2):
>   packet: hold bind lock when rebinding to fanout hook
>   packet: in packet_do_bind, test fanout with bind_lock held
>
>  net/packet/af_packet.c | 26 +++++++++++++++++---------
>  1 file changed, 17 insertions(+), 9 deletions(-)
>
Acked-by: Stefan Bader <[hidden email]>


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

signature.asc (836 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

ACK/APPLIED/cmnt: [PATCH 0/2] [SRU] [T] Fix for CVE-2017-15649

Juerg Haefliger
In reply to this post by Paolo Pisati-5
Added missing CVE lines.


On 07/10/2018 01:18 PM, Paolo Pisati wrote:
> https://people.canonical.com/~ubuntu-security/cve/2017/CVE-2017-15649.html
>
> Trusty only, some manual modifications were due.

These are not clean cherry picks so 'cherry picked from ..' should be
replaced with 'backported from' plus some comments about what the
changes are would be helpful.

Acked-by: Juerg Haefliger <[hidden email]>

Applied to trusty master-next.

...Juerg


> Willem de Bruijn (2):
>   packet: hold bind lock when rebinding to fanout hook
>   packet: in packet_do_bind, test fanout with bind_lock held
>
>  net/packet/af_packet.c | 26 +++++++++++++++++---------
>  1 file changed, 17 insertions(+), 9 deletions(-)
>



--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

signature.asc (849 bytes) Download Attachment