diff mbox series

[iproute2] ip route: save: exclude rtnh_flags which can't be set

Message ID 20211126134311.920808-1-alexander.mikhalitsyn@virtuozzo.com (mailing list archive)
State New, archived
Delegated to: David Ahern
Headers show
Series [iproute2] ip route: save: exclude rtnh_flags which can't be set | expand

Checks

Context Check Description
netdev/tree_selection success Not a local patch

Commit Message

Alexander Mikhalitsyn Nov. 26, 2021, 1:43 p.m. UTC
During "ip route save" we preserve all rtnh_flags,
even those that can't be set directly by the userspace.
This looks like a bug because a user can't restore
route dump which was generated by "ip route save" back.
This also prevents CRIU from correct restore of the
containers with some route configurations inside.

Reproducer:
$ ip link add type veth
$ ip addr add 10.0.0.1/24 dev veth0
$ ip link set veth0 up
$ ip route add default via 10.0.0.1
$ ip route save > route_dump
$ ip route restore < route_dump
Error: Invalid rtm_flags - can not contain DEAD or LINKDOWN.

Let's just omit non-settable rtnh_flags from the dump image.

According to the check in the fib_create_info() kernel
function it looks like we can't restore back only
RTNH_F_DEAD and RTNH_F_LINKDOWN flags, so RTNH_REJECT_MASK
contains this flags for now.

See also linux kernel patch:
[PATCH net-next] rtnetlink: add RTNH_REJECT_MASK

Cc: David Miller <davem@davemloft.net>
Cc: David Ahern <dsahern@gmail.com>
Cc: Stephen Hemminger <stephen@networkplumber.org>
Cc: Ido Schimmel <idosch@nvidia.com>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Roopa Prabhu <roopa@nvidia.com>
Cc: Andrei Vagin <avagin@gmail.com>
Cc: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
Cc: Alexander Mikhalitsyn <alexander@mihalicyn.com>
Signed-off-by: Alexander Mikhalitsyn <alexander.mikhalitsyn@virtuozzo.com>
---
 include/uapi/linux/rtnetlink.h | 3 +++
 ip/iproute.c                   | 6 ++++++
 2 files changed, 9 insertions(+)

Comments

Ido Schimmel Nov. 28, 2021, 1:09 p.m. UTC | #1
On Fri, Nov 26, 2021 at 04:43:10PM +0300, Alexander Mikhalitsyn wrote:
> +	/*
> +	 * Exclude flags which can't be set directly
> +	 * by the userspace from the rtmsg dump.
> +	 */

I believe iproute2 is following netdev style comments [1] so this should
be:

/* Exclude flags which can't be set directly
 * by the userspace from the rtmsg dump.
 */

At least it's consistent with other comments in the file.

[1] https://www.kernel.org/doc/html/v5.12/networking/netdev-FAQ.html#is-the-comment-style-convention-different-for-the-networking-content

> +	r->rtm_flags &= ~RTNH_REJECT_MASK;
> +
>  	ret = write(STDOUT_FILENO, n, n->nlmsg_len);
>  	if ((ret > 0) && (ret != n->nlmsg_len)) {
>  		fprintf(stderr, "Short write while saving nlmsg\n");
> -- 
> 2.31.1
>
diff mbox series

Patch

diff --git a/include/uapi/linux/rtnetlink.h b/include/uapi/linux/rtnetlink.h
index e01efa28..6de83c62 100644
--- a/include/uapi/linux/rtnetlink.h
+++ b/include/uapi/linux/rtnetlink.h
@@ -417,6 +417,9 @@  struct rtnexthop {
 #define RTNH_COMPARE_MASK	(RTNH_F_DEAD | RTNH_F_LINKDOWN | \
 				 RTNH_F_OFFLOAD | RTNH_F_TRAP)
 
+/* these flags can't be set by the userspace */
+#define RTNH_REJECT_MASK	(RTNH_F_DEAD | RTNH_F_LINKDOWN)
+
 /* Macros to handle hexthops */
 
 #define RTNH_ALIGNTO	4
diff --git a/ip/iproute.c b/ip/iproute.c
index 1447a5f7..a72b652b 100644
--- a/ip/iproute.c
+++ b/ip/iproute.c
@@ -1632,6 +1632,12 @@  static int save_route(struct nlmsghdr *n, void *arg)
 	if (!filter_nlmsg(n, tb, host_len))
 		return 0;
 
+	/*
+	 * Exclude flags which can't be set directly
+	 * by the userspace from the rtmsg dump.
+	 */
+	r->rtm_flags &= ~RTNH_REJECT_MASK;
+
 	ret = write(STDOUT_FILENO, n, n->nlmsg_len);
 	if ((ret > 0) && (ret != n->nlmsg_len)) {
 		fprintf(stderr, "Short write while saving nlmsg\n");