[v2] xt_connmark: Fix invalid tginfo* casting.

Message ID 20180416012104.11855-1-jack.ma@alliedtelesis.co.nz
State Superseded
Delegated to: Pablo Neira
Headers show
Series
  • [v2] xt_connmark: Fix invalid tginfo* casting.
Related show

Commit Message

Jack Ma April 16, 2018, 1:21 a.m.
This bug was found during testing pre-existing iptables options
with newly added v2 APIs.

Casting from (const struct xt_connmark_tginfo2 *) to
(const struct xt_connmark_tginfo1 *) results in the significance
to be lost. Subsequentially, the 'info->mode' is reset to zero,
when multiple options are parsed in.

Signed-off-by: Jack Ma <jack.ma@alliedtelesis.co.nz>
---
 net/netfilter/xt_connmark.c | 25 ++++++++++++++-----------
 1 file changed, 14 insertions(+), 11 deletions(-)

Comments

Jack Ma April 16, 2018, 1:21 a.m. | #1
Sorry, please ignore v1 patch.
version two is the clean patch to be applied.. 

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Pablo Neira Ayuso April 19, 2018, 2:24 p.m. | #2
On Mon, Apr 16, 2018 at 01:21:04PM +1200, Jack Ma wrote:
> This bug was found during testing pre-existing iptables options
> with newly added v2 APIs.
> 
> Casting from (const struct xt_connmark_tginfo2 *) to
> (const struct xt_connmark_tginfo1 *) results in the significance
> to be lost. Subsequentially, the 'info->mode' is reset to zero,
> when multiple options are parsed in.

Sent you a different patch to sort out this. Thanks.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/net/netfilter/xt_connmark.c b/net/netfilter/xt_connmark.c
index 710bc2bfe020..4350a6877077 100644
--- a/net/netfilter/xt_connmark.c
+++ b/net/netfilter/xt_connmark.c
@@ -37,21 +37,22 @@  MODULE_ALIAS("ip6t_connmark");
 
 static unsigned int
 connmark_tg_shift(struct sk_buff *skb,
-		const struct xt_connmark_tginfo1 *info,
+		u8 mode, u32 ctmark,
+		u32 ctmask, u32 nfmask,
 		u8 shift_bits, u8 shift_dir)
 {
 	enum ip_conntrack_info ctinfo;
 	struct nf_conn *ct;
-	u_int32_t newmark;
-	u_int32_t new_targetmark;
+	u_int32_t newmark = 0;
+	u_int32_t new_targetmark = 0;
 
 	ct = nf_ct_get(skb, &ctinfo);
 	if (ct == NULL)
 		return XT_CONTINUE;
 
-	switch (info->mode) {
+	switch (mode) {
 	case XT_CONNMARK_SET:
-		newmark = (ct->mark & ~info->ctmask) ^ info->ctmark;
+		newmark = (ct->mark & ~ctmask) ^ ctmark;
 		if (shift_dir == D_SHIFT_RIGHT)
 			newmark >>= shift_bits;
 		else
@@ -62,12 +63,12 @@  connmark_tg_shift(struct sk_buff *skb,
 		}
 		break;
 	case XT_CONNMARK_SAVE:
-		new_targetmark = (skb->mark & info->nfmask);
+		new_targetmark = (skb->mark & nfmask);
 		if (shift_dir == D_SHIFT_RIGHT)
 			new_targetmark >>= shift_bits;
 		else
 			new_targetmark <<= shift_bits;
-		newmark = (ct->mark & ~info->ctmask) ^
+		newmark = (ct->mark & ~ctmask) ^
 			  new_targetmark;
 		if (ct->mark != newmark) {
 			ct->mark = newmark;
@@ -75,12 +76,12 @@  connmark_tg_shift(struct sk_buff *skb,
 		}
 		break;
 	case XT_CONNMARK_RESTORE:
-		new_targetmark = (ct->mark & info->ctmask);
+		new_targetmark = (ct->mark & ctmask);
 		if (shift_dir == D_SHIFT_RIGHT)
 			new_targetmark >>= shift_bits;
 		else
 			new_targetmark <<= shift_bits;
-		newmark = (skb->mark & ~info->nfmask) ^
+		newmark = (skb->mark & ~nfmask) ^
 			  new_targetmark;
 		skb->mark = newmark;
 		break;
@@ -93,7 +94,8 @@  connmark_tg(struct sk_buff *skb, const struct xt_action_param *par)
 {
 	const struct xt_connmark_tginfo1 *info = par->targinfo;
 
-	return connmark_tg_shift(skb, info, 0, 0);
+	return connmark_tg_shift(skb, info->mode, info->ctmark,
+				 info->ctmask, info->nfmask, 0, 0);
 }
 
 static unsigned int
@@ -101,7 +103,8 @@  connmark_tg_v2(struct sk_buff *skb, const struct xt_action_param *par)
 {
 	const struct xt_connmark_tginfo2 *info = par->targinfo;
 
-	return connmark_tg_shift(skb, (const struct xt_connmark_tginfo1 *)info,
+	return connmark_tg_shift(skb, info->mode, info->ctmark,
+				 info->ctmask, info->nfmask,
 				 info->shift_bits, info->shift_dir);
 }