[FFmpeg-devel] libavutil/encryption_info: Allow multiple init info.
Michael Niedermayer
michael at niedermayer.cc
Thu Jun 21 19:47:55 EEST 2018
On Thu, Jun 07, 2018 at 11:51:30AM -0700, Jacob Trimble wrote:
> On Thu, May 31, 2018 at 5:50 PM Jacob Trimble <modmaker at google.com> wrote:
> >
> > On Thu, May 31, 2018 at 9:40 AM Jacob Trimble <modmaker at google.com> wrote:
> > >
> > > On Fri, May 25, 2018 at 6:13 PM Michael Niedermayer
> > > <michael at niedermayer.cc> wrote:
> > > >
> > > > [...]
> > > >
> > > > > Added fix for issue found by Chrome's ClusterFuzz (http://crbug.com/846662).
> > > >
> > > > this belongs in a seperate patch unless its a bug specific to the code added
> > > > with this patch
> > > >
> > >
> > > Ok. Now this patch depends on
> > > http://ffmpeg.org/pipermail/ffmpeg-devel/2018-May/230782.html.
> > >
> >
> > Noticed some bugs when integrating it.
> >
> > > > [...]
> > > >
> > > > --
> > > > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
> > > >
> > > > I have often repented speaking, but never of holding my tongue.
> > > > -- Xenocrates
> > > > _______________________________________________
> > > > ffmpeg-devel mailing list
> > > > ffmpeg-devel at ffmpeg.org
> > > > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> Removed controversial NULL checks. This patch no longer depends on anything.
> encryption_info.c | 140 ++++++++++++++++++++++++++++++++++++------------------
> encryption_info.h | 5 +
> 2 files changed, 100 insertions(+), 45 deletions(-)
> cc25918cb63c4ac75f0b693472b51590c1a74052 0001-libavutil-encryption_info-Allow-multiple-init-info-v7.patch
> From 0c4c33f26ba4204a775709cdd6367dd1ea4bd024 Mon Sep 17 00:00:00 2001
> From: Jacob Trimble <modmaker at google.com>
> Date: Mon, 23 Apr 2018 10:33:58 -0700
> Subject: [PATCH] libavutil/encryption_info: Allow multiple init info.
>
> It is possible for there to be multiple encryption init info structure.
> For example, to support multiple key systems or in key rotation. This
> changes the AVEncryptionInitInfo struct to be a linked list so there
> can be multiple structs without breaking ABI.
>
> Signed-off-by: Jacob Trimble <modmaker at google.com>
> ---
> libavutil/encryption_info.c | 140 ++++++++++++++++++++++++------------
> libavutil/encryption_info.h | 5 ++
> 2 files changed, 100 insertions(+), 45 deletions(-)
>
> diff --git a/libavutil/encryption_info.c b/libavutil/encryption_info.c
> index 20a752d6b4..1072d2795b 100644
> --- a/libavutil/encryption_info.c
> +++ b/libavutil/encryption_info.c
> @@ -160,13 +160,16 @@ uint8_t *av_encryption_info_add_side_data(const AVEncryptionInfo *info, size_t *
> }
>
> // The format of the AVEncryptionInitInfo side data:
> -// u32be system_id_size
> -// u32be num_key_ids
> -// u32be key_id_size
> -// u32be data_size
> -// u8[system_id_size] system_id
> -// u8[key_id_size][num_key_id] key_ids
> -// u8[data_size] data
> +// u32be init_info_count
> +// {
> +// u32be system_id_size
> +// u32be num_key_ids
> +// u32be key_id_size
> +// u32be data_size
> +// u8[system_id_size] system_id
> +// u8[key_id_size][num_key_id] key_ids
> +// u8[data_size] data
> +// }[init_info_count]
>
> #define FF_ENCRYPTION_INIT_INFO_EXTRA 16
>
> @@ -215,6 +218,7 @@ void av_encryption_init_info_free(AVEncryptionInitInfo *info)
> for (i = 0; i < info->num_key_ids; i++) {
> av_free(info->key_ids[i]);
> }
> + av_encryption_init_info_free(info->next);
> av_free(info->system_id);
> av_free(info->key_ids);
> av_free(info->data);
> @@ -225,71 +229,117 @@ void av_encryption_init_info_free(AVEncryptionInitInfo *info)
> AVEncryptionInitInfo *av_encryption_init_info_get_side_data(
> const uint8_t *side_data, size_t side_data_size)
> {
> - AVEncryptionInitInfo *info;
> - uint64_t system_id_size, num_key_ids, key_id_size, data_size, i;
> + // |ret| tracks the front of the list, |info| tracks the back.
> + AVEncryptionInitInfo *ret = NULL, *info, *temp_info;
> + uint64_t system_id_size, num_key_ids, key_id_size, data_size, i, j;
> + uint64_t init_info_count;
>
> - if (!side_data || side_data_size < FF_ENCRYPTION_INIT_INFO_EXTRA)
> + if (!side_data || side_data_size < 4)
> return NULL;
>
> - system_id_size = AV_RB32(side_data);
> - num_key_ids = AV_RB32(side_data + 4);
> - key_id_size = AV_RB32(side_data + 8);
> - data_size = AV_RB32(side_data + 12);
> + init_info_count = AV_RB32(side_data);
> + side_data += 4;
> + side_data_size -= 4;
> + for (i = 0; i < init_info_count; i++) {
> + if (side_data_size < FF_ENCRYPTION_INIT_INFO_EXTRA) {
> + av_encryption_init_info_free(ret);
> + return NULL;
> + }
>
> - // UINT32_MAX + UINT32_MAX + UINT32_MAX * UINT32_MAX == UINT64_MAX
> - if (side_data_size - FF_ENCRYPTION_INIT_INFO_EXTRA < system_id_size + data_size + num_key_ids * key_id_size)
> - return NULL;
> + system_id_size = AV_RB32(side_data);
> + num_key_ids = AV_RB32(side_data + 4);
> + key_id_size = AV_RB32(side_data + 8);
> + data_size = AV_RB32(side_data + 12);
>
> - info = av_encryption_init_info_alloc(system_id_size, num_key_ids, key_id_size, data_size);
> - if (!info)
> - return NULL;
> + // UINT32_MAX + UINT32_MAX + UINT32_MAX * UINT32_MAX == UINT64_MAX
> + if (side_data_size - FF_ENCRYPTION_INIT_INFO_EXTRA < system_id_size + data_size + num_key_ids * key_id_size) {
> + av_encryption_init_info_free(ret);
> + return NULL;
> + }
> + side_data += FF_ENCRYPTION_INIT_INFO_EXTRA;
> + side_data_size -= FF_ENCRYPTION_INIT_INFO_EXTRA;
>
> - memcpy(info->system_id, side_data + 16, system_id_size);
> - side_data += system_id_size + 16;
> - for (i = 0; i < num_key_ids; i++) {
> - memcpy(info->key_ids[i], side_data, key_id_size);
> - side_data += key_id_size;
> + temp_info = av_encryption_init_info_alloc(system_id_size, num_key_ids, key_id_size, data_size);
> + if (!temp_info) {
> + av_encryption_init_info_free(ret);
> + return NULL;
> + }
> + if (i == 0) {
> + info = ret = temp_info;
> + } else {
> + info->next = temp_info;
> + info = temp_info;
> + }
> +
> + memcpy(info->system_id, side_data, system_id_size);
> + side_data += system_id_size;
> + side_data_size -= system_id_size;
> + for (j = 0; j < num_key_ids; j++) {
> + memcpy(info->key_ids[j], side_data, key_id_size);
> + side_data += key_id_size;
> + side_data_size -= key_id_size;
> + }
> + memcpy(info->data, side_data, data_size);
> + side_data += data_size;
> + side_data_size -= data_size;
> }
> - memcpy(info->data, side_data, data_size);
>
> - return info;
> + return ret;
> }
>
> uint8_t *av_encryption_init_info_add_side_data(const AVEncryptionInitInfo *info, size_t *side_data_size)
> {
> + const AVEncryptionInitInfo *cur_info;
> uint8_t *buffer, *cur_buffer;
> - uint32_t i, max_size;
> + uint32_t i, init_info_count;
>
> if (UINT32_MAX - FF_ENCRYPTION_INIT_INFO_EXTRA < info->system_id_size ||
> UINT32_MAX - FF_ENCRYPTION_INIT_INFO_EXTRA - info->system_id_size < info->data_size) {
> return NULL;
> }
>
> - if (info->num_key_ids) {
> - max_size = UINT32_MAX - FF_ENCRYPTION_INIT_INFO_EXTRA - info->system_id_size - info->data_size;
> - if (max_size / info->num_key_ids < info->key_id_size)
> + *side_data_size = 4;
> + init_info_count = 0;
> + for (cur_info = info; cur_info; cur_info = cur_info->next) {
> + if (UINT32_MAX == init_info_count ||
> + UINT32_MAX - *side_data_size < FF_ENCRYPTION_INIT_INFO_EXTRA ||
> + UINT32_MAX - *side_data_size - FF_ENCRYPTION_INIT_INFO_EXTRA < info->system_id_size ||
> + UINT32_MAX - *side_data_size - FF_ENCRYPTION_INIT_INFO_EXTRA - info->system_id_size < info->data_size) {
> return NULL;
> + }
you can simplify this with (u)int64_t
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Let us carefully observe those good qualities wherein our enemies excel us
and endeavor to excel them, by avoiding what is faulty, and imitating what
is excellent in them. -- Plutarch
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20180621/449b82b6/attachment.sig>
More information about the ffmpeg-devel
mailing list