[LTP] [PATCH v3 0/4] Test for CVE 2023-31248

[LTP] [PATCH v3 0/4] Test for CVE 2023-31248

[Martin Doucha]

Add test for CVE 2023-31248. Support for older kernels requires some
changes in tst_netlink_check_acks() and a new LAPI header.

Changes tested on kernels 4.4, 4.12 and 5.14.

Martin Doucha (4):
  tst_netlink: Add helper functions for handling generic attributes
  tst_netlink_check_acks(): Stop on first error regardless of ACK
    request
  Add lapi/nf_tables.h
  Add test for CVE 2023-31248

 configure.ac                          |   1 +
 include/lapi/nf_tables.h              |  19 +++
 include/tst_netdevice.h               |   6 +-
 include/tst_netlink.h                 |  38 ++++-
 lib/tst_netdevice.c                   |  20 +--
 lib/tst_netlink.c                     |  97 +++++++++++-
 runtest/cve                           |   1 +
 testcases/cve/tcindex01.c             |  12 +-
 testcases/network/iptables/.gitignore |   1 +
 testcases/network/iptables/Makefile   |   2 +-
 testcases/network/iptables/nft02.c    | 213 ++++++++++++++++++++++++++
 11 files changed, 375 insertions(+), 35 deletions(-)
 create mode 100644 include/lapi/nf_tables.h
 create mode 100644 testcases/network/iptables/.gitignore
 create mode 100644 testcases/network/iptables/nft02.c

-- 
2.42.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v3 1/4] tst_netlink: Add helper functions for handling generic attributes

[Martin Doucha]

Refactor struct tst_rtnl_attr_list for generic use and add helper
functions for handling generic struct nlattr message attributes.

Signed-off-by: Martin Doucha <mdoucha@suse.cz>
---

Changes since v1: New patch
Changes since v2: None

 include/tst_netdevice.h   |  6 +--
 include/tst_netlink.h     | 38 ++++++++++++++----
 lib/tst_netdevice.c       | 20 +++++-----
 lib/tst_netlink.c         | 82 ++++++++++++++++++++++++++++++++++++++-
 testcases/cve/tcindex01.c | 12 +++---
 5 files changed, 131 insertions(+), 27 deletions(-)

diff --git a/include/tst_netdevice.h b/include/tst_netdevice.h
index 4239d0960..8d40148a6 100644
--- a/include/tst_netdevice.h
+++ b/include/tst_netdevice.h
@@ -132,7 +132,7 @@ int tst_netdev_remove_route_inet(const char *file, const int lineno,
 int tst_netdev_add_qdisc(const char *file, const int lineno, int strict,
 	const char *ifname, unsigned int family, unsigned int parent,
 	unsigned int handle, const char *qd_kind,
-	const struct tst_rtnl_attr_list *config);
+	const struct tst_netlink_attr_list *config);
 #define NETDEV_ADD_QDISC(ifname, family, parent, handle, qd_kind, config) \
 	tst_netdev_add_qdisc(__FILE__, __LINE__, 1, (ifname), (family), \
 		(parent), (handle), (qd_kind), (config))
@@ -154,7 +154,7 @@ int tst_netdev_remove_qdisc(const char *file, const int lineno, int strict,
 int tst_netdev_add_traffic_class(const char *file, const int lineno,
 	int strict, const char *ifname, unsigned int parent,
 	unsigned int handle, const char *qd_kind,
-	const struct tst_rtnl_attr_list *config);
+	const struct tst_netlink_attr_list *config);
 #define NETDEV_ADD_TRAFFIC_CLASS(ifname, parent, handle, qd_kind, config) \
 	tst_netdev_add_traffic_class(__FILE__, __LINE__, 1, (ifname), \
 		(parent), (handle), (qd_kind), (config))
