From: Alan T. DeKok <aland@freeradius.org>
Date: Sun, 17 Apr 2022 23:16:45 +0000 (-0400)
Subject: make fr_pair_raw_from_network() closer to other decode functions
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=3046888e22251f00e7728afa5f627598c3223ab3;p=thirdparty%2Ffreeradius-server.git

make fr_pair_raw_from_network() closer to other decode functions
---

diff --git a/src/lib/util/decode.c b/src/lib/util/decode.c
index 02128f734c1..a6e7fba0ff5 100644
--- a/src/lib/util/decode.c
+++ b/src/lib/util/decode.c
@@ -61,37 +61,46 @@ ssize_t fr_pair_array_from_network(TALLOC_CTX *ctx, fr_pair_list_t *out, fr_dict
 /** Create a "raw" pair from the network data
  *
  * @param[in] ctx context	to alloc new attributes in.
+ * @param[out] out		Where to write the decoded #fr_pair_t
  * @param[in] parent		dictionary entry
  * @param[in] data		to parse.
  * @param[in] data_len		of data to parse.
  */
-fr_pair_t *fr_pair_raw_from_network(TALLOC_CTX *ctx, fr_dict_attr_t const *parent, uint8_t const *data, size_t data_len)
+ssize_t fr_pair_raw_from_network(TALLOC_CTX *ctx, fr_pair_list_t *out, fr_dict_attr_t const *parent, uint8_t const *data, size_t data_len)
 {
+	ssize_t slen;
 	fr_pair_t *vp;
 	fr_dict_attr_t *unknown;
 	fr_dict_attr_t const *child;
 
 #if defined(__clang_analyzer__) || !defined(NDEBUG)
-	if (!parent->parent) return NULL; /* stupid static analyzers */
+	if (!parent->parent) return -1; /* stupid static analyzers */
 #endif
 
 	/*
 	 *	Build an unknown attr of the entire data.
 	 */
 	unknown = fr_dict_unknown_attr_afrom_da(ctx, parent);
-	if (!unknown) return NULL;
+	if (!unknown) return -1;
 	unknown->flags.is_raw = 1;
 
-	vp = fr_pair_afrom_da(ctx, unknown); /* makes a copy of 'child' */
+	vp = fr_pair_afrom_da(ctx, unknown); /* makes a copy of 'unknown' */
 	child = unknown;
-	fr_dict_unknown_free(&child);
-	if (!vp) return NULL;
+	fr_dict_unknown_free(&child); /* const issues */
+	if (!vp) return -1;
 
-	if (fr_value_box_from_network(vp, &vp->data, vp->da->type, vp->da,
-				      &FR_DBUFF_TMP(data, data_len), data_len, true) < 0) {
+	slen = fr_value_box_from_network(vp, &vp->data, vp->da->type, vp->da,
+					 &FR_DBUFF_TMP(data, data_len), data_len, true);
+	if (slen < 0) {
 		talloc_free(vp);
-		return NULL;
+		return slen;
 	}
 
-	return vp;
+	/*
+	 *	Raw VPs are always tainted.
+	 */
+	vp->vp_tainted = true;
+	fr_pair_append(out, vp);
+
+	return data_len;
 }
diff --git a/src/lib/util/decode.h b/src/lib/util/decode.h
index 5943574505e..f3ac4205fda 100644
--- a/src/lib/util/decode.h
+++ b/src/lib/util/decode.h
@@ -29,23 +29,30 @@ extern "C" {
 
 #include <freeradius-devel/util/pair.h>
 
-/** Typedefs for simplifying the use and declaration of protocol decoders.
+/** Decode a value from the network into an output #fr_pair_list_t
  *
+ * @param[in] ctx		context to alloc new attributes in.
+ * @param[out] out		Where to write the decoded options.
+ * @param[in] parent		of sub TLVs.
+ * @param[in] data		to parse.
+ * @param[in] data_len		of the data to parse
+ * @return
+ *	<= 0 on error
+ *	bytes successfully decoded on success (<= data_len)
  */
 typedef ssize_t (*fr_pair_decode_value_t)(TALLOC_CTX *ctx, fr_pair_list_t *out,
 					   fr_dict_attr_t const *parent,
 					   uint8_t const *data, size_t const data_len, void *decode_ctx);
 
-#define PROTO_DECODE_FUNC(_name) static ssize_t _name(TALLOC_CTX *ctx, fr_pair_list_t *out, \
+#define PROTO_DECODE_FUNC(_name) static ssize_t decode_ ## _name(TALLOC_CTX *ctx, fr_pair_list_t *out, \
 					   fr_dict_attr_t const *parent, \
-					   uint8_t const *data, size_t const data_len, fr_proto_decode_ctx_t *decode_ctx); \
+					   uint8_t const *data, size_t const data_len, void *decode_ctx)
 
 ssize_t fr_pair_array_from_network(TALLOC_CTX *ctx, fr_pair_list_t *out, fr_dict_attr_t const *parent,
 				   uint8_t const *data, size_t data_len, void *decode_ctx, fr_pair_decode_value_t decode_value) CC_HINT(nonnull(1,2,3,4,6));
 
-fr_pair_t *fr_pair_raw_from_network(TALLOC_CTX *ctx, fr_dict_attr_t const *parent,
-				    uint8_t const *data, size_t data_len) CC_HINT(nonnull);
-
+ssize_t fr_pair_raw_from_network(TALLOC_CTX *ctx, fr_pair_list_t *out, fr_dict_attr_t const *parent,
+				 uint8_t const *data, size_t data_len) CC_HINT(nonnull);
 
 #ifdef __cplusplus
 }
