Merge branch 'fix-spa-choice-get-vals' into 'master'

Fix handling of unusual choice POD

See merge request pipewire/pipewire!2485

doc/dox/api/spa-pod.dox

@@ -435,10 +435,14 @@ spa_pod_parser_get_object(&p,
 \endcode
 
 `spa_pod_get_values()` is a useful function. It returns a
-`struct spa_pod*` with and array of values. For normal POD's
-and choice none values, it simply returns the POD and one value.
-For other choice values it returns the choice type and an array
-of values:
+`struct spa_pod*` with and array of values. For invalid PODs
+it returns the POD and no values. For normal PODs it returns
+the POD and one value. For choice values it returns the choice
+type and an array of values. If the choice doesn't fit even a
+single value, the array will have no values. If the choice is
+of the `SPA_CHOICE_None` or the length of each element is
+misaligned, the array will have one value. Otherwise,
+the array will have the same length as the choice.
 
 \code{.c}
 struct spa_pod *value;

pipewire-v4l2/src/pipewire-v4l2.c

@@ -2255,8 +2255,8 @@ static int vidioc_queryctrl(struct file *file, struct v4l2_queryctrl *arg)
 
 			// check type and populate range
 			pod = spa_pod_get_values(type, &n_vals, &choice);
-			if (spa_pod_is_int(pod)) {
-				if (n_vals < 4)
+			if (pod->type == SPA_TYPE_Int) {
+				if (pod->size != sizeof(int) || n_vals < 4)
 					break;
 				arg->type = V4L2_CTRL_TYPE_INTEGER;
 				int *v = SPA_POD_BODY(pod);
@@ -2330,7 +2330,7 @@ static int vidioc_g_ctrl(struct file *file, struct v4l2_control *arg)
 		if (ctrl_id == arg->id) {
 			// TODO: support getting true ctrl values instead of defaults
 			pod = spa_pod_get_values(type, &n_vals, &choice);
-			if (spa_pod_is_int(pod)) {
+			if (pod->type == SPA_TYPE_Int) {
 				if (n_vals < 4)
 					break;
 				int *v = SPA_POD_BODY(pod);

spa/include/spa/debug/format.h

@@ -164,8 +164,7 @@ SPA_API_DEBUG_FORMAT int spa_debugc_format(struct spa_debug_context *ctx, int in
 		type = val->type;
 		size = val->size;
 
-		if (type < SPA_TYPE_None || type >= _SPA_TYPE_LAST || n_vals < 1 ||
-		    size < spa_pod_type_size(type))
+		if (type < SPA_TYPE_None || type >= _SPA_TYPE_LAST || n_vals < 1)
 			continue;
 
 		vals = SPA_POD_BODY(val);

spa/include/spa/pod/body.h

@@ -78,6 +78,65 @@ SPA_API_POD_BODY uint32_t spa_pod_type_size(uint32_t type)
 	return 0;
 }
 
+SPA_API_POD_BODY uint32_t spa_pod_type_align(uint32_t type)
+{
+	switch (type) {
+	case SPA_TYPE_None:
+	case SPA_TYPE_Bytes:
+	case SPA_TYPE_String:
+	case SPA_TYPE_Bitmap:
+		return 1;
+	case SPA_TYPE_Bool:
+	case SPA_TYPE_Int:
+		return SPA_ALIGNOF(int32_t);
+	case SPA_TYPE_Id:
+		return SPA_ALIGNOF(uint32_t);
+	case SPA_TYPE_Long:
+		return SPA_ALIGNOF(int64_t);
+	case SPA_TYPE_Float:
+		return SPA_ALIGNOF(float);
+	case SPA_TYPE_Double:
+		return SPA_ALIGNOF(double);
+	case SPA_TYPE_Rectangle:
+		return SPA_ALIGNOF(struct spa_rectangle);
+	case SPA_TYPE_Fraction:
+		return SPA_ALIGNOF(struct spa_fraction);
+	case SPA_TYPE_Pointer:
+		return SPA_ALIGNOF(struct spa_pod_pointer_body);
+	case SPA_TYPE_Fd:
+		return SPA_ALIGNOF(int64_t);
+	case SPA_TYPE_Sequence:
+	case SPA_TYPE_Choice:
+	case SPA_TYPE_Struct:
+	case SPA_TYPE_Pod:
+	case SPA_TYPE_Object:
+	case SPA_TYPE_Array:
+	default:
+		return SPA_POD_ALIGN;
+	}
+}
+
+SPA_API_POD_BODY uint32_t spa_pod_choice_min_values(uint32_t choice_type)
+{
+	switch (choice_type) {
+	case SPA_CHOICE_Enum:
+		return 2;
+	case SPA_CHOICE_Range:
+		return 3;
+	case SPA_CHOICE_Step:
+		return 4;
+	case SPA_CHOICE_None:
+	case SPA_CHOICE_Flags:
+	default:
+		/*
+		 * This must always return at least 1, because callers
+		 * assume that n_vals >= spa_pod_choice_min_values()
+		 * mean that n_vals is at least 1.
+		 */
+		return 1;
+	}
+}
+
 SPA_API_POD_BODY int spa_pod_body_from_data(void *data, size_t maxsize, off_t offset, size_t size,
 		struct spa_pod *pod, const void **body)
 {
@@ -326,13 +385,22 @@ SPA_API_POD_BODY int spa_pod_body_get_array(const struct spa_pod *pod, const voi
 	*arr_body = SPA_PTROFF(body, sizeof(struct spa_pod_array_body), void);
 	return 0;
 }
-SPA_API_POD_BODY const void *spa_pod_array_body_get_values(const struct spa_pod_array *arr,
+SPA_API_POD_BODY const void *spa_pod_array_body_get_values(const struct spa_pod_array *pod,
 		const void *body, uint32_t *n_values, uint32_t *val_size, uint32_t *val_type)
 {
-	uint32_t child_size = arr->body.child.size;
-	*n_values = child_size ? (arr->pod.size - sizeof(arr->body)) / child_size : 0;
-	*val_size = child_size;
-	*val_type = arr->body.child.type;
+	uint32_t child_size = *val_size = pod->body.child.size;
+	uint32_t child_type = *val_type = pod->body.child.type;
+	uint32_t child_type_size = spa_pod_type_size(child_size);
+	uint32_t max_body_size = pod->pod.size - sizeof(pod->body);
+
+	if (SPA_UNLIKELY(child_size < child_type_size) ||
+			child_size == 0) {
+		*n_values = 0;
+	} else if (SPA_UNLIKELY(child_size != child_type_size) &&
+	           SPA_UNLIKELY((child_size & (spa_pod_type_align(child_type) - 1)) != 0))
+		*n_values = max_body_size >= child_size ? 1u : 0u;
+	else
+		*n_values = max_body_size / child_size;
 	return body;
 }
 
@@ -366,13 +434,23 @@ SPA_API_POD_BODY const void *spa_pod_choice_body_get_values(const struct spa_pod
 		const void *body, uint32_t *n_values, uint32_t *choice,
 		uint32_t *val_size, uint32_t *val_type)
 {
-	uint32_t child_size = pod->body.child.size;
-	*val_size = child_size;
-	*val_type = pod->body.child.type;
-	*n_values = child_size ? (pod->pod.size - sizeof(pod->body)) / child_size : 0;
-	*choice = pod->body.type;
-	if (*choice == SPA_CHOICE_None)
-		*n_values = SPA_MIN(1u, *n_values);
+	/* precondition check */
+	spa_assert_se(pod->pod.size >= sizeof(pod->body));
+	uint32_t child_size = *val_size = pod->body.child.size;
+	uint32_t child_type = *val_type = pod->body.child.type;
+	uint32_t child_type_size = spa_pod_type_size(child_size);
+	uint32_t choice_type = *choice = pod->body.type;
+	uint32_t max_body_size = pod->pod.size - sizeof(pod->body);
+
+	if (SPA_UNLIKELY(child_size < child_type_size) ||
+			child_size == 0) {
+		*n_values = 0;
+	} else if (choice_type == SPA_CHOICE_None ||
+	         (SPA_UNLIKELY(child_size != child_type_size) &&
+	          SPA_UNLIKELY((child_size & (spa_pod_type_align(child_type) - 1)) != 0)))
+		*n_values = max_body_size >= child_size ? 1 : 0;
+	else
+		*n_values = max_body_size / child_size;
 	return body;
 }
 

spa/include/spa/pod/compare.h

@@ -226,6 +226,8 @@ SPA_API_POD_COMPARE int spa_pod_compare_is_in_range(uint32_t type, const void *v
 SPA_API_POD_COMPARE int spa_pod_compare_is_valid_choice(uint32_t type, uint32_t size,
 		const void *val, const void *vals, uint32_t n_vals, uint32_t choice)
 {
+	if (n_vals < spa_pod_choice_min_values(choice))
+		return 0;
 	switch (choice) {
 	case SPA_CHOICE_None:
 		if (spa_pod_compare_value(type, val, vals, size) == 0)

spa/include/spa/pod/filter.h

@@ -82,8 +82,9 @@ spa_pod_filter_prop(struct spa_pod_builder *b,
 	v1 = spa_pod_get_values(&p1->value, &nalt1, &p1c);
 	v2 = spa_pod_get_values(&p2->value, &nalt2, &p2c);
 
-	/* empty choices */
-	if (nalt1 < 1 || nalt2 < 1)
+	/* empty or bogus choices */
+	if (nalt1 < spa_pod_choice_min_values(p1c) ||
+	    nalt2 < spa_pod_choice_min_values(p2c))
 		return -EINVAL;
 
 	alt1 = SPA_POD_BODY(v1);
@@ -95,8 +96,6 @@ spa_pod_filter_prop(struct spa_pod_builder *b,
 	/* incompatible property types */
 	if (type != v2->type || size != v2->size || p1->key != p2->key)
 		return -EINVAL;
-	if (size < spa_pod_type_size(type))
-		return -EINVAL;
 
 	/* start with copying the property */
 	spa_pod_builder_prop(b, p1->key, p1->flags & p2->flags);
@@ -406,7 +405,7 @@ SPA_API_POD_FILTER int spa_pod_filter_object_make(struct spa_pod_object *pod)
 			struct spa_pod *v = spa_pod_get_values(&res->value, &nvals, &choice);
 			const void *vals = SPA_POD_BODY(v);
 
-			if (v->size < spa_pod_type_size(v->type))
+			if (nvals < spa_pod_choice_min_values(choice))
 				continue;
 
 			if (spa_pod_compare_is_valid_choice(v->type, v->size,

spa/include/spa/pod/iter.h

@@ -228,7 +228,7 @@ SPA_API_POD_ITER struct spa_pod *spa_pod_get_values(const struct spa_pod *pod,
 		spa_pod_choice_body_get_values(p, SPA_POD_BODY_CONST(p), n_vals, choice, &size, &type);
 		return (struct spa_pod*)&p->body.child;
 	} else {
-		*n_vals = 1;
+		*n_vals = (pod->size >= spa_pod_type_size(pod->type) ? 1u : 0u);
 		*choice = SPA_CHOICE_None;
 		return (struct spa_pod*)pod;
 	}

spa/plugins/v4l2/v4l2-utils.c

@@ -399,7 +399,7 @@ enum_filter_format(uint32_t media_type, int32_t media_subtype,
 				if (index == 0)
 					video_format = values[0];
 			} else {
-				if (index < n_values - 1)
+				if (index < n_values - 1 && val->size == sizeof(values[0]))
 					video_format = values[index + 1];
 			}
 		} else {

spa/plugins/videoconvert/videoconvert-ffmpeg.c

@@ -1209,10 +1209,7 @@ static struct spa_pod *transform_format(struct impl *this, struct port *port, co
 			uint32_t n_vals, choice, *id_vals;
 			struct spa_pod *val = spa_pod_get_values(&prop->value, &n_vals, &choice);
 
-			if (n_vals < 1)
-				return 0;
-
-			if (!spa_pod_is_id(val))
+			if (n_vals < 1 || val->type != SPA_TYPE_Id)
 				return 0;
 
 			id_vals = SPA_POD_BODY(val);

src/gst/gstpipewireformat.c

@@ -1117,7 +1117,7 @@ handle_int_prop (const struct spa_pod_prop *prop, const char *key, GstCaps *res)
     case SPA_CHOICE_Range:
     case SPA_CHOICE_Step:
     {
-      if (n_items < 3)
+      if (n_items < 3 || val->size != sizeof(ints[0]))
         return;
       gst_caps_set_simple (res, key, GST_TYPE_INT_RANGE, ints[1], ints[2], NULL);
       break;
@@ -1225,7 +1225,7 @@ handle_fraction_prop (const struct spa_pod_prop *prop, const char *key, GstCaps
     case SPA_CHOICE_Range:
     case SPA_CHOICE_Step:
     {
-      if (n_items < 3)
+      if (n_items < 3 || val->size != sizeof(fract[0]))
         return;
 
       if (fract[1].num == fract[2].num &&

src/modules/module-protocol-pulse/format.c

@@ -683,7 +683,7 @@ static int add_int(struct format_info *info, const char *k, struct spa_pod *para
 		return -ENOENT;
 
 	val = spa_pod_get_values(&prop->value, &n_values, &choice);
-	if (!spa_pod_is_int(val))
+	if (val->type != SPA_TYPE_Int || val->size != sizeof(values[0]))
 		return -ENOTSUP;
 
 	if (n_values == 0)
@@ -745,7 +745,7 @@ static int format_info_iec958_from_param(struct format_info *info, struct spa_po
 		return -ENOENT;
 
 	val = spa_pod_get_values(&prop->value, &n_values, &choice);
-	if (val->type != SPA_TYPE_Id)
+	if (val->type != SPA_TYPE_Id || val->size != sizeof(values[0]))
 		return -ENOTSUP;
 
 	if (index >= n_values)

src/pipewire/stream.c

@@ -1270,6 +1270,8 @@ static int node_event_param(void *object, int seq,
 			return 0;
 
 		c = calloc(1, sizeof(*c) + SPA_POD_SIZE(param));
+		if (c == NULL)
+			return -ENOMEM;
 		c->info = SPA_PTROFF(c, sizeof(*c), struct spa_pod);
 		memcpy(c->info, param, SPA_POD_SIZE(param));
 		c->control.n_values = 0;
@@ -1290,6 +1292,10 @@ static int node_event_param(void *object, int seq,
 			free(c);
 			return -EINVAL;
 		}
+		if (n_vals > 1 && pod->size != spa_pod_type_size(pod->type)) {
+			free(c);
+			return -ENOTSUP;
+		}
 
 		c->type = pod->type;
 		if (spa_pod_is_float(pod))