@@ -173,7 +173,7 @@ int tst_netdev_remove_traffic_class(const char *file, const int lineno,
 int tst_netdev_add_traffic_filter(const char *file, const int lineno,
 	int strict, const char *ifname, unsigned int parent,
 	unsigned int handle, unsigned int protocol, unsigned int priority,
-	const char *f_kind, const struct tst_rtnl_attr_list *config);
+	const char *f_kind, const struct tst_netlink_attr_list *config);
 #define NETDEV_ADD_TRAFFIC_FILTER(ifname, parent, handle, protocol, priority, \
 	f_kind, config) \
 	tst_netdev_add_traffic_filter(__FILE__, __LINE__, 1, (ifname), \
diff --git a/include/tst_netlink.h b/include/tst_netlink.h
index f10f1cf5d..7d96fd711 100644
--- a/include/tst_netlink.h
+++ b/include/tst_netlink.h
@@ -9,11 +9,11 @@
 
 struct tst_netlink_context;
 
-struct tst_rtnl_attr_list {
+struct tst_netlink_attr_list {
 	unsigned short type;
 	const void *data;
 	ssize_t len;
-	const struct tst_rtnl_attr_list *sublist;
+	const struct tst_netlink_attr_list *sublist;
 };
 
 struct tst_netlink_message {
@@ -72,25 +72,49 @@ int tst_netlink_add_message(const char *file, const int lineno,
 	tst_netlink_add_message(__FILE__, __LINE__, (ctx), (header), \
 		(payload), (psize))
 
-/* Add arbitrary attribute to last message */
+/* Add arbitrary nlattr attribute to last message */
+int tst_netlink_add_attr(const char *file, const int lineno,
+	struct tst_netlink_context *ctx, unsigned short type, const void *data,
+	unsigned short len);
+#define NETLINK_ADD_ATTR(ctx, type, data, len) \
+	tst_netlink_add_attr(__FILE__, __LINE__, (ctx), (type), (data), (len))
+
+/* Add string nlattr attribute to last message */
+int tst_netlink_add_attr_string(const char *file, const int lineno,
+	struct tst_netlink_context *ctx, unsigned short type, const char *data);
+#define NETLINK_ADD_ATTR_STRING(ctx, type, data) \
+	tst_netlink_add_attr_string(__FILE__, __LINE__, (ctx), (type), (data))
+
+/*
+ * Add list of arbitrary nlattr attributes to last message. The list is
+ * terminated by attribute with negative length. Nested sublists are supported.
+ */
+int tst_netlink_add_attr_list(const char *file, const int lineno,
+	struct tst_netlink_context *ctx,
+	const struct tst_netlink_attr_list *list);
+#define NETLINK_ADD_ATTR_LIST(ctx, list) \
+	tst_netlink_add_attr_list(__FILE__, __LINE__, (ctx), (list))
+
+/* Add arbitrary rtattr attribute to last message */
 int tst_rtnl_add_attr(const char *file, const int lineno,
 	struct tst_netlink_context *ctx, unsigned short type, const void *data,
 	unsigned short len);
 #define RTNL_ADD_ATTR(ctx, type, data, len) \
 	tst_rtnl_add_attr(__FILE__, __LINE__, (ctx), (type), (data), (len))
 
-/* Add string attribute to last message */
+/* Add string rtattr attribute to last message */
 int tst_rtnl_add_attr_string(const char *file, const int lineno,
 	struct tst_netlink_context *ctx, unsigned short type, const char *data);
 #define RTNL_ADD_ATTR_STRING(ctx, type, data) \
 	tst_rtnl_add_attr_string(__FILE__, __LINE__, (ctx), (type), (data))
 
 /*
- * Add list of arbitrary attributes to last message. The list is terminated
- * by attribute with negative length. Nested sublists are supported.
+ * Add list of arbitrary rtattr attributes to last message. The list is
+ * terminated by attribute with negative length. Nested sublists are supported.
  */
 int tst_rtnl_add_attr_list(const char *file, const int lineno,
-	struct tst_netlink_context *ctx, const struct tst_rtnl_attr_list *list);
+	struct tst_netlink_context *ctx,
+	const struct tst_netlink_attr_list *list);
 #define RTNL_ADD_ATTR_LIST(ctx, list) \
 	tst_rtnl_add_attr_list(__FILE__, __LINE__, (ctx), (list))
 
diff --git a/lib/tst_netdevice.c b/lib/tst_netdevice.c
index 6f86b8089..1042466bf 100644
--- a/lib/tst_netdevice.c
+++ b/lib/tst_netdevice.c
@@ -105,17 +105,17 @@ int tst_create_veth_pair(const char *file, const int lineno, int strict,
 	int ret;
 	struct ifinfomsg info = { .ifi_family = AF_UNSPEC };
 	struct tst_netlink_context *ctx;
-	struct tst_rtnl_attr_list peerinfo[] = {
+	struct tst_netlink_attr_list peerinfo[] = {
 		{IFLA_IFNAME, ifname2, strlen(ifname2) + 1, NULL},
 		{0, NULL, -1, NULL}
 	};
-	struct tst_rtnl_attr_list peerdata[] = {
+	struct tst_netlink_attr_list peerdata[] = {
 		{VETH_INFO_PEER, &info, sizeof(info), peerinfo},
 		{0, NULL, -1, NULL}
 	};
-	struct tst_rtnl_attr_list attrs[] = {
+	struct tst_netlink_attr_list attrs[] = {
 		{IFLA_IFNAME, ifname1, strlen(ifname1) + 1, NULL},
-		{IFLA_LINKINFO, NULL, 0, (const struct tst_rtnl_attr_list[]){
+		{IFLA_LINKINFO, NULL, 0, (const struct tst_netlink_attr_list[]){
 			{IFLA_INFO_KIND, "veth", 4, NULL},
 			{IFLA_INFO_DATA, NULL, 0, peerdata},
 			{0, NULL, -1, NULL}
@@ -164,9 +164,9 @@ int tst_netdev_add_device(const char *file, const int lineno, int strict,
 	int ret;
 	struct ifinfomsg info = { .ifi_family = AF_UNSPEC };
 	struct tst_netlink_context *ctx;
-	struct tst_rtnl_attr_list attrs[] = {
+	struct tst_netlink_attr_list attrs[] = {
 		{IFLA_IFNAME, ifname, strlen(ifname) + 1, NULL},
-		{IFLA_LINKINFO, NULL, 0, (const struct tst_rtnl_attr_list[]){
+		{IFLA_LINKINFO, NULL, 0, (const struct tst_netlink_attr_list[]){
 			{IFLA_INFO_KIND, devtype, strlen(devtype), NULL},
 			{0, NULL, -1, NULL}
 		}},
@@ -527,7 +527,7 @@ static int modify_qdisc(const char *file, const int lineno, int strict,
 	const char *object, unsigned int action, unsigned int nl_flags,
 	const char *ifname, unsigned int family, unsigned int parent,
 	unsigned int handle, unsigned int info, const char *qd_kind,
-	const struct tst_rtnl_attr_list *config)
+	const struct tst_netlink_attr_list *config)
 {
 	struct tst_netlink_context *ctx;
 	int ret;
@@ -585,7 +585,7 @@ static int modify_qdisc(const char *file, const int lineno, int strict,
 int tst_netdev_add_qdisc(const char *file, const int lineno, int strict,
 	const char *ifname, unsigned int family, unsigned int parent,
 	unsigned int handle, const char *qd_kind,
-	const struct tst_rtnl_attr_list *config)
+	const struct tst_netlink_attr_list *config)
 {
 	return modify_qdisc(file, lineno, strict, "queueing discipline",
 		RTM_NEWQDISC, NLM_F_CREATE | NLM_F_EXCL, ifname, family,
@@ -604,7 +604,7 @@ int tst_netdev_remove_qdisc(const char *file, const int lineno, int strict,
 int tst_netdev_add_traffic_class(const char *file, const int lineno,
 	int strict, const char *ifname, unsigned int parent,
 	unsigned int handle, const char *qd_kind,
-	const struct tst_rtnl_attr_list *config)
+	const struct tst_netlink_attr_list *config)
 {
 	return modify_qdisc(file, lineno, strict, "traffic class",
 		RTM_NEWTCLASS, NLM_F_CREATE | NLM_F_EXCL, ifname, AF_UNSPEC,
@@ -623,7 +623,7 @@ int tst_netdev_remove_traffic_class(const char *file, const int lineno,
 int tst_netdev_add_traffic_filter(const char *file, const int lineno,
 	int strict, const char *ifname, unsigned int parent,
 	unsigned int handle, unsigned int protocol, unsigned int priority,
-	const char *f_kind, const struct tst_rtnl_attr_list *config)
+	const char *f_kind, const struct tst_netlink_attr_list *config)
 {
 	return modify_qdisc(file, lineno, strict, "traffic filter",
 		RTM_NEWTFILTER, NLM_F_CREATE | NLM_F_EXCL, ifname, AF_UNSPEC,
diff --git a/lib/tst_netlink.c b/lib/tst_netlink.c
index bd05df81a..7bc98af56 100644
--- a/lib/tst_netlink.c
+++ b/lib/tst_netlink.c
@@ -283,6 +283,86 @@ int tst_netlink_add_message(const char *file, const int lineno,
 	return 1;
 }
 
+int tst_netlink_add_attr(const char *file, const int lineno,
+	struct tst_netlink_context *ctx, unsigned short type,
+	const void *data, unsigned short len)
+{
+	size_t size = NLA_HDRLEN + NLA_ALIGN(len);
+	struct nlattr *attr;
+
+	if (!ctx->curmsg) {
+		tst_brk_(file, lineno, TBROK,
+			"%s(): No message to add attributes to", __func__);
+		return 0;
+	}
+
+	if (!netlink_grow_buffer(file, lineno, ctx, size))
+		return 0;
+
+	size = NLMSG_ALIGN(ctx->curmsg->nlmsg_len);
+	attr = (struct nlattr *)(((char *)ctx->curmsg) + size);
+	attr->nla_type = type;
+	attr->nla_len = NLA_HDRLEN + len;
+	memcpy(((char *)attr) + NLA_HDRLEN, data, len);
+	ctx->curmsg->nlmsg_len = size + attr->nla_len;
+	ctx->datalen = NLMSG_ALIGN(ctx->datalen) + attr->nla_len;
+
+	return 1;
+}
+
+int tst_netlink_add_attr_string(const char *file, const int lineno,
+	struct tst_netlink_context *ctx, unsigned short type,
+	const char *data)
+{
+	return tst_netlink_add_attr(file, lineno, ctx, type, data,
+		strlen(data) + 1);
+}
+
+int tst_netlink_add_attr_list(const char *file, const int lineno,
+	struct tst_netlink_context *ctx,
+	const struct tst_netlink_attr_list *list)
+{
+	int i, ret;
+	size_t offset;
+
+	for (i = 0; list[i].len >= 0; i++) {
+		if (list[i].len > USHRT_MAX) {
+			tst_brk_(file, lineno, TBROK,
+				"%s(): Attribute value too long", __func__);
+			return -1;
+		}
+
+		offset = NLMSG_ALIGN(ctx->datalen);
+		ret = tst_netlink_add_attr(file, lineno, ctx, list[i].type,
+			list[i].data, list[i].len);
+
+		if (!ret)
+			return -1;
+
+		if (list[i].sublist) {
+			struct rtattr *attr;
+
+			ret = tst_netlink_add_attr_list(file, lineno, ctx,
+				list[i].sublist);
+
+			if (ret < 0)
+				return ret;
+
+			attr = (struct rtattr *)(ctx->buffer + offset);
+
+			if (ctx->datalen - offset > USHRT_MAX) {
+				tst_brk_(file, lineno, TBROK,
+					"%s(): Sublist too long", __func__);
+				return -1;
+			}
+
+			attr->rta_len = ctx->datalen - offset;
+		}
+	}
+
+	return i;
+}
+
 int tst_rtnl_add_attr(const char *file, const int lineno,
 	struct tst_netlink_context *ctx, unsigned short type,
 	const void *data, unsigned short len)
@@ -320,7 +400,7 @@ int tst_rtnl_add_attr_string(const char *file, const int lineno,
 
 int tst_rtnl_add_attr_list(const char *file, const int lineno,
 	struct tst_netlink_context *ctx,
-	const struct tst_rtnl_attr_list *list)
+	const struct tst_netlink_attr_list *list)
 {
 	int i, ret;
 	size_t offset;
diff --git a/testcases/cve/tcindex01.c b/testcases/cve/tcindex01.c
index d441a2449..b4f2bb01a 100644
--- a/testcases/cve/tcindex01.c
+++ b/testcases/cve/tcindex01.c
@@ -46,15 +46,15 @@ static const struct tc_htb_glob qd_opt = {
 static struct tc_htb_opt cls_opt = {};
 
 /* htb qdisc and class options */
-static const struct tst_rtnl_attr_list qd_config[] = {
-	{TCA_OPTIONS, NULL, 0, (const struct tst_rtnl_attr_list[]){
+static const struct tst_netlink_attr_list qd_config[] = {
+	{TCA_OPTIONS, NULL, 0, (const struct tst_netlink_attr_list[]){
 		{TCA_HTB_INIT, &qd_opt, sizeof(qd_opt), NULL},
 		{0, NULL, -1, NULL}
 	}},
 	{0, NULL, -1, NULL}
 };
-static const struct tst_rtnl_attr_list cls_config[] = {
-	{TCA_OPTIONS, NULL, 0, (const struct tst_rtnl_attr_list[]){
+static const struct tst_netlink_attr_list cls_config[] = {
+	{TCA_OPTIONS, NULL, 0, (const struct tst_netlink_attr_list[]){
 		{TCA_HTB_PARMS, &cls_opt, sizeof(cls_opt), NULL},
 		{0, NULL, -1, NULL}
 	}},
@@ -62,8 +62,8 @@ static const struct tst_rtnl_attr_list cls_config[] = {
 };
 
 /* tcindex filter options */
-static const struct tst_rtnl_attr_list f_config[] = {
-	{TCA_OPTIONS, NULL, 0, (const struct tst_rtnl_attr_list[]){
+static const struct tst_netlink_attr_list f_config[] = {
+	{TCA_OPTIONS, NULL, 0, (const struct tst_netlink_attr_list[]){
 		{TCA_TCINDEX_MASK, &mask, sizeof(mask), NULL},
 		{TCA_TCINDEX_SHIFT, &shift, sizeof(shift), NULL},
 		{TCA_TCINDEX_CLASSID, &clsid, sizeof(clsid), NULL},
-- 
2.42.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v3 1/4] tst_netlink: Add helper functions for handling generic attributes

[Petr Vorel]

Hi Martin,

> Refactor struct tst_rtnl_attr_list for generic use and add helper
> functions for handling generic struct nlattr message attributes.

There is still tst_rtnl_attr_list mentioned at doc/C-Test-Network-API.asciidoc,
could you please update that?

...
> -/* Add arbitrary attribute to last message */
> +/* Add arbitrary nlattr attribute to last message */
> +int tst_netlink_add_attr(const char *file, const int lineno,
> +	struct tst_netlink_context *ctx, unsigned short type, const void *data,
> +	unsigned short len);
> +#define NETLINK_ADD_ATTR(ctx, type, data, len) \
> +	tst_netlink_add_attr(__FILE__, __LINE__, (ctx), (type), (data), (len))
> +
> +/* Add string nlattr attribute to last message */
> +int tst_netlink_add_attr_string(const char *file, const int lineno,
> +	struct tst_netlink_context *ctx, unsigned short type, const char *data);
> +#define NETLINK_ADD_ATTR_STRING(ctx, type, data) \
> +	tst_netlink_add_attr_string(__FILE__, __LINE__, (ctx), (type), (data))

FYI NETLINK_ADD_ATTR_STRING() is not used anywhere. I suppose you plan to use it
in some of the following CVE tests, right?

...
> +int tst_netlink_add_attr_list(const char *file, const int lineno,
> +	struct tst_netlink_context *ctx,
> +	const struct tst_netlink_attr_list *list)
> +{
> +	int i, ret;
> +	size_t offset;
> +
> +	for (i = 0; list[i].len >= 0; i++) {
> +		if (list[i].len > USHRT_MAX) {
> +			tst_brk_(file, lineno, TBROK,
> +				"%s(): Attribute value too long", __func__);
> +			return -1;
Here (and on other function working on more items) is return -1 on error and >=
0 on success.  The other functions return 0 on error and 1 on success.
While this is logic, it would still be nice to describe return in tst_netlink.h.

The rest LGTM.

Reviewed-by: Petr Vorel <pvorel@suse.cz>

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v3 2/4] tst_netlink_check_acks(): Stop on first error regardless of ACK request

[Martin Doucha]

tst_netlink_check_acks() currently ignores errors in response
to netlink messages that did not explicitly ask for ack.
If a multimessage netlink request fails early, this will lead
to TBROK failures due to missing acks. Return the first netlink
error even if the corresponding request did not ask for ack.

Signed-off-by: Martin Doucha <mdoucha@suse.cz>
---

Changes since v2: New patch

 lib/tst_netlink.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/lib/tst_netlink.c b/lib/tst_netlink.c
index 7bc98af56..d61cc8b88 100644
--- a/lib/tst_netlink.c
+++ b/lib/tst_netlink.c
@@ -455,18 +455,19 @@ int tst_netlink_check_acks(const char *file, const int lineno,
 		if (!(msg->nlmsg_flags & NLM_F_ACK))
 			continue;
 
-		while (res->header && res->header->nlmsg_seq != msg->nlmsg_seq)
+		while (res->header && !(res->err && res->err->error) &&
+			res->header->nlmsg_seq != msg->nlmsg_seq)
 			res++;
 
-		if (!res->err || res->header->nlmsg_seq != msg->nlmsg_seq) {
-			tst_brk_(file, lineno, TBROK,
-				"No ACK found for Netlink message %u",
-				msg->nlmsg_seq);
+		if (res->err && res->err->error) {
+			tst_netlink_errno = -res->err->error;
 			return 0;
 		}
 
-		if (res->err->error) {
-			tst_netlink_errno = -res->err->error;
+		if (!res->header || res->header->nlmsg_seq != msg->nlmsg_seq) {
+			tst_brk_(file, lineno, TBROK,
+				"No ACK found for Netlink message %u",
+				msg->nlmsg_seq);
 			return 0;
 		}
 	}
-- 
2.42.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v3 2/4] tst_netlink_check_acks(): Stop on first error regardless of ACK request

[Petr Vorel]

Hi Martin,

> tst_netlink_check_acks() currently ignores errors in response
> to netlink messages that did not explicitly ask for ack.
> If a multimessage netlink request fails early, this will lead
> to TBROK failures due to missing acks. Return the first netlink
> error even if the corresponding request did not ask for ack.

+1

Reviewed-by: Petr Vorel <pvorel@suse.cz>

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v3 3/4] Add lapi/nf_tables.h

[Martin Doucha]

Signed-off-by: Martin Doucha <mdoucha@suse.cz>
---

Changes since v2: New patch

 configure.ac             |  1 +
 include/lapi/nf_tables.h | 19 +++++++++++++++++++
 2 files changed, 20 insertions(+)
 create mode 100644 include/lapi/nf_tables.h

diff --git a/configure.ac b/configure.ac
index 3fa350f9e..e1a10a10e 100644
--- a/configure.ac
+++ b/configure.ac
@@ -35,6 +35,7 @@ AC_PREFIX_DEFAULT(/opt/ltp)
 
 AC_CHECK_DECLS([IFLA_NET_NS_PID],,,[#include <linux/if_link.h>])
 AC_CHECK_DECLS([MADV_MERGEABLE],,,[#include <sys/mman.h>])
+AC_CHECK_DECLS([NFTA_CHAIN_ID, NFTA_VERDICT_CHAIN_ID],,,[#include <linux/netfilter/nf_tables.h>])
 AC_CHECK_DECLS([PR_CAPBSET_DROP, PR_CAPBSET_READ],,,[#include <sys/prctl.h>])
 AC_CHECK_DECLS([SEM_STAT_ANY],,,[#include <sys/sem.h>])
 
diff --git a/include/lapi/nf_tables.h b/include/lapi/nf_tables.h
new file mode 100644
index 000000000..6b751f9cc
--- /dev/null
+++ b/include/lapi/nf_tables.h
@@ -0,0 +1,19 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Copyright (c) 2021 Petr Vorel <petr.vorel@gmail.com>
+ */
+
+#ifndef LAPI_NF_TABLES_H__
+#define LAPI_NF_TABLES_H__
+
+#include <linux/netfilter/nf_tables.h>
+
+#ifndef HAVE_DECL_NFTA_CHAIN_ID
+# define NFTA_CHAIN_ID 11
+#endif
+
+#ifndef HAVE_DECL_NFTA_VERDICT_CHAIN_ID
+# define NFTA_VERDICT_CHAIN_ID 3
+#endif
+
+#endif /* LAPI_NF_TABLES_H__ */
-- 
2.42.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v3 3/4] Add lapi/nf_tables.h

[Petr Vorel]

Hi Martin,

...
> +++ b/include/lapi/nf_tables.h
> @@ -0,0 +1,19 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (c) 2021 Petr Vorel <petr.vorel@gmail.com>
Well, here should be your copyright :).

Copyright (C) 2023 SUSE LLC <mdoucha@suse.cz>

I can change it before merge.
Reviewed-by: Petr Vorel <pvorel@suse.cz>

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v3 3/4] Add lapi/nf_tables.h

[Martin Doucha]

On 21. 11. 23 11:10, Petr Vorel wrote:
> Hi Martin,
> 
> ...
>> +++ b/include/lapi/nf_tables.h
>> @@ -0,0 +1,19 @@
>> +/* SPDX-License-Identifier: GPL-2.0-or-later */
>> +/*
>> + * Copyright (c) 2021 Petr Vorel <petr.vorel@gmail.com>
> Well, here should be your copyright :).
> 
> Copyright (C) 2023 SUSE LLC <mdoucha@suse.cz>
> 
> I can change it before merge.

Yes, sorry, please do.

> Reviewed-by: Petr Vorel <pvorel@suse.cz>
> 
> Kind regards,
> Petr

-- 
Martin Doucha   mdoucha@suse.cz
SW Quality Engineer
SUSE LINUX, s.r.o.
CORSO IIa
Krizikova 148/34
186 00 Prague 8
Czech Republic


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v3 4/4] Add test for CVE 2023-31248

[Martin Doucha]

Fixes #1058

Signed-off-by: Martin Doucha <mdoucha@suse.cz>
Co-Developed-by: Souta Kawahara <souta.kawahara@miraclelinux.com>
---

Changes since v1: New patch
Changes since v2:
- Use netfilter GOTO rule jumping into the deleted chain
- Check for ENOENT error instead of kernel taint

The test does not use any external utilities so I've decided not to add it
to the net.tcp_cmds runfile.

 runtest/cve                           |   1 +
 testcases/network/iptables/.gitignore |   1 +
 testcases/network/iptables/Makefile   |   2 +-
 testcases/network/iptables/nft02.c    | 213 ++++++++++++++++++++++++++
 4 files changed, 216 insertions(+), 1 deletion(-)
 create mode 100644 testcases/network/iptables/.gitignore
 create mode 100644 testcases/network/iptables/nft02.c

diff --git a/runtest/cve b/runtest/cve
index 569558af2..1d1d87597 100644
--- a/runtest/cve
+++ b/runtest/cve
@@ -86,6 +86,7 @@ cve-2022-2590 dirtyc0w_shmem
 cve-2022-23222 bpf_prog07
 cve-2023-1829 tcindex01
 cve-2023-0461 setsockopt10
+cve-2023-31248 nft02
 # Tests below may cause kernel memory leak
 cve-2020-25704 perf_event_open03
 cve-2022-0185 fsconfig03
diff --git a/testcases/network/iptables/.gitignore b/testcases/network/iptables/.gitignore
new file mode 100644
index 000000000..0f47a7313
--- /dev/null
+++ b/testcases/network/iptables/.gitignore
@@ -0,0 +1 @@
+nft02
diff --git a/testcases/network/iptables/Makefile b/testcases/network/iptables/Makefile
index 1b42f25db..02e228cbc 100644
--- a/testcases/network/iptables/Makefile
+++ b/testcases/network/iptables/Makefile
@@ -5,7 +5,7 @@
 
 top_srcdir		?= ../../..
 
-include $(top_srcdir)/include/mk/env_pre.mk
+include $(top_srcdir)/include/mk/testcases.mk
 
 INSTALL_TARGETS		:= *.sh
 
diff --git a/testcases/network/iptables/nft02.c b/testcases/network/iptables/nft02.c
new file mode 100644
index 000000000..a6e795af3
--- /dev/null
+++ b/testcases/network/iptables/nft02.c
@@ -0,0 +1,213 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2023 SUSE LLC
+ * Author: Marcos Paulo de Souza <mpdesouza@suse.com>
+ * LTP port: Martin Doucha <mdoucha@suse.cz>
+ */
+
+/*\
+ * CVE-2023-31248
+ *
+ * Test for use-after-free when adding a new rule to a chain deleted
+ * in the same netlink message batch.
+ *
+ * Kernel bug fixed in:
+ *
+ *  commit 515ad530795c118f012539ed76d02bacfd426d89
+ *  Author: Thadeu Lima de Souza Cascardo <cascardo@canonical.com>
+ *  Date:   Wed Jul 5 09:12:55 2023 -0300
+ *
+ *  netfilter: nf_tables: do not ignore genmask when looking up chain by id
+ */
+
+#include <linux/netlink.h>
+#include <linux/tcp.h>
+#include <arpa/inet.h>
+#include <linux/netfilter.h>
+#include "lapi/nf_tables.h"
+#include <linux/netfilter/nfnetlink.h>
+#include "tst_test.h"
+#include "tst_netlink.h"
+
+#define TABNAME "ltp_table1"
+#define SRCCHAIN "ltp_chain_src"
+#define DESTCHAIN "ltp_chain_dest"
+
+static uint32_t chain_id;
+static uint32_t imm_dreg, imm_verdict;
+static struct tst_netlink_context *ctx;
+
+/* Table creation config */
+static const struct tst_netlink_attr_list table_config[] = {
+	{NFTA_TABLE_NAME, TABNAME, strlen(TABNAME) + 1, NULL},
+	{0, NULL, -1, NULL}
+};
+
+/* Chain creation and deletion config */
+static const struct tst_netlink_attr_list destchain_config[] = {
+	{NFTA_TABLE_NAME, TABNAME, strlen(TABNAME) + 1, NULL},
+	{NFTA_CHAIN_NAME, DESTCHAIN, strlen(DESTCHAIN) + 1, NULL},
+	{NFTA_CHAIN_ID, &chain_id, sizeof(chain_id), NULL},
+	{0, NULL, -1, NULL}
+};
+
+static const struct tst_netlink_attr_list delchain_config[] = {
+	{NFTA_TABLE_NAME, TABNAME, strlen(TABNAME) + 1, NULL},
+	{NFTA_CHAIN_NAME, DESTCHAIN, strlen(DESTCHAIN) + 1, NULL},
+	{0, NULL, -1, NULL}
+};
+
+static const struct tst_netlink_attr_list srcchain_config[] = {
+	{NFTA_TABLE_NAME, TABNAME, strlen(TABNAME) + 1, NULL},
+	{NFTA_CHAIN_NAME, SRCCHAIN, strlen(SRCCHAIN) + 1, NULL},
+	{0, NULL, -1, NULL}
+};
+
+/* Rule creation config */
+static const struct tst_netlink_attr_list rule_verdict_config[] = {
+	{NFTA_VERDICT_CODE, &imm_verdict, sizeof(imm_verdict), NULL},
+	{NFTA_VERDICT_CHAIN_ID, &chain_id, sizeof(chain_id), NULL},
+	{0, NULL, -1, NULL}
+};
+
+static const struct tst_netlink_attr_list rule_data_config[] = {
+	{NFTA_IMMEDIATE_DREG, &imm_dreg, sizeof(imm_dreg), NULL},
+	{NFTA_IMMEDIATE_DATA, NULL, 0, (const struct tst_netlink_attr_list[]) {
+		{NFTA_DATA_VERDICT, NULL, 0, rule_verdict_config},
+		{0, NULL, -1, NULL}
+	}},
+	{0, NULL, -1, NULL}
+};
+
+static const struct tst_netlink_attr_list rule_expr_config[] = {
+	{NFTA_LIST_ELEM, NULL, 0, (const struct tst_netlink_attr_list[]) {
+		{NFTA_EXPR_NAME, "immediate", 10, NULL},
+		{NFTA_EXPR_DATA, NULL, 0, rule_data_config},
+		{0, NULL, -1, NULL}
+	}},
+	{0, NULL, -1, NULL}
+};
+
+static const struct tst_netlink_attr_list rule_config[] = {
+	{NFTA_RULE_EXPRESSIONS, NULL, 0, rule_expr_config},
+	{NFTA_RULE_TABLE, TABNAME, strlen(TABNAME) + 1, NULL},
+	{NFTA_RULE_CHAIN, SRCCHAIN, strlen(SRCCHAIN) + 1, NULL},
+	{0, NULL, -1, NULL}
+};
+
+static void setup(void)
+{
+	tst_setup_netns();
+
+	chain_id = htonl(77);
+	imm_dreg = htonl(NFT_REG_VERDICT);
+	imm_verdict = htonl(NFT_GOTO);
+}
+
+static void run(void)
+{
+	int ret;
+	struct nlmsghdr header;
+	struct nfgenmsg nfpayload;
+
+	memset(&header, 0, sizeof(header));
+	memset(&nfpayload, 0, sizeof(nfpayload));
+	nfpayload.version = NFNETLINK_V0;
+
+	ctx = NETLINK_CREATE_CONTEXT(NETLINK_NETFILTER);
+
+	/* Start netfilter batch */
+	header.nlmsg_type = NFNL_MSG_BATCH_BEGIN;
+	header.nlmsg_flags = NLM_F_REQUEST;
+	nfpayload.nfgen_family = AF_UNSPEC;
+	nfpayload.res_id = htons(NFNL_SUBSYS_NFTABLES);
+	NETLINK_ADD_MESSAGE(ctx, &header, &nfpayload, sizeof(nfpayload));
+
+	/* Add table */
+	header.nlmsg_type = (NFNL_SUBSYS_NFTABLES << 8) | NFT_MSG_NEWTABLE;
+	header.nlmsg_flags = NLM_F_REQUEST | NLM_F_CREATE;
+	nfpayload.nfgen_family = NFPROTO_IPV4;
+	nfpayload.res_id = htons(0);
+	NETLINK_ADD_MESSAGE(ctx, &header, &nfpayload, sizeof(nfpayload));
+	NETLINK_ADD_ATTR_LIST(ctx, table_config);
+
+	/* Add destination chain */
+	header.nlmsg_type = (NFNL_SUBSYS_NFTABLES << 8) | NFT_MSG_NEWCHAIN;
+	header.nlmsg_flags = NLM_F_REQUEST | NLM_F_CREATE;
+	nfpayload.nfgen_family = NFPROTO_IPV4;
+	nfpayload.res_id = htons(0);
+	NETLINK_ADD_MESSAGE(ctx, &header, &nfpayload, sizeof(nfpayload));
+	NETLINK_ADD_ATTR_LIST(ctx, destchain_config);
+
+	/* Delete destination chain */
+	header.nlmsg_type = (NFNL_SUBSYS_NFTABLES << 8) | NFT_MSG_DELCHAIN;
+	header.nlmsg_flags = NLM_F_REQUEST;
+	nfpayload.nfgen_family = NFPROTO_IPV4;
+	nfpayload.res_id = htons(0);
+	NETLINK_ADD_MESSAGE(ctx, &header, &nfpayload, sizeof(nfpayload));
+	NETLINK_ADD_ATTR_LIST(ctx, delchain_config);
+
+	/* Add destination chain */
+	header.nlmsg_type = (NFNL_SUBSYS_NFTABLES << 8) | NFT_MSG_NEWCHAIN;
+	header.nlmsg_flags = NLM_F_REQUEST | NLM_F_CREATE;
+	nfpayload.nfgen_family = NFPROTO_IPV4;
+	nfpayload.res_id = htons(0);
+	NETLINK_ADD_MESSAGE(ctx, &header, &nfpayload, sizeof(nfpayload));
+	NETLINK_ADD_ATTR_LIST(ctx, srcchain_config);
+
+	/* Add rule to source chain. Require ACK and check for ENOENT error. */
+	header.nlmsg_type = (NFNL_SUBSYS_NFTABLES << 8) | NFT_MSG_NEWRULE;
+	header.nlmsg_flags = NLM_F_REQUEST | NLM_F_APPEND | NLM_F_CREATE |
+		NLM_F_ACK;
+	nfpayload.nfgen_family = NFPROTO_IPV4;
+	nfpayload.res_id = htons(0);
+	NETLINK_ADD_MESSAGE(ctx, &header, &nfpayload, sizeof(nfpayload));
+	NETLINK_ADD_ATTR_LIST(ctx, rule_config);
+
+	/* End batch */
+	header.nlmsg_type = NFNL_MSG_BATCH_END;
+	header.nlmsg_flags = NLM_F_REQUEST;
+	nfpayload.nfgen_family = AF_UNSPEC;
+	nfpayload.res_id = htons(NFNL_SUBSYS_NFTABLES);
+	NETLINK_ADD_MESSAGE(ctx, &header, &nfpayload, sizeof(nfpayload));
+
+	ret = NETLINK_SEND_VALIDATE(ctx);
+	TST_ERR = tst_netlink_errno;
+	NETLINK_DESTROY_CONTEXT(ctx);
+	ctx = NULL;
+
+	if (ret)
+		tst_res(TFAIL, "Netfilter chain list is corrupted");
+	else if (TST_ERR == ENOENT)
+		tst_res(TPASS, "Deleted netfilter chain cannot be referenced");
+	else if (TST_ERR == EOPNOTSUPP || TST_ERR == EINVAL)
+		tst_brk(TCONF, "Test requires unavailable netfilter features");
+	else
+		tst_brk(TBROK | TTERRNO, "Unknown nfnetlink error");
+}
+
+static void cleanup(void)
+{
+	NETLINK_DESTROY_CONTEXT(ctx);
+}
+
+static struct tst_test test = {
+	.test_all = run,
+	.setup = setup,
+	.cleanup = cleanup,
+	.taint_check = TST_TAINT_W | TST_TAINT_D,
+	.needs_kconfigs = (const char *[]) {
+		"CONFIG_USER_NS=y",
+		"CONFIG_NF_TABLES",
+		NULL
+	},
+	.save_restore = (const struct tst_path_val[]) {
+		{"/proc/sys/user/max_user_namespaces", "1024", TST_SR_SKIP},
+		{}
+	},
+	.tags = (const struct tst_tag[]) {
+		{"linux-git", "515ad530795c"},
+		{"CVE", "2023-31248"},
+		{}
+	}
+};
-- 
2.42.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v3 4/4] Add test for CVE 2023-31248

[Petr Vorel]

Hi Martin, Souta,

nice work, few very minor nits below.

> Fixes #1058

> Signed-off-by: Martin Doucha <mdoucha@suse.cz>
> Co-Developed-by: Souta Kawahara <souta.kawahara@miraclelinux.com>
> ---

> Changes since v1: New patch
> Changes since v2:
> - Use netfilter GOTO rule jumping into the deleted chain
> - Check for ENOENT error instead of kernel taint

> The test does not use any external utilities so I've decided not to add it
> to the net.tcp_cmds runfile.

Sure.

>  runtest/cve                           |   1 +
>  testcases/network/iptables/.gitignore |   1 +
>  testcases/network/iptables/Makefile   |   2 +-
>  testcases/network/iptables/nft02.c    | 213 ++++++++++++++++++++++++++
>  4 files changed, 216 insertions(+), 1 deletion(-)
>  create mode 100644 testcases/network/iptables/.gitignore
>  create mode 100644 testcases/network/iptables/nft02.c

> diff --git a/runtest/cve b/runtest/cve
> index 569558af2..1d1d87597 100644
> --- a/runtest/cve
> +++ b/runtest/cve
> @@ -86,6 +86,7 @@ cve-2022-2590 dirtyc0w_shmem
>  cve-2022-23222 bpf_prog07
>  cve-2023-1829 tcindex01
>  cve-2023-0461 setsockopt10
> +cve-2023-31248 nft02
>  # Tests below may cause kernel memory leak
>  cve-2020-25704 perf_event_open03
>  cve-2022-0185 fsconfig03
> diff --git a/testcases/network/iptables/.gitignore b/testcases/network/iptables/.gitignore
> new file mode 100644
> index 000000000..0f47a7313
> --- /dev/null
> +++ b/testcases/network/iptables/.gitignore
> @@ -0,0 +1 @@
> +nft02
> diff --git a/testcases/network/iptables/Makefile b/testcases/network/iptables/Makefile
> index 1b42f25db..02e228cbc 100644
> --- a/testcases/network/iptables/Makefile
> +++ b/testcases/network/iptables/Makefile
> @@ -5,7 +5,7 @@

>  top_srcdir		?= ../../..

> -include $(top_srcdir)/include/mk/env_pre.mk
> +include $(top_srcdir)/include/mk/testcases.mk

>  INSTALL_TARGETS		:= *.sh

> diff --git a/testcases/network/iptables/nft02.c b/testcases/network/iptables/nft02.c
> new file mode 100644
> index 000000000..a6e795af3
> --- /dev/null
> +++ b/testcases/network/iptables/nft02.c
> @@ -0,0 +1,213 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (C) 2023 SUSE LLC
> + * Author: Marcos Paulo de Souza <mpdesouza@suse.com>
> + * LTP port: Martin Doucha <mdoucha@suse.cz>
> + */
> +
> +/*\
We usually add [Description] here. Although it looks bogus to me, I can add it
before merge.

> + * CVE-2023-31248
> + *
> + * Test for use-after-free when adding a new rule to a chain deleted
> + * in the same netlink message batch.
> + *
> + * Kernel bug fixed in:
> + *
> + *  commit 515ad530795c118f012539ed76d02bacfd426d89
> + *  Author: Thadeu Lima de Souza Cascardo <cascardo@canonical.com>
> + *  Date:   Wed Jul 5 09:12:55 2023 -0300
> + *
> + *  netfilter: nf_tables: do not ignore genmask when looking up chain by id
> + */
> +
> +#include <linux/netlink.h>
> +#include <linux/tcp.h>
> +#include <arpa/inet.h>
> +#include <linux/netfilter.h>
> +#include "lapi/nf_tables.h"
> +#include <linux/netfilter/nfnetlink.h>
> +#include "tst_test.h"
> +#include "tst_netlink.h"
> +
> +#define TABNAME "ltp_table1"
> +#define SRCCHAIN "ltp_chain_src"
> +#define DESTCHAIN "ltp_chain_dest"
> +
> +static uint32_t chain_id;
> +static uint32_t imm_dreg, imm_verdict;
> +static struct tst_netlink_context *ctx;
> +
> +/* Table creation config */
> +static const struct tst_netlink_attr_list table_config[] = {
> +	{NFTA_TABLE_NAME, TABNAME, strlen(TABNAME) + 1, NULL},
> +	{0, NULL, -1, NULL}
> +};
> +
> +/* Chain creation and deletion config */
> +static const struct tst_netlink_attr_list destchain_config[] = {
> +	{NFTA_TABLE_NAME, TABNAME, strlen(TABNAME) + 1, NULL},
> +	{NFTA_CHAIN_NAME, DESTCHAIN, strlen(DESTCHAIN) + 1, NULL},
> +	{NFTA_CHAIN_ID, &chain_id, sizeof(chain_id), NULL},
> +	{0, NULL, -1, NULL}
> +};
> +
> +static const struct tst_netlink_attr_list delchain_config[] = {
> +	{NFTA_TABLE_NAME, TABNAME, strlen(TABNAME) + 1, NULL},
> +	{NFTA_CHAIN_NAME, DESTCHAIN, strlen(DESTCHAIN) + 1, NULL},
> +	{0, NULL, -1, NULL}
> +};
> +
> +static const struct tst_netlink_attr_list srcchain_config[] = {
> +	{NFTA_TABLE_NAME, TABNAME, strlen(TABNAME) + 1, NULL},
> +	{NFTA_CHAIN_NAME, SRCCHAIN, strlen(SRCCHAIN) + 1, NULL},
> +	{0, NULL, -1, NULL}
> +};
> +
> +/* Rule creation config */
> +static const struct tst_netlink_attr_list rule_verdict_config[] = {
> +	{NFTA_VERDICT_CODE, &imm_verdict, sizeof(imm_verdict), NULL},
> +	{NFTA_VERDICT_CHAIN_ID, &chain_id, sizeof(chain_id), NULL},
> +	{0, NULL, -1, NULL}
> +};
> +
> +static const struct tst_netlink_attr_list rule_data_config[] = {
> +	{NFTA_IMMEDIATE_DREG, &imm_dreg, sizeof(imm_dreg), NULL},
> +	{NFTA_IMMEDIATE_DATA, NULL, 0, (const struct tst_netlink_attr_list[]) {
> +		{NFTA_DATA_VERDICT, NULL, 0, rule_verdict_config},
> +		{0, NULL, -1, NULL}
> +	}},
> +	{0, NULL, -1, NULL}
> +};
> +
> +static const struct tst_netlink_attr_list rule_expr_config[] = {
> +	{NFTA_LIST_ELEM, NULL, 0, (const struct tst_netlink_attr_list[]) {
> +		{NFTA_EXPR_NAME, "immediate", 10, NULL},
> +		{NFTA_EXPR_DATA, NULL, 0, rule_data_config},
> +		{0, NULL, -1, NULL}
> +	}},
> +	{0, NULL, -1, NULL}
> +};
> +
> +static const struct tst_netlink_attr_list rule_config[] = {
> +	{NFTA_RULE_EXPRESSIONS, NULL, 0, rule_expr_config},
> +	{NFTA_RULE_TABLE, TABNAME, strlen(TABNAME) + 1, NULL},
> +	{NFTA_RULE_CHAIN, SRCCHAIN, strlen(SRCCHAIN) + 1, NULL},
> +	{0, NULL, -1, NULL}
> +};
> +
> +static void setup(void)
> +{
> +	tst_setup_netns();
> +
> +	chain_id = htonl(77);
nit: Although it's obvious that ID chain_id is some random number I would define
77 also above.

> +	imm_dreg = htonl(NFT_REG_VERDICT);
> +	imm_verdict = htonl(NFT_GOTO);
> +}
> +
> +static void run(void)
> +{
> +	int ret;
> +	struct nlmsghdr header;
> +	struct nfgenmsg nfpayload;
> +
> +	memset(&header, 0, sizeof(header));
> +	memset(&nfpayload, 0, sizeof(nfpayload));
> +	nfpayload.version = NFNETLINK_V0;
> +
> +	ctx = NETLINK_CREATE_CONTEXT(NETLINK_NETFILTER);
> +
> +	/* Start netfilter batch */
> +	header.nlmsg_type = NFNL_MSG_BATCH_BEGIN;
> +	header.nlmsg_flags = NLM_F_REQUEST;
> +	nfpayload.nfgen_family = AF_UNSPEC;
> +	nfpayload.res_id = htons(NFNL_SUBSYS_NFTABLES);
> +	NETLINK_ADD_MESSAGE(ctx, &header, &nfpayload, sizeof(nfpayload));
> +
> +	/* Add table */
> +	header.nlmsg_type = (NFNL_SUBSYS_NFTABLES << 8) | NFT_MSG_NEWTABLE;
> +	header.nlmsg_flags = NLM_F_REQUEST | NLM_F_CREATE;
> +	nfpayload.nfgen_family = NFPROTO_IPV4;
> +	nfpayload.res_id = htons(0);
> +	NETLINK_ADD_MESSAGE(ctx, &header, &nfpayload, sizeof(nfpayload));
> +	NETLINK_ADD_ATTR_LIST(ctx, table_config);
> +
> +	/* Add destination chain */
> +	header.nlmsg_type = (NFNL_SUBSYS_NFTABLES << 8) | NFT_MSG_NEWCHAIN;
> +	header.nlmsg_flags = NLM_F_REQUEST | NLM_F_CREATE;
> +	nfpayload.nfgen_family = NFPROTO_IPV4;
> +	nfpayload.res_id = htons(0);
> +	NETLINK_ADD_MESSAGE(ctx, &header, &nfpayload, sizeof(nfpayload));
> +	NETLINK_ADD_ATTR_LIST(ctx, destchain_config);
> +
> +	/* Delete destination chain */
> +	header.nlmsg_type = (NFNL_SUBSYS_NFTABLES << 8) | NFT_MSG_DELCHAIN;
> +	header.nlmsg_flags = NLM_F_REQUEST;
> +	nfpayload.nfgen_family = NFPROTO_IPV4;
> +	nfpayload.res_id = htons(0);
> +	NETLINK_ADD_MESSAGE(ctx, &header, &nfpayload, sizeof(nfpayload));
> +	NETLINK_ADD_ATTR_LIST(ctx, delchain_config);
> +
> +	/* Add destination chain */
nit: this looks to be source chain
Out of curriosity I'm looking at the reproducer
(https://bugzilla.suse.com/attachment.cgi?id=868806)
and it needs just single chain.
But test needs both for some reason.
Anyway, nice work, kernel oops printed to dmesg on older kernel.

Reviewed-by: Petr Vorel <pvorel@suse.cz>
Tested-by: Petr Vorel <pvorel@suse.cz>

> +	header.nlmsg_type = (NFNL_SUBSYS_NFTABLES << 8) | NFT_MSG_NEWCHAIN;
> +	header.nlmsg_flags = NLM_F_REQUEST | NLM_F_CREATE;
> +	nfpayload.nfgen_family = NFPROTO_IPV4;
> +	nfpayload.res_id = htons(0);
> +	NETLINK_ADD_MESSAGE(ctx, &header, &nfpayload, sizeof(nfpayload));
> +	NETLINK_ADD_ATTR_LIST(ctx, srcchain_config);
> +
> +	/* Add rule to source chain. Require ACK and check for ENOENT error. */
> +	header.nlmsg_type = (NFNL_SUBSYS_NFTABLES << 8) | NFT_MSG_NEWRULE;
> +	header.nlmsg_flags = NLM_F_REQUEST | NLM_F_APPEND | NLM_F_CREATE |
> +		NLM_F_ACK;
> +	nfpayload.nfgen_family = NFPROTO_IPV4;
> +	nfpayload.res_id = htons(0);
> +	NETLINK_ADD_MESSAGE(ctx, &header, &nfpayload, sizeof(nfpayload));
> +	NETLINK_ADD_ATTR_LIST(ctx, rule_config);
> +
> +	/* End batch */
> +	header.nlmsg_type = NFNL_MSG_BATCH_END;
> +	header.nlmsg_flags = NLM_F_REQUEST;
> +	nfpayload.nfgen_family = AF_UNSPEC;
> +	nfpayload.res_id = htons(NFNL_SUBSYS_NFTABLES);
> +	NETLINK_ADD_MESSAGE(ctx, &header, &nfpayload, sizeof(nfpayload));
> +
> +	ret = NETLINK_SEND_VALIDATE(ctx);
> +	TST_ERR = tst_netlink_errno;
> +	NETLINK_DESTROY_CONTEXT(ctx);
> +	ctx = NULL;
> +
> +	if (ret)
> +		tst_res(TFAIL, "Netfilter chain list is corrupted");
> +	else if (TST_ERR == ENOENT)
> +		tst_res(TPASS, "Deleted netfilter chain cannot be referenced");
> +	else if (TST_ERR == EOPNOTSUPP || TST_ERR == EINVAL)
> +		tst_brk(TCONF, "Test requires unavailable netfilter features");
> +	else
> +		tst_brk(TBROK | TTERRNO, "Unknown nfnetlink error");
> +}
> +
> +static void cleanup(void)
> +{
> +	NETLINK_DESTROY_CONTEXT(ctx);
> +}
> +
> +static struct tst_test test = {
> +	.test_all = run,
> +	.setup = setup,
> +	.cleanup = cleanup,
> +	.taint_check = TST_TAINT_W | TST_TAINT_D,
> +	.needs_kconfigs = (const char *[]) {
> +		"CONFIG_USER_NS=y",
> +		"CONFIG_NF_TABLES",
> +		NULL
> +	},
> +	.save_restore = (const struct tst_path_val[]) {
> +		{"/proc/sys/user/max_user_namespaces", "1024", TST_SR_SKIP},
Out of curiosity, why this?

CVE mentions "Exploiting it requires CAP_NET_ADMIN in any user or network
namespace.", but how is it related to changing max_user_namespaces value?

Also, vulnerable kernel reproducers with any max_user_namespaces value, or
without setting max_user_namespaces at all.

I can fix all the typos (only) before merge or you send v4 (whatever you prefer).

Kind regards,
Petr

> +		{}
> +	},
> +	.tags = (const struct tst_tag[]) {
> +		{"linux-git", "515ad530795c"},
> +		{"CVE", "2023-31248"},
> +		{}
> +	}
> +};

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v3 4/4] Add test for CVE 2023-31248

[Martin Doucha]

Hi,
I'll send a v4 shortly, just a few comments below.

On 21. 11. 23 14:19, Petr Vorel wrote:
>> +	/* Add destination chain */
>> +	header.nlmsg_type = (NFNL_SUBSYS_NFTABLES << 8) | NFT_MSG_NEWCHAIN;
>> +	header.nlmsg_flags = NLM_F_REQUEST | NLM_F_CREATE;
>> +	nfpayload.nfgen_family = NFPROTO_IPV4;
>> +	nfpayload.res_id = htons(0);
>> +	NETLINK_ADD_MESSAGE(ctx, &header, &nfpayload, sizeof(nfpayload));
>> +	NETLINK_ADD_ATTR_LIST(ctx, destchain_config);
>> +
>> +	/* Delete destination chain */
>> +	header.nlmsg_type = (NFNL_SUBSYS_NFTABLES << 8) | NFT_MSG_DELCHAIN;
>> +	header.nlmsg_flags = NLM_F_REQUEST;
>> +	nfpayload.nfgen_family = NFPROTO_IPV4;
>> +	nfpayload.res_id = htons(0);
>> +	NETLINK_ADD_MESSAGE(ctx, &header, &nfpayload, sizeof(nfpayload));
>> +	NETLINK_ADD_ATTR_LIST(ctx, delchain_config);
>> +
>> +	/* Add destination chain */
> nit: this looks to be source chain
> Out of curriosity I'm looking at the reproducer
> (https://bugzilla.suse.com/attachment.cgi?id=868806)
> and it needs just single chain.
> But test needs both for some reason.

Yes, the comment is a copy-paste mistake, I'll fix that in v4. In the 
original reproducer, the rule being added to a deleted chain is silently 
ignored on fixed kernels and the test does not get any errors that could 
be used to determine the presence of vulnerability. I've rewritten the 
scenario originally submitted by Souta Kawahara which adds a rule to an 
existing chain which references the deleted chain though a GOTO 
operation. That'll trigger ENOENT error on fixed kernels.

>> +static struct tst_test test = {
>> +	.test_all = run,
>> +	.setup = setup,
>> +	.cleanup = cleanup,
>> +	.taint_check = TST_TAINT_W | TST_TAINT_D,
>> +	.needs_kconfigs = (const char *[]) {
>> +		"CONFIG_USER_NS=y",
>> +		"CONFIG_NF_TABLES",
>> +		NULL
>> +	},
>> +	.save_restore = (const struct tst_path_val[]) {
>> +		{"/proc/sys/user/max_user_namespaces", "1024", TST_SR_SKIP},
> Out of curiosity, why this?
> 
> CVE mentions "Exploiting it requires CAP_NET_ADMIN in any user or network
> namespace.", but how is it related to changing max_user_namespaces value?
> 
> Also, vulnerable kernel reproducers with any max_user_namespaces value, or
> without setting max_user_namespaces at all.

There were some issues with older distros which set max_user_namespaces 
to zero by default. It's just standard boilerplate for safely calling 
tst_setup_netns().

-- 
Martin Doucha   mdoucha@suse.cz
SW Quality Engineer
SUSE LINUX, s.r.o.
CORSO IIa
Krizikova 148/34
186 00 Prague 8
Czech Republic


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp