[Sound-open-firmware] [alsa-devel] [PATCH v3 09/14] ASoC: SOF: Add firmware, loader support

[Takashi Iwai]

On Fri, 21 Dec 2018 10:05:04 +0100,
Bard liao wrote:
> 
> 
> On 12/20/2018 11:07 PM, Pierre-Louis Bossart wrote:
> >
> >>>>> + snd_sof_dsp_block_write(sdev, offset,
> >>>>> +                    (void *)block + sizeof(*block),
> >>>>> +                    block->size);
> >>>>> +
> >>>>> +        /* next block */
> >>>>> +        block = (void *)block + sizeof(*block) + block->size;
> >>>> This may lead to an unaligned access.
> >>> Did you mean we should double check the block->size to
> >>> prevent access to an invalid address?
> >> You need two types of checks for the given data:
> >> - The bounce check of block->size;
> >>    We need to avoid out-of-bounce access.
> > s/bounce/bounds ?
> >>
> >> - Alignment of block->size;
> >>    For some non-x86 platforms, the access to an unaligned address might
> >>    be illegal.
> >
> > Maybe I am missing something but I don't see any sort of explicit
> > restriction on alignment in the SOF tools. it looks implicit based
> > on address offsets and bases.
> 
> Maybe get_unaligned() is good enough to avoid unaligned access?

That's another option, but you'd need to put everywhere.

An alternative is to just copy the data on a new block header on the
stack temporarily and refer it.

Or simply assuring the alignment by checking block->size as Pierre
suggested...


Takashi


> 
> >
> > Liam, do you see any negative side effects if we enforce a 32-bit
> > alignment for all blocks (which essentially means all block sizes
> > are multiple of 4)? we can try and experiment but it's better if we
> > have an agreement on the design.
> >
> >
>