Re: [PATCH for-3.18 15/15] af_key: Fix sadb_x_ipsecrequest parsing

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Please ignore this one. This patch is already in your stable queue.

Regards,
Amit Pundir

On 26 July 2017 at 02:15, Amit Pundir <amit.pundir@xxxxxxxxxx> wrote:
> From: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>
>
> commit 096f41d3a8fcbb8dde7f71379b1ca85fe213eded upstream.
>
> The parsing of sadb_x_ipsecrequest is broken in a number of ways.
> First of all we're not verifying sadb_x_ipsecrequest_len.  This
> is needed when the structure carries addresses at the end.  Worse
> we don't even look at the length when we parse those optional
> addresses.
>
> The migration code had similar parsing code that's better but
> it also has some deficiencies.  The length is overcounted first
> of all as it includes the header itself.  It also fails to check
> the length before dereferencing the sa_family field.
>
> This patch fixes those problems in parse_sockaddr_pair and then
> uses it in parse_ipsecrequest.
>
> Reported-by: Andrey Konovalov <andreyknvl@xxxxxxxxxx>
> Signed-off-by: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Steffen Klassert <steffen.klassert@xxxxxxxxxxx>
> Signed-off-by: Amit Pundir <amit.pundir@xxxxxxxxxx>
> ---
>  net/key/af_key.c | 47 ++++++++++++++++++++++++++---------------------
>  1 file changed, 26 insertions(+), 21 deletions(-)
>
> diff --git a/net/key/af_key.c b/net/key/af_key.c
> index d49fa0dd2634..08e2c6159e03 100644
> --- a/net/key/af_key.c
> +++ b/net/key/af_key.c
> @@ -65,6 +65,10 @@ struct pfkey_sock {
>         } dump;
>  };
>
> +static int parse_sockaddr_pair(struct sockaddr *sa, int ext_len,
> +                              xfrm_address_t *saddr, xfrm_address_t *daddr,
> +                              u16 *family);
> +
>  static inline struct pfkey_sock *pfkey_sk(struct sock *sk)
>  {
>         return (struct pfkey_sock *)sk;
> @@ -1921,19 +1925,14 @@ parse_ipsecrequest(struct xfrm_policy *xp, struct sadb_x_ipsecrequest *rq)
>
>         /* addresses present only in tunnel mode */
>         if (t->mode == XFRM_MODE_TUNNEL) {
> -               u8 *sa = (u8 *) (rq + 1);
> -               int family, socklen;
> +               int err;
>
> -               family = pfkey_sockaddr_extract((struct sockaddr *)sa,
> -                                               &t->saddr);
> -               if (!family)
> -                       return -EINVAL;
> -
> -               socklen = pfkey_sockaddr_len(family);
> -               if (pfkey_sockaddr_extract((struct sockaddr *)(sa + socklen),
> -                                          &t->id.daddr) != family)
> -                       return -EINVAL;
> -               t->encap_family = family;
> +               err = parse_sockaddr_pair(
> +                       (struct sockaddr *)(rq + 1),
> +                       rq->sadb_x_ipsecrequest_len - sizeof(*rq),
> +                       &t->saddr, &t->id.daddr, &t->encap_family);
> +               if (err)
> +                       return err;
>         } else
>                 t->encap_family = xp->family;
>
> @@ -1953,7 +1952,11 @@ parse_ipsecrequests(struct xfrm_policy *xp, struct sadb_x_policy *pol)
>         if (pol->sadb_x_policy_len * 8 < sizeof(struct sadb_x_policy))
>                 return -EINVAL;
>
> -       while (len >= sizeof(struct sadb_x_ipsecrequest)) {
> +       while (len >= sizeof(*rq)) {
> +               if (len < rq->sadb_x_ipsecrequest_len ||
> +                   rq->sadb_x_ipsecrequest_len < sizeof(*rq))
> +                       return -EINVAL;
> +
>                 if ((err = parse_ipsecrequest(xp, rq)) < 0)
>                         return err;
>                 len -= rq->sadb_x_ipsecrequest_len;
> @@ -2416,7 +2419,6 @@ out:
>         return err;
>  }
>
> -#ifdef CONFIG_NET_KEY_MIGRATE
>  static int pfkey_sockaddr_pair_size(sa_family_t family)
>  {
>         return PFKEY_ALIGN8(pfkey_sockaddr_len(family) * 2);
> @@ -2428,7 +2430,7 @@ static int parse_sockaddr_pair(struct sockaddr *sa, int ext_len,
>  {
>         int af, socklen;
>
> -       if (ext_len < pfkey_sockaddr_pair_size(sa->sa_family))
> +       if (ext_len < 2 || ext_len < pfkey_sockaddr_pair_size(sa->sa_family))
>                 return -EINVAL;
>
>         af = pfkey_sockaddr_extract(sa, saddr);
> @@ -2444,6 +2446,7 @@ static int parse_sockaddr_pair(struct sockaddr *sa, int ext_len,
>         return 0;
>  }
>
> +#ifdef CONFIG_NET_KEY_MIGRATE
>  static int ipsecrequests_to_migrate(struct sadb_x_ipsecrequest *rq1, int len,
>                                     struct xfrm_migrate *m)
>  {
> @@ -2451,13 +2454,14 @@ static int ipsecrequests_to_migrate(struct sadb_x_ipsecrequest *rq1, int len,
>         struct sadb_x_ipsecrequest *rq2;
>         int mode;
>
> -       if (len <= sizeof(struct sadb_x_ipsecrequest) ||
> -           len < rq1->sadb_x_ipsecrequest_len)
> +       if (len < sizeof(*rq1) ||
> +           len < rq1->sadb_x_ipsecrequest_len ||
> +           rq1->sadb_x_ipsecrequest_len < sizeof(*rq1))
>                 return -EINVAL;
>
>         /* old endoints */
>         err = parse_sockaddr_pair((struct sockaddr *)(rq1 + 1),
> -                                 rq1->sadb_x_ipsecrequest_len,
> +                                 rq1->sadb_x_ipsecrequest_len - sizeof(*rq1),
>                                   &m->old_saddr, &m->old_daddr,
>                                   &m->old_family);
>         if (err)
> @@ -2466,13 +2470,14 @@ static int ipsecrequests_to_migrate(struct sadb_x_ipsecrequest *rq1, int len,
>         rq2 = (struct sadb_x_ipsecrequest *)((u8 *)rq1 + rq1->sadb_x_ipsecrequest_len);
>         len -= rq1->sadb_x_ipsecrequest_len;
>
> -       if (len <= sizeof(struct sadb_x_ipsecrequest) ||
> -           len < rq2->sadb_x_ipsecrequest_len)
> +       if (len <= sizeof(*rq2) ||
> +           len < rq2->sadb_x_ipsecrequest_len ||
> +           rq2->sadb_x_ipsecrequest_len < sizeof(*rq2))
>                 return -EINVAL;
>
>         /* new endpoints */
>         err = parse_sockaddr_pair((struct sockaddr *)(rq2 + 1),
> -                                 rq2->sadb_x_ipsecrequest_len,
> +                                 rq2->sadb_x_ipsecrequest_len - sizeof(*rq2),
>                                   &m->new_saddr, &m->new_daddr,
>                                   &m->new_family);
>         if (err)
> --
> 2.7.4
>



[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]