[PATCH] soundwire: qcom: fix max auto-enumeration devices

[PATCH] soundwire: qcom: fix max auto-enumeration devices

[Srinivas Kandagatla]

Controller only supports up to max of 1-11 device ids via auto-enumeration,
and it has only those many registers.

In the existing code, we can protentially cross this boundary and read incorrect
registers.

Cc: stable@vger.kernel.org
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Fixes: a6e6581942ca ("soundwire: qcom: add auto enumeration support")
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
Thanks to Dan for reporting an overflow issue, which turned out to be
another issue, where we could read registers that do not belong to
auto-enumeration devid.
Either way this fixes both issues, one reported by Dan and other
incorrect register access.

Thanks,
Srini

 drivers/soundwire/qcom.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c
index 9df970eeca45..dd1365a44458 100644
--- a/drivers/soundwire/qcom.c
+++ b/drivers/soundwire/qcom.c
@@ -119,6 +119,8 @@
 #define MAX_FIFO_RD_RETRY 3
 #define SWR_OVERFLOW_RETRY_COUNT 30
 #define SWRM_LINK_STATUS_RETRY_CNT 100
+/* devid 1 - 11 */
+#define SWRM_MAX_AUTO_ENUM_DEVICES	11
 
 enum {
 	MASTER_ID_WSA = 1,
@@ -479,7 +481,7 @@ static int qcom_swrm_enumerate(struct sdw_bus *bus)
 	int i;
 	char *buf1 = (char *)&val1, *buf2 = (char *)&val2;
 
-	for (i = 1; i <= SDW_MAX_DEVICES; i++) {
+	for (i = 1; i <= SWRM_MAX_AUTO_ENUM_DEVICES; i++) {
 		/* do not continue if the status is Not Present  */
 		if (!ctrl->status[i])
 			continue;
-- 
2.25.1

Re: [PATCH] soundwire: qcom: fix max auto-enumeration devices

[Dan Carpenter]

On Fri, Jul 08, 2022 at 10:19:47AM +0100, Srinivas Kandagatla wrote:
> Controller only supports up to max of 1-11 device ids via auto-enumeration,
> and it has only those many registers.
> 
> In the existing code, we can protentially cross this boundary and read incorrect
> registers.
> 
> Cc: stable@vger.kernel.org
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> Fixes: a6e6581942ca ("soundwire: qcom: add auto enumeration support")
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> ---
> Thanks to Dan for reporting an overflow issue, which turned out to be
> another issue, where we could read registers that do not belong to
> auto-enumeration devid.
> Either way this fixes both issues, one reported by Dan and other
> incorrect register access.
> 
> Thanks,
> Srini
> 
>  drivers/soundwire/qcom.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c
> index 9df970eeca45..dd1365a44458 100644
> --- a/drivers/soundwire/qcom.c
> +++ b/drivers/soundwire/qcom.c
> @@ -119,6 +119,8 @@
>  #define MAX_FIFO_RD_RETRY 3
>  #define SWR_OVERFLOW_RETRY_COUNT 30
>  #define SWRM_LINK_STATUS_RETRY_CNT 100
> +/* devid 1 - 11 */
> +#define SWRM_MAX_AUTO_ENUM_DEVICES	11
>  
>  enum {
>  	MASTER_ID_WSA = 1,
> @@ -479,7 +481,7 @@ static int qcom_swrm_enumerate(struct sdw_bus *bus)
>  	int i;
>  	char *buf1 = (char *)&val1, *buf2 = (char *)&val2;
>  
> -	for (i = 1; i <= SDW_MAX_DEVICES; i++) {
> +	for (i = 1; i <= SWRM_MAX_AUTO_ENUM_DEVICES; i++) {

I'm sorry, I don't understand.  Both of these defines are 11 so this
doesn't change anything?

regards,
dan carpenter

>  		/* do not continue if the status is Not Present  */
>  		if (!ctrl->status[i])
>  			continue;
> -- 
> 2.25.1

Re: [PATCH] soundwire: qcom: fix max auto-enumeration devices

[Srinivas Kandagatla]

On 08/07/2022 11:04, Dan Carpenter wrote:
> On Fri, Jul 08, 2022 at 10:19:47AM +0100, Srinivas Kandagatla wrote:
>> Controller only supports up to max of 1-11 device ids via auto-enumeration,
>> and it has only those many registers.
>>
>> In the existing code, we can protentially cross this boundary and read incorrect
>> registers.
>>
>> Cc: stable@vger.kernel.org
>> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
>> Fixes: a6e6581942ca ("soundwire: qcom: add auto enumeration support")
>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>> ---
>> Thanks to Dan for reporting an overflow issue, which turned out to be
>> another issue, where we could read registers that do not belong to
>> auto-enumeration devid.
>> Either way this fixes both issues, one reported by Dan and other
>> incorrect register access.
>>
>> Thanks,
>> Srini
>>
>>   drivers/soundwire/qcom.c | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c
>> index 9df970eeca45..dd1365a44458 100644
>> --- a/drivers/soundwire/qcom.c
>> +++ b/drivers/soundwire/qcom.c
>> @@ -119,6 +119,8 @@
>>   #define MAX_FIFO_RD_RETRY 3
>>   #define SWR_OVERFLOW_RETRY_COUNT 30
>>   #define SWRM_LINK_STATUS_RETRY_CNT 100
>> +/* devid 1 - 11 */
>> +#define SWRM_MAX_AUTO_ENUM_DEVICES	11
>>   
>>   enum {
>>   	MASTER_ID_WSA = 1,
>> @@ -479,7 +481,7 @@ static int qcom_swrm_enumerate(struct sdw_bus *bus)
>>   	int i;
>>   	char *buf1 = (char *)&val1, *buf2 = (char *)&val2;
>>   
>> -	for (i = 1; i <= SDW_MAX_DEVICES; i++) {
>> +	for (i = 1; i <= SWRM_MAX_AUTO_ENUM_DEVICES; i++) {
> 
> I'm sorry, I don't understand.  Both of these defines are 11 so this
> doesn't change anything?
> 
My bad, I thought this was 15...


--srini


> regards,
> dan carpenter
> 
>>   		/* do not continue if the status is Not Present  */
>>   		if (!ctrl->status[i])
>>   			continue;
>> -- 
>> 2.25.1