2023-01-31 16:01:55

by Mukesh Ojha

Subject: [PATCH] remoteproc: qcom: fix sparse warnings

This patch try to address below sparse warnings.

drivers/remoteproc/qcom_common.c:126:27: warning: restricted __le32 degrades to integer
drivers/remoteproc/qcom_common.c:133:32: warning: cast to restricted __le32
drivers/remoteproc/qcom_common.c:133:32: warning: cast from restricted __le64

Signed-off-by: Mukesh Ojha <[email protected]>
---
drivers/remoteproc/qcom_common.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/remoteproc/qcom_common.c b/drivers/remoteproc/qcom_common.c
index 020349f..7133c1f 100644
--- a/drivers/remoteproc/qcom_common.c
+++ b/drivers/remoteproc/qcom_common.c
@@ -123,14 +123,14 @@ static int qcom_add_minidump_segments(struct rproc *rproc, struct minidump_subsy

for (i = 0; i < seg_cnt; i++) {
memcpy_fromio(&region, ptr + i, sizeof(region));
- if (region.valid == MD_REGION_VALID) {
+ if (le32_to_cpu(region.valid) == MD_REGION_VALID) {
name = kstrdup(region.name, GFP_KERNEL);
if (!name) {
iounmap(ptr);
return -ENOMEM;
}
da = le64_to_cpu(region.address);
- size = le32_to_cpu(region.size);
+ size = le64_to_cpu(region.size);
rproc_coredump_add_custom_segment(rproc, da, size, NULL, name);
}
}
--
2.7.4

2023-02-12 16:11:26

by Mukesh Ojha

Subject: Re: [PATCH] remoteproc: qcom: fix sparse warnings

Friendly reminder.

-Mukesh

On 1/31/2023 9:31 PM, Mukesh Ojha wrote:
> This patch try to address below sparse warnings.
>
> drivers/remoteproc/qcom_common.c:126:27: warning: restricted __le32 degrades to integer
> drivers/remoteproc/qcom_common.c:133:32: warning: cast to restricted __le32
> drivers/remoteproc/qcom_common.c:133:32: warning: cast from restricted __le64
>
> Signed-off-by: Mukesh Ojha <[email protected]>
> ---
> drivers/remoteproc/qcom_common.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/remoteproc/qcom_common.c b/drivers/remoteproc/qcom_common.c
> index 020349f..7133c1f 100644
> --- a/drivers/remoteproc/qcom_common.c
> +++ b/drivers/remoteproc/qcom_common.c
> @@ -123,14 +123,14 @@ static int qcom_add_minidump_segments(struct rproc *rproc, struct minidump_subsy
>
> for (i = 0; i < seg_cnt; i++) {
> memcpy_fromio(&region, ptr + i, sizeof(region));
> - if (region.valid == MD_REGION_VALID) {
> + if (le32_to_cpu(region.valid) == MD_REGION_VALID) {
> name = kstrdup(region.name, GFP_KERNEL);
> if (!name) {
> iounmap(ptr);
> return -ENOMEM;
> }
> da = le64_to_cpu(region.address);
> - size = le32_to_cpu(region.size);
> + size = le64_to_cpu(region.size);
> rproc_coredump_add_custom_segment(rproc, da, size, NULL, name);
> }
> }

2023-02-13 00:55:53

by Dmitry Baryshkov

Subject: Re: [PATCH] remoteproc: qcom: fix sparse warnings

On 31/01/2023 18:01, Mukesh Ojha wrote:
> This patch try to address below sparse warnings.
>
> drivers/remoteproc/qcom_common.c:126:27: warning: restricted __le32 degrades to integer
> drivers/remoteproc/qcom_common.c:133:32: warning: cast to restricted __le32
> drivers/remoteproc/qcom_common.c:133:32: warning: cast from restricted __le64
>
> Signed-off-by: Mukesh Ojha <[email protected]>

Reviewed-by: Dmitry Baryshkov <[email protected]>

Also see below.

> ---
> drivers/remoteproc/qcom_common.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/remoteproc/qcom_common.c b/drivers/remoteproc/qcom_common.c
> index 020349f..7133c1f 100644
> --- a/drivers/remoteproc/qcom_common.c
> +++ b/drivers/remoteproc/qcom_common.c
> @@ -123,14 +123,14 @@ static int qcom_add_minidump_segments(struct rproc *rproc, struct minidump_subsy
>
> for (i = 0; i < seg_cnt; i++) {
> memcpy_fromio(&region, ptr + i, sizeof(region));
> - if (region.valid == MD_REGION_VALID) {
> + if (le32_to_cpu(region.valid) == MD_REGION_VALID) {
> name = kstrdup(region.name, GFP_KERNEL);

While you are at it, please replace this kstrdup() with kstrndup().
There is no guarantee that region.name will be 0-terminated.

> if (!name) {
> iounmap(ptr);
> return -ENOMEM;
> }
> da = le64_to_cpu(region.address);
> - size = le32_to_cpu(region.size);
> + size = le64_to_cpu(region.size);
> rproc_coredump_add_custom_segment(rproc, da, size, NULL, name);
> }
> }

--
With best wishes
Dmitry

2023-02-14 14:18:26

by Mukesh Ojha

Subject: Re: [PATCH] remoteproc: qcom: fix sparse warnings

On 2/13/2023 6:25 AM, Dmitry Baryshkov wrote:
> On 31/01/2023 18:01, Mukesh Ojha wrote:
>> This patch try to address below sparse warnings.
>>
>> drivers/remoteproc/qcom_common.c:126:27: warning: restricted __le32
>> degrades to integer
>> drivers/remoteproc/qcom_common.c:133:32: warning: cast to restricted
>> __le32
>> drivers/remoteproc/qcom_common.c:133:32: warning: cast from restricted
>> __le64
>>
>> Signed-off-by: Mukesh Ojha <[email protected]>
>
> Reviewed-by: Dmitry Baryshkov <[email protected]>
>

Thanks for the review.

> Also see below.
>
>> ---
>>   drivers/remoteproc/qcom_common.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/remoteproc/qcom_common.c
>> b/drivers/remoteproc/qcom_common.c
>> index 020349f..7133c1f 100644
>> --- a/drivers/remoteproc/qcom_common.c
>> +++ b/drivers/remoteproc/qcom_common.c
>> @@ -123,14 +123,14 @@ static int qcom_add_minidump_segments(struct
>> rproc *rproc, struct minidump_subsy
>>       for (i = 0; i < seg_cnt; i++) {
>>           memcpy_fromio(&region, ptr + i, sizeof(region));
>> -        if (region.valid == MD_REGION_VALID) {
>> +        if (le32_to_cpu(region.valid) == MD_REGION_VALID) {
>>               name = kstrdup(region.name, GFP_KERNEL);
>
> While you are at it, please replace this kstrdup() with kstrndup().
> There is no guarantee that region.name will be 0-terminated.

Kept it as separate patch in this mail thread itself.

-Mukesh
>>               if (!name) {
>>                   iounmap(ptr);
>>                   return -ENOMEM;
>>               }
>>               da = le64_to_cpu(region.address);
>> -            size = le32_to_cpu(region.size);
>> +            size = le64_to_cpu(region.size);
>>               rproc_coredump_add_custom_segment(rproc, da, size, NULL,
>> name);
>>           }
>>       }
>

2023-02-14 14:23:39

by Mukesh Ojha

Subject: [PATCH] remoteproc: qcom: replace kstrdup with kstrndup

Since, there is no guarantee that region.name will be 0-terminated
from the firmware side, replace kstrdup with kstrndup.

Suggested-by: Dmitry Baryshkov <[email protected]>
Signed-off-by: Mukesh Ojha <[email protected]>
---
drivers/remoteproc/qcom_common.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/remoteproc/qcom_common.c b/drivers/remoteproc/qcom_common.c
index 020349f..7810f91 100644
--- a/drivers/remoteproc/qcom_common.c
+++ b/drivers/remoteproc/qcom_common.c
@@ -124,7 +124,7 @@ static int qcom_add_minidump_segments(struct rproc *rproc, struct minidump_subsy
for (i = 0; i < seg_cnt; i++) {
memcpy_fromio(&region, ptr + i, sizeof(region));
if (region.valid == MD_REGION_VALID) {
- name = kstrdup(region.name, GFP_KERNEL);
+ name = kstrndup(region.name, MAX_REGION_NAME_LENGTH - 1, GFP_KERNEL);
if (!name) {
iounmap(ptr);
return -ENOMEM;
--
2.7.4

2023-02-14 16:37:45

by Bjorn Andersson

Subject: Re: [PATCH] remoteproc: qcom: replace kstrdup with kstrndup

On Tue, Feb 14, 2023 at 07:38:11PM +0530, Mukesh Ojha wrote:
> Since, there is no guarantee that region.name will be 0-terminated
> from the firmware side, replace kstrdup with kstrndup.
>
> Suggested-by: Dmitry Baryshkov <[email protected]>
> Signed-off-by: Mukesh Ojha <[email protected]>

Please don't send additional patches in-reply-to another patch, it makes
it impossible to use b4 to pick up the first patch.

And please don't send two patches which clearly will conflict with
each other. Now I had to manually apply the first patch...

Regards,
Bjorn

> ---
> drivers/remoteproc/qcom_common.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/remoteproc/qcom_common.c b/drivers/remoteproc/qcom_common.c
> index 020349f..7810f91 100644
> --- a/drivers/remoteproc/qcom_common.c
> +++ b/drivers/remoteproc/qcom_common.c
> @@ -124,7 +124,7 @@ static int qcom_add_minidump_segments(struct rproc *rproc, struct minidump_subsy
> for (i = 0; i < seg_cnt; i++) {
> memcpy_fromio(&region, ptr + i, sizeof(region));
> if (region.valid == MD_REGION_VALID) {
> - name = kstrdup(region.name, GFP_KERNEL);
> + name = kstrndup(region.name, MAX_REGION_NAME_LENGTH - 1, GFP_KERNEL);
> if (!name) {
> iounmap(ptr);
> return -ENOMEM;
> --
> 2.7.4
>

2023-02-14 17:28:42

by Bjorn Andersson

Subject: Re: [PATCH] remoteproc: qcom: replace kstrdup with kstrndup

On Tue, 14 Feb 2023 19:38:11 +0530, Mukesh Ojha wrote:
> Since, there is no guarantee that region.name will be 0-terminated
> from the firmware side, replace kstrdup with kstrndup.
>
>

Applied, thanks!

[1/1] remoteproc: qcom: replace kstrdup with kstrndup
commit: 9d5b9ad97f83b2390a6006eeb5ae5e48ec4298ce

Best regards,
--
Bjorn Andersson <[email protected]>

2023-02-15 05:17:14

by Mukesh Ojha

Subject: Re: [PATCH] remoteproc: qcom: replace kstrdup with kstrndup

On 2/14/2023 10:09 PM, Bjorn Andersson wrote:
> On Tue, Feb 14, 2023 at 07:38:11PM +0530, Mukesh Ojha wrote:
>> Since, there is no guarantee that region.name will be 0-terminated
>> from the firmware side, replace kstrdup with kstrndup.
>>
>> Suggested-by: Dmitry Baryshkov <[email protected]>
>> Signed-off-by: Mukesh Ojha <[email protected]>
>
> Please don't send additional patches in-reply-to another patch, it makes
> it impossible to use b4 to pick up the first patch.
>
> And please don't send two patches which clearly will conflict with
> each other. Now I had to manually apply the first patch...
>

Really sorry, for the inconvenience.
will keep in mind.

Thanks,
-Mukesh

> Regards,
> Bjorn
>
>> ---
>> drivers/remoteproc/qcom_common.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/remoteproc/qcom_common.c b/drivers/remoteproc/qcom_common.c
>> index 020349f..7810f91 100644
>> --- a/drivers/remoteproc/qcom_common.c
>> +++ b/drivers/remoteproc/qcom_common.c
>> @@ -124,7 +124,7 @@ static int qcom_add_minidump_segments(struct rproc *rproc, struct minidump_subsy
>> for (i = 0; i < seg_cnt; i++) {
>> memcpy_fromio(&region, ptr + i, sizeof(region));
>> if (region.valid == MD_REGION_VALID) {
>> - name = kstrdup(region.name, GFP_KERNEL);
>> + name = kstrndup(region.name, MAX_REGION_NAME_LENGTH - 1, GFP_KERNEL);
>> if (!name) {
>> iounmap(ptr);
>> return -ENOMEM;
>> --
>> 2.7.4
>>