[FFmpeg-devel] [PATCH] aes: fix array index out of bounds warnings

[Reimar Döffinger]

On Sat, Jun 26, 2010 at 09:12:52AM +0100, M?ns Rullg?rd wrote:
> Reimar D?ffinger <Reimar.Doeffinger at gmx.de> writes:
> 
> > On Sat, Jun 26, 2010 at 12:54:21AM +0100, Mans Rullgard wrote:
> >> ---
> >>  libavutil/aes.c |    4 ++--
> >>  1 files changed, 2 insertions(+), 2 deletions(-)
> >> 
> >> diff --git a/libavutil/aes.c b/libavutil/aes.c
> >> index d3a271c..763f856 100644
> >> --- a/libavutil/aes.c
> >> +++ b/libavutil/aes.c
> >> @@ -130,7 +130,7 @@ int av_aes_init(AVAES *a, const uint8_t *key, int key_bits, int decrypt) {
> >>      uint8_t  log8[256];
> >>      uint8_t alog8[512];
> >>  
> >> -    if(!enc_multbl[0][sizeof(enc_multbl)/sizeof(enc_multbl[0][0])-1]){
> >> +    if(!enc_multbl[FF_ARRAY_ELEMS(enc_multbl)-1][FF_ARRAY_ELEMS(enc_multbl[0])-1]){
> >>          j=1;
> >>          for(i=0; i<255; i++){
> >>              alog8[i]=
> >> @@ -184,7 +184,7 @@ int av_aes_init(AVAES *a, const uint8_t *key, int key_bits, int decrypt) {
> >>      }else{
> >>          for(i=0; i<(rounds+1)>>1; i++){
> >>              for(j=0; j<16; j++)
> >> -                FFSWAP(int, a->round_key[i][0][j], a->round_key[rounds-i][0][j]);
> >> +                FFSWAP(int, a->round_key[i][j>>2][j&3], a->round_key[rounds-i][j>>2][j&3]);
> >
> > If the compilers were any good this would be the same,
> 
> The existing code has UNDEFINED BEHAVIOUR from indexing outside an
> array.  Talk about same has no meaning.

I admit I didn't check the standard about that special case but I
think the conclusion was that given the other constraints for a
two-dimensional array there isn't a good reason for it not to
work in any implementation.

> > but last time this was checked IIRC produced quite a lot slower
> > code.
> 
> And that matters in an init function?  I also can't find anyone ever
> claiming to have actually checked it.  All I do find is Michael
> vehemently rejecting the notion of the out-of-array indexing being a
> bug.

Sorry, I didn't realize that. For AES the speed of the init function
might be someonewhat relevant (I think it would matter the way it is misused
as has etc. for BluRay), however this is even the encode init function.
I retract my objection.