linux-kernel - Re: [PATCH 2/2] soundwire: fix port_ready[] dynamic allocation in mipi

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <e9f12e13-49e0-5306-a975-b1b854baef02@linux.intel.com>
Date:   Tue, 18 Aug 2020 07:09:41 -0500
From:   Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>
To:     Vinod Koul <vkoul@...nel.org>,
        Bard Liao <yung-chuan.liao@...ux.intel.com>
Cc:     alsa-devel@...a-project.org, linux-kernel@...r.kernel.org,
        tiwai@...e.de, broonie@...nel.org, gregkh@...uxfoundation.org,
        jank@...ence.com, srinivas.kandagatla@...aro.org,
        rander.wang@...ux.intel.com, ranjani.sridharan@...ux.intel.com,
        hui.wang@...onical.com, sanyog.r.kale@...el.com,
        mengdong.lin@...el.com, bard.liao@...el.com
Subject: Re: [PATCH 2/2] soundwire: fix port_ready[] dynamic allocation in
 mipi_disco and ASoC codecs

On 8/18/20 1:36 AM, Vinod Koul wrote:
> On 18-08-20, 01:47, Bard Liao wrote:
>> From: Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>
>>
>> The existing code allocates memory for the total number of ports.
>> This only works if the ports are contiguous, but will break if e.g. a
>> Devices uses port0, 1, and 14. The port_ready[] array would contain 3
>> elements, which would lead to an out-of-bounds access. Conversely in
>> other cases, the wrong port index would be used leading to timeouts on
>> prepare.
>>
>> This can be fixed by allocating for the worst-case of 15
>> ports (DP0..DP14). In addition since the number is now fixed, we can
>> use an array instead of a dynamic allocation.
>>
>> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>
>> Reviewed-by: Rander Wang <rander.wang@...ux.intel.com>
>> Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@...ux.intel.com>
>> Signed-off-by: Bard Liao <yung-chuan.liao@...ux.intel.com>
>> ---
>>   drivers/soundwire/mipi_disco.c  | 18 +-----------------
>>   drivers/soundwire/slave.c       |  4 ++++
>>   include/linux/soundwire/sdw.h   |  2 +-
>>   sound/soc/codecs/max98373-sdw.c | 15 +--------------
>>   sound/soc/codecs/rt1308-sdw.c   | 14 +-------------
>>   sound/soc/codecs/rt5682-sdw.c   | 15 +--------------
>>   sound/soc/codecs/rt700-sdw.c    | 15 +--------------
>>   sound/soc/codecs/rt711-sdw.c    | 15 +--------------
>>   sound/soc/codecs/rt715-sdw.c    | 33 +--------------------------------
> 
> This looks fine, but the asoc changes are not dependent, so maybe we
> should split them up and then can go thru Mark. Or Mark acks, either way
> would work for me

There are 3 dependencies that we tracked between SoundWire and ASoC 
subsystems:

a) addition of SDCA control macro (needed before SDCA codec drivers can 
be shared)
b) this series - we could indeed submit the codec changes to Mark's tree 
separately, but then the SoundWire tree would be broken: the codec 
drivers would still try to allocate dynamically what is now a fixed-size 
array.
c) configuration of the interrupt masks in codec drivers instead of 
hard-coded in bus driver + spurious parity error workaround (not posted 
yet but ready).

The changes in ASoC codecs are really only on the initialization part 
(either removing a dynamic allocation or setting masks), there's no 
functional change otherwise.

I think the simplest to avoid multiple back-and-forth is to have these 
small interface/initialization changes merged through the SoundWire 
subsystem, then merged by Mark from a single immutable tag. Would this 
work for everyone?

In addition, there's a WIP change to regmap to add support for SoundWire 
1.2 MBQ-based register access, but this only affects regmap and ASoC 
trees, all handled by Mark.

I don't think we have any other cross-tree changes planned for now, the 
SDCA infrastructure plumbing is still rather open.