Bug#862295: unblock: v4l-utils/1.12.5-1

[Niels Thykier <niels@thykier.net>]

Control: tags -1 moreinfo

Gregor Jasny:
> Package: release.debian.org
> Severity: normal
> User: release.debian.org@packages.debian.org
> Usertags: unblock
> 
> Please unblock package v4l-utils
> 
> Hello,
> 
> please unblock the latest stable version of v4l-utils 1.12. It fixes
> a crash in the Digital Video Broadcasting (DVB) library that occurs
> when a German DVB-T2 transport stream is parsed. Additionally it fixes
> a bug where identical transport streams were not detected as such.
> 
> Thanks,
> Gregor
> 
> [...]
> diff -Nru v4l-utils-1.12.3/lib/include/libdvbv5/desc_t2_delivery.h v4l-utils-1.12.5/lib/include/libdvbv5/desc_t2_delivery.h
> --- v4l-utils-1.12.3/lib/include/libdvbv5/desc_t2_delivery.h	2017-01-22 18:33:34.000000000 +0100
> +++ v4l-utils-1.12.5/lib/include/libdvbv5/desc_t2_delivery.h	2017-05-10 20:21:39.000000000 +0200
> @@ -45,10 +45,14 @@
>   *
>   * @param cell_id_extension	cell id extension
>   * @param transposer_frequency	transposer frequency
> + *
> + * NOTE: This struct is deprecated and will never be filled. All
> + * subcell transposer frequencies will be added to
> + * dvb_desc_t2_delivery::centre_frequency array.
>   */
>  struct dvb_desc_t2_delivery_subcell {
>  	uint8_t cell_id_extension;
> -	uint16_t transposer_frequency;
> +	uint16_t transposer_frequency;		// Should be 32 bits, instead
>  } __attribute__((packed));
>  
>  /**
> @@ -65,7 +69,8 @@
>   * @param other_frequency_flag	other frequency flag
>   * @param tfs_flag		tfs flag
>   *
> - * @param centre_frequency	centre frequency vector
> + * @param centre_frequency	centre frequency vector, for all cell and
> + * 				subcel ID's
>   * @param frequency_loop_length	size of the dvb_desc_t2_delivery::centre_frequency
>   *				vector
>   *
> @@ -86,13 +91,15 @@
>  			uint16_t transmission_mode:3;
>  			uint16_t guard_interval:3;
>  			uint16_t reserved:2;
> -			uint16_t bandwidth:3;
> +			uint16_t bandwidth:4;
>  			uint16_t SISO_MISO:2;
>  		} __attribute__((packed));

Doesn't this change break the ABI?

> [...]
> diff -Nru v4l-utils-1.12.3/lib/libdvbv5/descriptors/desc_t2_delivery.c v4l-utils-1.12.5/lib/libdvbv5/descriptors/desc_t2_delivery.c
> --- v4l-utils-1.12.3/lib/libdvbv5/descriptors/desc_t2_delivery.c	2017-01-22 18:33:34.000000000 +0100
> +++ v4l-utils-1.12.5/lib/libdvbv5/descriptors/desc_t2_delivery.c	2017-05-10 20:21:39.000000000 +0200
> [...]
> @@ -40,53 +40,76 @@
> [...]
> +		d->frequency_loop_length += n;
> +		d->centre_frequency = realloc(d->centre_frequency,
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Smells very much like the common "realloc memory leak on error" bug
(Usually cppcheck can spot this kind of bug).  Not sure how long lived
that leak is, but fixing it might be faster than figuring it out.

> +					      d->frequency_loop_length * sizeof(*d->centre_frequency));
> +		if (!d->centre_frequency) {
> +			dvb_logerr("%s: out of memory", __func__);
> +			return -3;
> +		}
> [...]
> +			// Add transposer_frequency at centre_frequency table
> +			d->frequency_loop_length++;
> +			d->centre_frequency = realloc(d->centre_frequency,
                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Same, except ...

> +						      d->frequency_loop_length * sizeof(*d->centre_frequency));
> +			memcpy(&d->centre_frequency[pos], p, sizeof(*d->centre_frequency));
                               ^^^^^^^^^^^^^^^^^^^^^^^^^

No check for an error (inconsistent with the previous realloc of same field)

> [...]
> 
> unblock v4l-utils/1.12.5-1
> 
> [...]
> 

Please clarify the possible ABI breakage situation and resolve the above
issues.

Thanks,
~Niels