| Message ID | 20220708091947.5610-1-srinivas.kandagatla@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | soundwire: qcom: fix max auto-enumeration devices | expand |
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
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
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;
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(-)