Re: [PATCH] possible overflow of sock->sk_policy
From: Herbert Xu <[EMAIL PROTECTED]> Subject: Re: [PATCH] possible overflow of sock->sk_policy Date: Tue, 26 Jul 2005 13:07:14 +1000 > Balazs Scheidler <[EMAIL PROTECTED]> wrote: > > > > While reading through the xfrm code I've found a possible array overflow > > in struct sock. > > Thanks for catching this. However, the check should be done in xfrm_user > as we do for af_key. The following patch does just that. > > Signed-off-by: Herbert Xu <[EMAIL PROTECTED]> Applied, thanks Herbert. - 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
Re: [PATCH] possible overflow of sock->sk_policy
Balazs Scheidler <[EMAIL PROTECTED]> wrote:
>
> While reading through the xfrm code I've found a possible array overflow
> in struct sock.
Thanks for catching this. However, the check should be done in xfrm_user
as we do for af_key. The following patch does just that.
Signed-off-by: Herbert Xu <[EMAIL PROTECTED]>
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[EMAIL PROTECTED]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -1350,6 +1350,9 @@ static struct xfrm_policy *xfrm_compile_
if (nr > XFRM_MAX_DEPTH)
return NULL;
+ if (p->dir > XFRM_POLICY_OUT)
+ return NULL;
+
xp = xfrm_policy_alloc(GFP_KERNEL);
if (xp == NULL) {
*dir = -ENOBUFS;
-
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
Re: [PATCH] possible overflow of sock->sk_policy
> Hi,
>
> 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().
test program originally missed, here it is this time.
--
Bazsi
#include
#include
#include
#include
#include
#include
#define IP_XFRM_POLICY 17
int
main()
{
int fd;
struct sockaddr_in s_in;
struct xfrm_userpolicy_info xp;
inet_aton("192.168.13.8", &s_in.sin_addr);
s_in.sin_family = AF_INET;
s_in.sin_port = htons(555);
fd = socket(PF_INET, SOCK_DGRAM, IPPROTO_UDP);
if (fd < 0)
{
perror("socket");
return 1;
}
if (connect(fd, (struct sockaddr *) &s_in, sizeof(s_in)) < 0)
{
perror("connect");
return 1;
}
inet_aton("192.168.13.7", (struct in_addr *) &xp.sel.saddr);
xp.sel.prefixlen_s = 32;
inet_aton("192.168.13.8", (struct in_addr *) &xp.sel.daddr);
xp.sel.prefixlen_d = 32;
xp.sel.dport = 0;
xp.sel.dport_mask = 0;
xp.sel.sport = 0;
xp.sel.sport_mask = 0;
xp.sel.family = AF_INET;
xp.sel.proto = 0;
xp.sel.ifindex = 0;
xp.sel.user = 0;
xp.lft.soft_byte_limit = 1000;
xp.lft.hard_byte_limit = 1000;
xp.lft.soft_packet_limit = 1000;
xp.lft.hard_packet_limit = 1000;
xp.lft.soft_add_expires_seconds = 3600;
xp.lft.hard_add_expires_seconds = 3600;
xp.lft.soft_use_expires_seconds = 3600;
xp.lft.hard_use_expires_seconds = 3600;
xp.curlft.bytes = 0;
xp.curlft.packets = 0;
xp.curlft.add_time = 0;
xp.curlft.use_time = 0;
xp.priority = 0;
xp.index = 0;
xp.dir = XFRM_POLICY_FWD;
xp.action = XFRM_POLICY_ALLOW;
xp.flags = 0;
xp.share = XFRM_SHARE_UNIQUE;
if (setsockopt(fd, SOL_IP, IP_XFRM_POLICY, &xp, sizeof(xp)) < 0)
{
perror("setsockopt(IP_XFRM_POLICY)");
return 1;
}
send(fd, "abrakadabra", 11, 0);
close(fd);
}
[PATCH] possible overflow of sock->sk_policy
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
