route:tc: fix rtnl_tc_clone() calling to_clone() and add comment

rtnl_tc_data() allocates a data if necessary (or ENOMEM). So the code

    void *a = rtnl_tc_data(dst), *b = rtnl_tc_data(src);

    if (!a)
             return 0;
    else if (!b)
             return -NLE_NOMEM;

is not right. It also means, rtnl_tc_clone() will always add the data
pointers if the previously doesn't exist. In particular, it means
that clone modifies "src", and that "dst" will always have an allocated
pointer afterwards. Fix that.

Also, add a comment about the subtleties for how to_clone() must fix the
aliased pointers. Otherwise, we will crash. All implementations actually
got this wrong earlier, and it only worked if they didn't encounter
ENOMEM.

1 files changed, 8 insertions, 10 deletions

diff --git a/lib/route/tc.c b/lib/route/tc.c
index b47172f..3d993b9 100644
--- a/lib/route/tc.c
+++ b/lib/route/tc.c
@@ -837,18 +837,16 @@ int rtnl_tc_clone(struct nl_object *dstobj, struct nl_object *srcobj)
 		if (!(dst->tc_subdata = nl_data_clone(src->tc_subdata))) {
 			return -NLE_NOMEM;
 		}
-	}
-
-	ops = rtnl_tc_get_ops(src);
-	if (ops && ops->to_clone) {
-		void *a = rtnl_tc_data(dst), *b = rtnl_tc_data(src);
 
-		if (!a)
-			return 0;
-		else if (!b)
-			return -NLE_NOMEM;
+		/* Warning: if the data contains pointer, then at this point, dst->tc_subdata
+		 * will alias those pointers.
+		 *
+		 * ops->to_clone() MUST fix that. */
 
-		return ops->to_clone(a, b);
+		ops = rtnl_tc_get_ops(src);
+		if (ops && ops->to_clone) {
+			return ops->to_clone(rtnl_tc_data(dst), rtnl_tc_data(src));
+		}
 	}
 
 	return 0;