[SRU][trusty][PATCH 0/2] Fix for CVE-2017-16939

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

[SRU][trusty][PATCH 0/2] Fix for CVE-2017-16939

Kleber Souza
Patch 2/2 (ipsec: Fix aborted xfrm policy dump crash) is the real fix and
it needed a simple backport to fix the context because it lacks
283bc9f35bbbc (xfrm: Namespacify xfrm state/policy locks). Patch 1/1 is a
prerequisite and is a clean cherry-pick.

Tested with the POC available on
https://bugzilla.suse.com/show_bug.cgi?id=1069702.

Herbert Xu (1):
  ipsec: Fix aborted xfrm policy dump crash

Tom Herbert (1):
  netlink: add a start callback for starting a netlink dump

 include/linux/netlink.h  |  2 ++
 include/net/genetlink.h  |  2 ++
 net/netlink/af_netlink.c |  4 ++++
 net/netlink/genetlink.c  | 16 ++++++++++++++++
 net/xfrm/xfrm_user.c     | 25 +++++++++++++++----------
 5 files changed, 39 insertions(+), 10 deletions(-)

--
2.14.1


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

[SRU][trusty][PATCH 1/2] netlink: add a start callback for starting a netlink dump

Kleber Souza
From: Tom Herbert <[hidden email]>

The start callback allows the caller to set up a context for the
dump callbacks. Presumably, the context can then be destroyed in
the done callback.

Signed-off-by: Tom Herbert <[hidden email]>
Signed-off-by: David S. Miller <[hidden email]>

CVE-2017-16939
(cherry picked from commit fc9e50f5a5a4e1fa9ba2756f745a13e693cf6a06)
Signed-off-by: Kleber Sacilotto de Souza <[hidden email]>
---
 include/linux/netlink.h  |  2 ++
 include/net/genetlink.h  |  2 ++
 net/netlink/af_netlink.c |  4 ++++
 net/netlink/genetlink.c  | 16 ++++++++++++++++
 4 files changed, 24 insertions(+)