diff --git a/src/lib/util/struct.c b/src/lib/util/struct.c
index db0bab05ed7..f217a598e3a 100644
--- a/src/lib/util/struct.c
+++ b/src/lib/util/struct.c
@@ -41,6 +41,7 @@ ssize_t fr_struct_from_network(TALLOC_CTX *ctx, fr_pair_list_t *out,
 	fr_pair_t		*vp, *key_vp, *struct_vp = NULL;
 	unsigned int		offset = 0;
 	TALLOC_CTX		*child_ctx;
+	ssize_t			slen;
 
 	if (data_len == 0) {
 		fr_strerror_const("struct decoder was passed zero bytes of data");
@@ -61,7 +62,6 @@ ssize_t fr_struct_from_network(TALLOC_CTX *ctx, fr_pair_list_t *out,
 
 		struct_vp = fr_pair_afrom_da(ctx, parent);
 		if (!struct_vp) {
-		oom:
 			fr_strerror_const("out of memory");
 			return -1;
 		}
@@ -190,8 +190,6 @@ ssize_t fr_struct_from_network(TALLOC_CTX *ctx, fr_pair_list_t *out,
 		 *	Decode child TLVs, according to the parent attribute.
 		 */
 		if (child->type == FR_TYPE_TLV) {
-			ssize_t slen;
-
 			fr_assert(!key_vp);
 
 			if (!decode_tlv) {
@@ -238,8 +236,6 @@ ssize_t fr_struct_from_network(TALLOC_CTX *ctx, fr_pair_list_t *out,
 		 *	the callback should deal with this.
 		 */
 		if (decode_value) {
-			ssize_t slen;
-
 			if (child->flags.array) {
 				slen = fr_pair_array_from_network(child_ctx, child_list, child, p, child_length, decode_ctx, decode_value);
 			} else {
@@ -297,13 +293,8 @@ ssize_t fr_struct_from_network(TALLOC_CTX *ctx, fr_pair_list_t *out,
 				fr_pair_list_free(child_list);
 			}
 
-			vp = fr_pair_raw_from_network(ctx, parent, data, data_len);
-			if (!vp) goto oom;
-
-			/*
-			 *	And append this one VP to the output cursor.
-			 */
-			fr_pair_append(out, vp);
+			slen = fr_pair_raw_from_network(ctx, out, parent, data, data_len);
+			if (slen < 0) return slen;
 			return data_len;
 		}
 
@@ -326,7 +317,6 @@ ssize_t fr_struct_from_network(TALLOC_CTX *ctx, fr_pair_list_t *out,
 	 *	decode it.
 	 */
 	if (key_vp) {
-		ssize_t slen;
 		fr_dict_enum_value_t const *enumv;
 		child = NULL;
 
@@ -358,14 +348,13 @@ ssize_t fr_struct_from_network(TALLOC_CTX *ctx, fr_pair_list_t *out,
 				goto unknown;
 			}
 
-			vp = fr_pair_raw_from_network(child_ctx, child, p, end - p);
-			if (!vp) {
+			slen = fr_pair_raw_from_network(child_ctx, child_list, child, p, end - p);
+			if (slen < 0) {
 				FR_PROTO_TRACE("Failed creating raw VP from malformed or unknown substruct");
 				fr_dict_unknown_free(&child);
-				goto oom;
+				return slen;
 			}
 
-			fr_pair_append(child_list, vp);
 			p = end;
 		} else {
 			fr_assert(child->type == FR_TYPE_STRUCT);
diff --git a/src/protocols/dhcpv4/decode.c b/src/protocols/dhcpv4/decode.c
index 131d7eefce3..a5f9578ec7e 100644
--- a/src/protocols/dhcpv4/decode.c
+++ b/src/protocols/dhcpv4/decode.c
@@ -378,12 +378,7 @@ static ssize_t decode_tlv(TALLOC_CTX *ctx, fr_pair_list_t *out, fr_dict_attr_t c
 	 *	the data as "raw" in the parents context/
 	 */
 	if (data_len < 3) {
-		fr_pair_t *vp;
-
-		vp = fr_pair_raw_from_network(ctx, parent, data, data_len);
-		if (!vp) return -1;
-		fr_pair_append(out, vp);
-		return data_len;
+		return fr_pair_raw_from_network(ctx, out, parent, data, data_len);
 	}
 
 	FR_PROTO_TRACE("%s called to parse %zu byte(s)", __FUNCTION__, data_len);
diff --git a/src/protocols/radius/decode.c b/src/protocols/radius/decode.c
index 3246cf3383c..31efd8cd61a 100644
--- a/src/protocols/radius/decode.c
+++ b/src/protocols/radius/decode.c
@@ -1652,11 +1652,7 @@ ssize_t fr_radius_decode_pair_value(TALLOC_CTX *ctx, fr_pair_list_t *out,
 	raw:
 		if (vp) talloc_free(vp);
 
-		vp = fr_pair_raw_from_network(ctx, parent, data, attr_len);
-		if (!vp) return -1;
-
-		tag = 0;
-		goto done;
+		return fr_pair_raw_from_network(ctx, out, parent, data, attr_len);
 	}
 
 	/*
@@ -1779,7 +1775,6 @@ ssize_t fr_radius_decode_pair_value(TALLOC_CTX *ctx, fr_pair_list_t *out,
 		break;
 	}
 
-done:
 	vp->vp_tainted = true;
 
 	if (!tag) {