Re: [PATCH v4] netfilter: introduce l2tp match extension

[Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx>]

On Thu, Jan 02, 2014 at 09:57:13PM +0100, Pablo Neira Ayuso wrote:
[...]
> I'm testing this with the last userspace iptables patch that you
> posted [1]. I'm using the example in the manpage:
> 
> # iptables -A INPUT -s 1.2.3.4 -m l2tp --tid 42
> iptables: Invalid argument. Run `dmesg' for more information.
> # dmesg
> ...
> [  490.827569] xt_l2tp: missing encapsulation
> 
> The error message is added by the patch I made on top of your last
> kernel patch (find it attached, feel free to merge it to your next v5).

Forgot attachment, here it comes.

diff --git a/net/netfilter/xt_l2tp.c b/net/netfilter/xt_l2tp.c
index d4ec208..f2104aa 100644
--- a/net/netfilter/xt_l2tp.c
+++ b/net/netfilter/xt_l2tp.c
@@ -221,33 +221,45 @@ static int l2tp_mt_check(const struct xt_mtchk_param *par)
 
 	/* Check for invalid flags */
 	if (info->flags & ~(XT_L2TP_TID | XT_L2TP_SID | XT_L2TP_VERSION |
-			    XT_L2TP_ENCAP | XT_L2TP_TYPE))
+			    XT_L2TP_ENCAP | XT_L2TP_TYPE)) {
+		pr_info("unknown flags: %x\n", info->flags);
 		return -EINVAL;
+	}
 
 	/* At least one of tid, sid or type=control must be specified */
 	if ((!(info->flags & XT_L2TP_TID)) &&
 	    (!(info->flags & XT_L2TP_SID)) &&
 	    ((!(info->flags & XT_L2TP_TYPE)) ||
-	     (info->type != XT_L2TP_TYPE_CONTROL)))
+	     (info->type != XT_L2TP_TYPE_CONTROL))) {
+		pr_info("invalid flags combination: %x\n", info->flags);
 		return -EINVAL;
+	}
 
 	/* If version 2 is specified, check that incompatible params
 	 * are not supplied
 	 */
 	if (info->flags & XT_L2TP_VERSION) {
-		if ((info->version < 2) || (info->version > 3))
+		if ((info->version < 2) || (info->version > 3)) {
+			pr_info("wrong L2TP version: %u\n", info->version);
 			return -EINVAL;
+		}
 
 		if (info->version == 2) {
 			if ((info->flags & XT_L2TP_TID) &&
-			    (info->tid > 0xffff))
+			    (info->tid > 0xffff)) {
+				pr_info("tid > 0xffff: %u\n", info->tid);
 				return -EINVAL;
+			}
 			if ((info->flags & XT_L2TP_SID) &&
-			    (info->sid > 0xffff))
+			    (info->sid > 0xffff)) {
+				pr_info("sid > 0xffff: %u\n", info->sid);
 				return -EINVAL;
+			}
 			if ((info->flags & XT_L2TP_ENCAP) &&
-			    (info->encap == XT_L2TP_ENCAP_IP))
+			    (info->encap == XT_L2TP_ENCAP_IP)) {
+				pr_info("v2 doesn't support IP mode\n");
 				return -EINVAL;
+			}
 
 			/* Force UDP encap */
 			info->encap = XT_L2TP_ENCAP_UDP;
@@ -256,8 +268,10 @@ static int l2tp_mt_check(const struct xt_mtchk_param *par)
 	}
 
 	/* Encap must be specified */
-	if (!(info->flags & XT_L2TP_ENCAP))
+	if (!(info->flags & XT_L2TP_ENCAP)) {
+		pr_info("missing encapsulation\n");
 		return -EINVAL;
+	}
 
 	return 0;
 }