[FFmpeg-devel] [PATCH 1/5] avcodec/lpc: copy levenson coeffs only when they have been computed

[Lynne]

On 12/05/2024 02:18, Michael Niedermayer wrote:
> On Sun, May 12, 2024 at 02:13:06AM +0200, Lynne via ffmpeg-devel wrote:
>> On 12/05/2024 02:03, Michael Niedermayer wrote:
>>> Fixes: CID1473514 Uninitialized scalar variable
>>>
>>> Sponsored-by: Sovereign Tech Fund
>>> Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
>>> ---
>>>    libavcodec/lpc.c | 5 +++--
>>>    1 file changed, 3 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/libavcodec/lpc.c b/libavcodec/lpc.c
>>> index 8305cc0596a..981dacce8a5 100644
>>> --- a/libavcodec/lpc.c
>>> +++ b/libavcodec/lpc.c
>>> @@ -282,8 +282,9 @@ int ff_lpc_calc_coefs(LPCContext *s,
>>>            double av_uninit(weight);
>>>            memset(var, 0, FFALIGN(MAX_LPC_ORDER+1,4)*sizeof(*var));
>>> -        for(j=0; j<max_order; j++)
>>> -            m[0].coeff[max_order-1][j] = -lpc[max_order-1][j];
>>> +        if (lpc_passes > 1)
>>> +            for(j=0; j<max_order; j++)
>>> +                m[0].coeff[max_order-1][j] = -lpc[max_order-1][j];
>>>            for(; pass<lpc_passes; pass++){
>>>                avpriv_init_lls(&m[pass&1], max_order);
>>
>> max_order is a function argument, I don't think that's the right place to
>> fix this.
> 
> max_orders is fine
> 
> what the problem is, is that CHOLESKY with lpc_passes = 1
> skips the first pass LEVINSON but this line copies the output
> from LEVINSON so it copies Uninitialized data.
> a few lines later thats cleared with avpriv_init_lls()
> but that access to uninitialized data i think is undefined behavior
> 
> if my analysis is not wrong then i think my fix is correct
> 
> thx
> 
> [...]
> 
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".

Ah, I see. Could you put a small comment above, like:
/* Avoids initializing with an unused value when max_order == 1 */?
Other than that looks fine.