[Devel] [PATCH RH7 4/5] netfilter: don't leak compat connmark, conntrack structs

[Dmitry Safonov]

Found by Solar Designer during vz7 audi:
> At first glance, it was puzzling to me that compat_from_user() didn't
> have the "src" pointer annotated as __user, and that indeed Vz7's added
> implementations treated this pointer as a kernel one.  Yet this appears
> consistent with upstream's usage, so that's probably as it should be,
> even if the naming is a bit confusing.
>
> This leaves us with investigating possible infoleaks in compat_to_user(),
> and there appear to be some.  Note that some instances below allocate a
> temporary struct on *_compat_to_user()'s stack, yet don't initialize its
> explicit padding fields (__pad1 and __pad2) present in several of the
> structs.
>
> "struct compat_xt_conntrack_info" does not include explicit padding
> fields, but it isn't immediately obvious to me whether it contains any
> implicit padding or not.
>
> Finally, "struct compat_xt_mark_target_info" with its sole field looks
> safe to me.

https://jira.sw.ru/browse/PSBM-51365

Cc: Kirill Tkhai <ktkhai at virtuozzo.com>
Signed-off-by: Dmitry Safonov <dsafonov at virtuozzo.com>
---
 net/netfilter/xt_connmark.c  | 22 ++++++++++++----------
 net/netfilter/xt_conntrack.c | 17 +++++++++--------
 net/netfilter/xt_mark.c      | 20 +++++++++++---------
 3 files changed, 32 insertions(+), 27 deletions(-)

diff --git a/net/netfilter/xt_connmark.c b/net/netfilter/xt_connmark.c
index 3059aa361e8e..2f7142f38f38 100644
--- a/net/netfilter/xt_connmark.c
+++ b/net/netfilter/xt_connmark.c
@@ -237,11 +237,12 @@ static void connmark_tg_compat_from_user_v0(void *dst, const void *src)
 static int connmark_tg_compat_to_user_v0(void __user *dst, const void *src)
 {
 	const struct xt_connmark_target_info *m = src;
-	struct compat_xt_connmark_target_info cm = {
-		.mark	= m->mark,
-		.mask	= m->mask,
-		.mode	= m->mode,
-	};
+	struct compat_xt_connmark_target_info cm;
+
+	memset(&cm, 0, sizeof(cm));
+	cm.mark	= m->mark;
+	cm.mask	= m->mask;
+	cm.mode	= m->mode;
 	return copy_to_user(dst, &cm, sizeof(cm)) ? -EFAULT : 0;
 }
 #endif /* CONFIG_COMPAT */
@@ -296,11 +297,12 @@ static void connmark_mt_compat_from_user_v0(void *dst, const void *src)
 static int connmark_mt_compat_to_user_v0(void __user *dst, const void *src)
 {
 	const struct xt_connmark_info *m = src;
-	struct compat_xt_connmark_info cm = {
-		.mark	= m->mark,
-		.mask	= m->mask,
-		.invert	= m->invert,
-	};
+	struct compat_xt_connmark_info cm;
+
+	memset(&cm, 0, sizeof(cm));
+	cm.mark = m->mark;
+	cm.mask = m->mask;
+	cm.invert = m->invert;
 	return copy_to_user(dst, &cm, sizeof(cm)) ? -EFAULT : 0;
 }
 #endif /* CONFIG_COMPAT */
diff --git a/net/netfilter/xt_conntrack.c b/net/netfilter/xt_conntrack.c
index 4846430e6a48..acc65a552bc7 100644
--- a/net/netfilter/xt_conntrack.c
+++ b/net/netfilter/xt_conntrack.c
@@ -368,14 +368,15 @@ static void conntrack_mt_compat_from_user_v0(void *dst, const void *src)
 static int conntrack_mt_compat_to_user_v0(void __user *dst, const void *src)
 {
 	const struct xt_conntrack_info *m = src;
-	struct compat_xt_conntrack_info cm = {
-		.statemask	= m->statemask,
-		.statusmask	= m->statusmask,
-		.expires_min	= m->expires_min,
-		.expires_max	= m->expires_max,
-		.flags		= m->flags,
-		.invflags	= m->invflags,
-	};
+	struct compat_xt_conntrack_info cm;
+
+	memset(&cm, 0, sizeof(cm));
+	cm.statemask	= m->statemask;
+	cm.statusmask	= m->statusmask;
+	cm.expires_min	= m->expires_min;
+	cm.expires_max	= m->expires_max;
+	cm.flags	= m->flags;
+	cm.invflags	= m->invflags;
 	memcpy(cm.tuple, m->tuple, sizeof(cm.tuple));
 	memcpy(cm.sipmsk, m->sipmsk, sizeof(cm.sipmsk));
 	memcpy(cm.dipmsk, m->dipmsk, sizeof(cm.dipmsk));
diff --git a/net/netfilter/xt_mark.c b/net/netfilter/xt_mark.c
index 09ef00fdd41e..4239b7c9630c 100644
--- a/net/netfilter/xt_mark.c
+++ b/net/netfilter/xt_mark.c
@@ -154,10 +154,11 @@ static void mark_tg_compat_from_user_v1(void *dst, const void *src)
 static int mark_tg_compat_to_user_v1(void __user *dst, const void *src)
 {
 	const struct xt_mark_target_info_v1 *m = src;
-	struct compat_xt_mark_target_info_v1 cm = {
-		.mark	= m->mark,
-		.mode	= m->mode,
-	};
+	struct compat_xt_mark_target_info_v1 cm;
+
+	memset(&cm, 0, sizeof(cm));
+	cm.mark = m->mark;
+	cm.mode = m->mode;
 	return copy_to_user(dst, &cm, sizeof(cm)) ? -EFAULT : 0;
 }
 #endif /* CONFIG_COMPAT */
@@ -250,11 +251,12 @@ static void mark_mt_compat_from_user_v0(void *dst, const void *src)
 static int mark_mt_compat_to_user_v0(void __user *dst, const void *src)
 {
 	const struct xt_mark_info *m = src;
-	struct compat_xt_mark_info cm = {
-		.mark	= m->mark,
-		.mask	= m->mask,
-		.invert	= m->invert,
-	};
+	struct compat_xt_mark_info cm;
+
+	memset(&cm, 0, sizeof(cm));
+	cm.mark = m->mark;
+	cm.mask = m->mask;
+	cm.invert = m->invert;
 	return copy_to_user(dst, &cm, sizeof(cm)) ? -EFAULT : 0;
 }
 #endif /* CONFIG_COMPAT */
-- 
2.10.0