diff --git a/include/linux/netlink.h b/include/linux/netlink.h
index 8b50a62ef98b..5676fd784080 100644
--- a/include/linux/netlink.h
+++ b/include/linux/netlink.h
@@ -120,6 +120,7 @@ netlink_skb_clone(struct sk_buff *skb, gfp_t gfp_mask)
 struct netlink_callback {
  struct sk_buff *skb;
  const struct nlmsghdr *nlh;
+ int (*start)(struct netlink_callback *);
  int (*dump)(struct sk_buff * skb,
  struct netlink_callback *cb);
  int (*done)(struct netlink_callback *cb);
@@ -142,6 +143,7 @@ struct nlmsghdr *
 __nlmsg_put(struct sk_buff *skb, u32 portid, u32 seq, int type, int len, int flags);
 
 struct netlink_dump_control {
+ int (*start)(struct netlink_callback *);
  int (*dump)(struct sk_buff *skb, struct netlink_callback *);
  int (*done)(struct netlink_callback *);
  void *data;
diff --git a/include/net/genetlink.h b/include/net/genetlink.h
index 1b177ed803b7..8dbcc76d06f5 100644
--- a/include/net/genetlink.h
+++ b/include/net/genetlink.h
@@ -104,6 +104,7 @@ static inline void genl_info_net_set(struct genl_info *info, struct net *net)
  * @flags: flags
  * @policy: attribute validation policy
  * @doit: standard command callback
+ * @start: start callback for dumps
  * @dumpit: callback for dumpers
  * @done: completion callback for dumps
  * @ops_list: operations list
@@ -112,6 +113,7 @@ struct genl_ops {
  const struct nla_policy *policy;
  int       (*doit)(struct sk_buff *skb,
        struct genl_info *info);
+ int       (*start)(struct netlink_callback *cb);
  int       (*dumpit)(struct sk_buff *skb,
  struct netlink_callback *cb);
  int       (*done)(struct netlink_callback *cb);
diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index 0038f9bb8a09..8af1f82802e3 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -2787,6 +2787,7 @@ int __netlink_dump_start(struct sock *ssk, struct sk_buff *skb,
 
  cb = &nlk->cb;
  memset(cb, 0, sizeof(*cb));
+ cb->start = control->start;
  cb->dump = control->dump;
  cb->done = control->done;
  cb->nlh = nlh;
@@ -2799,6 +2800,9 @@ int __netlink_dump_start(struct sock *ssk, struct sk_buff *skb,
 
  mutex_unlock(nlk->cb_mutex);
 
+ if (cb->start)
+ cb->start(cb);
+
  ret = netlink_dump(sk);
  sock_put(sk);
 
diff --git a/net/netlink/genetlink.c b/net/netlink/genetlink.c
index 5e4af16d5ada..e548711899c9 100644
--- a/net/netlink/genetlink.c
+++ b/net/netlink/genetlink.c
@@ -491,6 +491,20 @@ void *genlmsg_put(struct sk_buff *skb, u32 portid, u32 seq,
 }
 EXPORT_SYMBOL(genlmsg_put);
 
+static int genl_lock_start(struct netlink_callback *cb)
+{
+ /* our ops are always const - netlink API doesn't propagate that */
+ const struct genl_ops *ops = cb->data;
+ int rc = 0;
+
+ if (ops->start) {
+ genl_lock();
+ rc = ops->start(cb);
+ genl_unlock();
+ }
+ return rc;
+}
+
 static int genl_lock_dumpit(struct sk_buff *skb, struct netlink_callback *cb)
 {
  /* our ops are always const - netlink API doesn't propagate that */
@@ -555,6 +569,7 @@ static int genl_family_rcv_msg(struct genl_family *family,
  .module = family->module,
  /* we have const, but the netlink API doesn't */
  .data = (void *)ops,
+ .start = genl_lock_start,
  .dump = genl_lock_dumpit,
  .done = genl_lock_done,
  };
@@ -566,6 +581,7 @@ static int genl_family_rcv_msg(struct genl_family *family,
  } else {
  struct netlink_dump_control c = {
  .module = family->module,
+ .start = ops->start,
  .dump = ops->dumpit,
  .done = ops->done,
  };
--
2.14.1


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

[SRU][trusty][PATCH 2/2] ipsec: Fix aborted xfrm policy dump crash

Kleber Souza
In reply to this post by Kleber Souza
From: Herbert Xu <[hidden email]>

An independent security researcher, Mohamed Ghannam, has reported
this vulnerability to Beyond Security's SecuriTeam Secure Disclosure
program.

The xfrm_dump_policy_done function expects xfrm_dump_policy to
have been called at least once or it will crash.  This can be
triggered if a dump fails because the target socket's receive
buffer is full.

This patch fixes it by using the cb->start mechanism to ensure that
the initialisation is always done regardless of the buffer situation.

Fixes: 12a169e7d8f4 ("ipsec: Put dumpers on the dump list")
Signed-off-by: Herbert Xu <[hidden email]>
Signed-off-by: Steffen Klassert <[hidden email]>

CVE-2017-16939
(backported from commit 1137b5e2529a8f5ca8ee709288ecba3e68044df2)
[kleber.souza: adjusted for context because it lacks 283bc9f35bbbc
 (xfrm: Namespacify xfrm state/policy locks)]
Signed-off-by: Kleber Sacilotto de Souza <[hidden email]>
---
 net/xfrm/xfrm_user.c | 25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index 258f8243d077..1cf8c9a66aa4 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -1553,31 +1553,33 @@ static int dump_one_policy(struct xfrm_policy *xp, int dir, int count, void *ptr
 
 static int xfrm_dump_policy_done(struct netlink_callback *cb)
 {
- struct xfrm_policy_walk *walk = (struct xfrm_policy_walk *) &cb->args[1];
+ struct xfrm_policy_walk *walk = (struct xfrm_policy_walk *)cb->args;
 
  xfrm_policy_walk_done(walk);
  return 0;
 }
 
+static int xfrm_dump_policy_start(struct netlink_callback *cb)
+{
+ struct xfrm_policy_walk *walk = (struct xfrm_policy_walk *)cb->args;
+
+ BUILD_BUG_ON(sizeof(*walk) > sizeof(cb->args));
+
+ xfrm_policy_walk_init(walk, XFRM_POLICY_TYPE_ANY);
+ return 0;
+}
+
 static int xfrm_dump_policy(struct sk_buff *skb, struct netlink_callback *cb)
 {
  struct net *net = sock_net(skb->sk);
- struct xfrm_policy_walk *walk = (struct xfrm_policy_walk *) &cb->args[1];
+ struct xfrm_policy_walk *walk = (struct xfrm_policy_walk *)cb->args;
  struct xfrm_dump_info info;
 
- BUILD_BUG_ON(sizeof(struct xfrm_policy_walk) >
-     sizeof(cb->args) - sizeof(cb->args[0]));
-
  info.in_skb = cb->skb;
  info.out_skb = skb;
  info.nlmsg_seq = cb->nlh->nlmsg_seq;
  info.nlmsg_flags = NLM_F_MULTI;
 
- if (!cb->args[0]) {
- cb->args[0] = 1;
- xfrm_policy_walk_init(walk, XFRM_POLICY_TYPE_ANY);
- }
-
  (void) xfrm_policy_walk(net, walk, dump_one_policy, &info);
 
  return skb->len;
@@ -2327,6 +2329,7 @@ static const struct nla_policy xfrma_policy[XFRMA_MAX+1] = {
 
 static const struct xfrm_link {
  int (*doit)(struct sk_buff *, struct nlmsghdr *, struct nlattr **);
+ int (*start)(struct netlink_callback *);
  int (*dump)(struct sk_buff *, struct netlink_callback *);
  int (*done)(struct netlink_callback *);
 } xfrm_dispatch[XFRM_NR_MSGTYPES] = {
@@ -2338,6 +2341,7 @@ static const struct xfrm_link {
  [XFRM_MSG_NEWPOLICY   - XFRM_MSG_BASE] = { .doit = xfrm_add_policy    },
  [XFRM_MSG_DELPOLICY   - XFRM_MSG_BASE] = { .doit = xfrm_get_policy    },
  [XFRM_MSG_GETPOLICY   - XFRM_MSG_BASE] = { .doit = xfrm_get_policy,
+   .start = xfrm_dump_policy_start,
    .dump = xfrm_dump_policy,
    .done = xfrm_dump_policy_done },
  [XFRM_MSG_ALLOCSPI    - XFRM_MSG_BASE] = { .doit = xfrm_alloc_userspi },
@@ -2381,6 +2385,7 @@ static int xfrm_user_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
 
  {
  struct netlink_dump_control c = {
+ .start = link->start,
  .dump = link->dump,
  .done = link->done,
  };
--
2.14.1


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

ACK: [SRU][trusty][PATCH 0/2] Fix for CVE-2017-16939

Thadeu Lima de Souza Cascardo-3
In reply to this post by Kleber Souza
On Fri, Dec 01, 2017 at 05:06:24PM +0100, Kleber Sacilotto de Souza wrote:
> Patch 2/2 (ipsec: Fix aborted xfrm policy dump crash) is the real fix and
> it needed a simple backport to fix the context because it lacks
> 283bc9f35bbbc (xfrm: Namespacify xfrm state/policy locks). Patch 1/1 is a
> prerequisite and is a clean cherry-pick.
>
> Tested with the POC available on
> https://bugzilla.suse.com/show_bug.cgi?id=1069702.

Backport looked fine, tested fix.

Acked-by: Thadeu Lima de Souza Cascardo <[hidden email]>

>
> Herbert Xu (1):
>   ipsec: Fix aborted xfrm policy dump crash
>
> Tom Herbert (1):
>   netlink: add a start callback for starting a netlink dump
>
>  include/linux/netlink.h  |  2 ++
>  include/net/genetlink.h  |  2 ++
>  net/netlink/af_netlink.c |  4 ++++
>  net/netlink/genetlink.c  | 16 ++++++++++++++++
>  net/xfrm/xfrm_user.c     | 25 +++++++++++++++----------
>  5 files changed, 39 insertions(+), 10 deletions(-)
>
> --
> 2.14.1
>
>
> --
> kernel-team mailing list
> [hidden email]
> https://lists.ubuntu.com/mailman/listinfo/kernel-team

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

ACK: [SRU][trusty][PATCH 0/2] Fix for CVE-2017-16939

Stefan Bader-2
In reply to this post by Kleber Souza
On 01.12.2017 17:06, Kleber Sacilotto de Souza wrote:

> Patch 2/2 (ipsec: Fix aborted xfrm policy dump crash) is the real fix and
> it needed a simple backport to fix the context because it lacks
> 283bc9f35bbbc (xfrm: Namespacify xfrm state/policy locks). Patch 1/1 is a
> prerequisite and is a clean cherry-pick.
>
> Tested with the POC available on
> https://bugzilla.suse.com/show_bug.cgi?id=1069702.
>
> Herbert Xu (1):
>   ipsec: Fix aborted xfrm policy dump crash
>
> Tom Herbert (1):
>   netlink: add a start callback for starting a netlink dump
>
>  include/linux/netlink.h  |  2 ++
>  include/net/genetlink.h  |  2 ++
>  net/netlink/af_netlink.c |  4 ++++
>  net/netlink/genetlink.c  | 16 ++++++++++++++++
>  net/xfrm/xfrm_user.c     | 25 +++++++++++++++----------
>  5 files changed, 39 insertions(+), 10 deletions(-)
>
Backport looks ok to me and tested.

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
|

APPLIED: [SRU][trusty][PATCH 0/2] Fix for CVE-2017-16939

Thadeu Lima de Souza Cascardo-3
In reply to this post by Kleber Souza
Applied to trusty master-next branch.

Thanks.
Cascardo.

Applied-to: trusty/master-next

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