[PATCH] v4l: Cast timestamp tv_usec to singned

[PATCH] v4l: Cast timestamp tv_usec to singned

[Stanimir Varbanov]

Some of the MPEG4 standards allows negative timestamps. Correct
tv_usec cast to s32.

Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
---
 include/media/v4l2-common.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h
index 3eb202259e8c..1ed61416003a 100644
--- a/include/media/v4l2-common.h
+++ b/include/media/v4l2-common.h
@@ -544,11 +544,11 @@ static inline u64 v4l2_buffer_get_timestamp(const struct v4l2_buffer *buf)
 {
 	/*
 	 * When the timestamp comes from 32-bit user space, there may be
-	 * uninitialized data in tv_usec, so cast it to u32.
+	 * uninitialized data in tv_usec, so cast it to s32.
 	 * Otherwise allow invalid input for backwards compatibility.
 	 */
 	return buf->timestamp.tv_sec * NSEC_PER_SEC +
-		(u32)buf->timestamp.tv_usec * NSEC_PER_USEC;
+		(s32)buf->timestamp.tv_usec * NSEC_PER_USEC;
 }
 
 static inline void v4l2_buffer_set_timestamp(struct v4l2_buffer *buf,
-- 
2.25.1

Re: [PATCH] v4l: Cast timestamp tv_usec to singned

[Arnd Bergmann]

On Thu, Mar 25, 2021 at 11:29 AM Stanimir Varbanov
<stanimir.varbanov@linaro.org> wrote:
>
> Some of the MPEG4 standards allows negative timestamps. Correct
> tv_usec cast to s32.
>
> Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>

Can you clarify what exactly is allowed in MPEG4? Normally we require
a normalized timestamp to come from user space in other kernel interfaces,
i.e. an arbitrary .tv_sec value that may or may not be negative, plus
a sub-second .tv_usec between 0 and 999999, or a .tv_nsect between 0
and 999999999 to indicate the time after the last full second.

E.g. a negative timestamp of -1.0001 seconds would be represented as
.tv_sec = -2, .tv_usec = 999900.

What is the range defined in MPEG4? It might be necessary to check
for the specific range and reject anything outside of that, in particular
if MPEG4 also allows positive microsecond values larger than 999999.

         Arnd

>  include/media/v4l2-common.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h
> index 3eb202259e8c..1ed61416003a 100644
> --- a/include/media/v4l2-common.h
> +++ b/include/media/v4l2-common.h
> @@ -544,11 +544,11 @@ static inline u64 v4l2_buffer_get_timestamp(const struct v4l2_buffer *buf)
>  {
>         /*
>          * When the timestamp comes from 32-bit user space, there may be
> -        * uninitialized data in tv_usec, so cast it to u32.
> +        * uninitialized data in tv_usec, so cast it to s32.
>          * Otherwise allow invalid input for backwards compatibility.
>          */
>         return buf->timestamp.tv_sec * NSEC_PER_SEC +
> -               (u32)buf->timestamp.tv_usec * NSEC_PER_USEC;
> +               (s32)buf->timestamp.tv_usec * NSEC_PER_USEC;
>  }
>

Re: [PATCH] v4l: Cast timestamp tv_usec to singned

[Stanimir Varbanov]

On 3/25/21 12:51 PM, Arnd Bergmann wrote:
> On Thu, Mar 25, 2021 at 11:29 AM Stanimir Varbanov
> <stanimir.varbanov@linaro.org> wrote:
>>
>> Some of the MPEG4 standards allows negative timestamps. Correct
>> tv_usec cast to s32.
>>
>> Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
> 
> Can you clarify what exactly is allowed in MPEG4? Normally we require
> a normalized timestamp to come from user space in other kernel interfaces,
> i.e. an arbitrary .tv_sec value that may or may not be negative, plus
> a sub-second .tv_usec between 0 and 999999, or a .tv_nsect between 0
> and 999999999 to indicate the time after the last full second.
> 
> E.g. a negative timestamp of -1.0001 seconds would be represented as
> .tv_sec = -2, .tv_usec = 999900.

Sure, I will try to collect some more info. Thanks!

> 
> What is the range defined in MPEG4? It might be necessary to check
> for the specific range and reject anything outside of that, in particular
> if MPEG4 also allows positive microsecond values larger than 999999.
> 
>          Arnd
> 
>>  include/media/v4l2-common.h | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h
>> index 3eb202259e8c..1ed61416003a 100644
>> --- a/include/media/v4l2-common.h
>> +++ b/include/media/v4l2-common.h
>> @@ -544,11 +544,11 @@ static inline u64 v4l2_buffer_get_timestamp(const struct v4l2_buffer *buf)
>>  {
>>         /*
>>          * When the timestamp comes from 32-bit user space, there may be
>> -        * uninitialized data in tv_usec, so cast it to u32.
>> +        * uninitialized data in tv_usec, so cast it to s32.
>>          * Otherwise allow invalid input for backwards compatibility.
>>          */
>>         return buf->timestamp.tv_sec * NSEC_PER_SEC +
>> -               (u32)buf->timestamp.tv_usec * NSEC_PER_USEC;
>> +               (s32)buf->timestamp.tv_usec * NSEC_PER_USEC;
>>  }
>>

-- 
regards,
Stan