Hi,

While reading through the xfrm code I've found a possible array overflow
in struct sock.

When issuing a setsockopt(SOL_IP, IP_XFRM_POLICY) on a socket, a
function called xfrm_user_policy() is called to compile and install a
socket specific XFRM policy.

This function calls km->compile_policy() to compile the incoming
setsockopt data to struct xfrm_policy which validates the incoming data
and checks (among other things) that struct xfrm_userpolicy_info->dir is
valid. Valid here means that it is either XFRM_POLICY_{IN,OUT,FWD}.

xfrm_sk_policy_insert() then takes this value blindly and inserts the
new policy to struct sock->sk_policy[dir], if dir is XFRM_POLICY_FWD,
then the sk_policy array is overflown as it is declared as having two
items only and XFRM_POLICY_FWD numerically equals to 2.

I'm attaching a small testprogram which tries to install an
XFRM_POLICY_FWD, and I confirmed with a printk that the value of 2 is
successfully propagated to xfrm_sk_policy_insert().

The value after the sk_policy is an "rwlock_t sk_dst_lock;" which seems
to be used from TCP only.

The effect of this bug is probably only a deadlock, as rwlock_t contains
a single unsigned int value and no pointers.

A possible fix below can be found at the end of this mail.

Signed-off-by: Balazs Scheidler <[EMAIL PROTECTED]>

--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -571,6 +571,9 @@ int xfrm_policy_delete(struct xfrm_polic
 int xfrm_sk_policy_insert(struct sock *sk, int dir, struct xfrm_policy *pol)
 {
        struct xfrm_policy *old_pol;
+
+       if (dir > XFRM_POLICY_OUT)
+               return -EINVAL;

        write_lock_bh(&xfrm_policy_lock);
        old_pol = sk->sk_policy[dir];
diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -920,9 +920,8 @@ int xfrm_user_policy(struct sock *sk, in
        read_unlock(&xfrm_km_lock);

        if (err >= 0) {
-               xfrm_sk_policy_insert(sk, err, pol);
+               err = xfrm_sk_policy_insert(sk, err, pol);
                xfrm_pol_put(pol);
-               err = 0;
        }

 out:


-- 
Bazsi

-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to