From: Kazunori MIYAZAWA <[EMAIL PROTECTED]>
Date: Mon, 04 Dec 2006 11:50:09 +0900
> Thank you for your tracing the bug.
>
> I understood the issue.
> Mmm, if we can not use ut->family, can we use
> ut->id.family instead?
>
> Or is it also uninitialized?
struct xfrm_id does not have a family field
struct xfrm_id
{
xfrm_address_t daddr;
__be32 spi;
__u8 proto;
};
That's what ut->id is.
You're thinking of xfrm_usersa_id, which is used by
another structure, xfrm_aevent_id.
For the time being I'm thinking of using the following
patch. I removed the xp->family clobbering, the AF_KEY
changes don't do this, so there is no reason for the
xfrm_user side to do it either.
Every path that leads to copy_templates() will perform
a validate_tmpl() which will change ut->family == 0
to the policy's family value. Any other non-supported
value will trigger an error.
Let's hope this fix is sufficient. I'm about to test
this now.
diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index 6f97665..311205f 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -858,7 +858,6 @@ static void copy_templates(struct xfrm_p
int i;
xp->xfrm_nr = nr;
- xp->family = ut->family;
for (i = 0; i < nr; i++, ut++) {
struct xfrm_tmpl *t = &xp->xfrm_vec[i];
@@ -876,19 +875,53 @@ static void copy_templates(struct xfrm_p
}
}
+static int validate_tmpl(int nr, struct xfrm_user_tmpl *ut, u16 family)
+{
+ int i;
+
+ if (nr > XFRM_MAX_DEPTH)
+ return -EINVAL;
+
+ for (i = 0; i < nr; i++) {
+ /* We never validated the ut->family value, so many
+ * applications simply leave it at zero. The check was
+ * never made and ut->family was ignored because all
+ * templates could be assumed to have the same family as
+ * the policy itself. Now that we will have ipv4-in-ipv6
+ * and ipv6-in-ipv4 tunnels, this is no longer true.
+ */
+ if (!ut[i].family)
+ ut[i].family = family;
+
+ switch (ut[i].family) {
+ case AF_INET:
+ break;
+#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
+ case AF_INET6:
+ break;
+#endif
+ default:
+ return -EINVAL;
+ };
+ }
+
+ return 0;
+}
+
static int copy_from_user_tmpl(struct xfrm_policy *pol, struct rtattr **xfrma)
{
struct rtattr *rt = xfrma[XFRMA_TMPL-1];
- struct xfrm_user_tmpl *utmpl;
- int nr;
if (!rt) {
pol->xfrm_nr = 0;
} else {
- nr = (rt->rta_len - sizeof(*rt)) / sizeof(*utmpl);
+ struct xfrm_user_tmpl *utmpl = RTA_DATA(rt);
+ int nr = (rt->rta_len - sizeof(*rt)) / sizeof(*utmpl);
+ int err;
- if (nr > XFRM_MAX_DEPTH)
- return -EINVAL;
+ err = validate_tmpl(nr, utmpl, pol->family);
+ if (err)
+ return err;
copy_templates(pol, RTA_DATA(rt), nr);
}
@@ -1530,7 +1563,8 @@ static int xfrm_add_acquire(struct sk_bu
}
/* build an XP */
- xp = xfrm_policy_construct(&ua->policy, (struct rtattr **) xfrma,
&err); if (!xp) {
+ xp = xfrm_policy_construct(&ua->policy, (struct rtattr **) xfrma, &err);
+ if (!xp) {
kfree(x);
return err;
}
@@ -1979,7 +2013,7 @@ #endif
return NULL;
nr = ((len - sizeof(*p)) / sizeof(*ut));
- if (nr > XFRM_MAX_DEPTH)
+ if (validate_tmpl(nr, ut, p->sel.family))
return NULL;
if (p->dir > XFRM_POLICY_OUT)
-
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
