[PATCH] staging: android: ion: Initialize dma_address of new sg list

[PATCH] staging: android: ion: Initialize dma_address of new sg list

[Liam Mark]

Fix the dup_sg_table function to initialize the dma_address of the new
sg list entries instead of the source dma_address entries.

Fixes: 17fd283f3870 ("staging: android: ion: Duplicate sg_table")
Signed-off-by: Liam Mark <lmark@codeaurora.org>
---
 drivers/staging/android/ion/ion.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c
index f480885e346b..3ace3a0d9210 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -197,7 +197,7 @@ static struct sg_table *dup_sg_table(struct sg_table *table)
 	new_sg = new_table->sgl;
 	for_each_sg(table->sgl, sg, table->nents, i) {
 		memcpy(new_sg, sg, sizeof(*sg));
-		sg->dma_address = 0;
+		new_sg->dma_address = 0;
 		new_sg = sg_next(new_sg);
 	}
 
-- 
1.8.5.2


Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: android: ion: Initialize dma_address of new sg list

[Dan Carpenter]

On Fri, Feb 09, 2018 at 10:16:56PM -0800, Liam Mark wrote:
> Fix the dup_sg_table function to initialize the dma_address of the new
> sg list entries instead of the source dma_address entries.
> 
> Fixes: 17fd283f3870 ("staging: android: ion: Duplicate sg_table")
> Signed-off-by: Liam Mark <lmark@codeaurora.org>

How did you find this bug?  What are the user visible effects of this
bug?  I'm probably going to ask you to send a v2 patch with a new
changelog depending on the answers to these questions.

regards,
dan carpenter

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: android: ion: Initialize dma_address of new sg list

[Liam Mark]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 909 bytes --]


On Mon, 12 Feb 2018, Dan Carpenter wrote:

> On Fri, Feb 09, 2018 at 10:16:56PM -0800, Liam Mark wrote:
> > Fix the dup_sg_table function to initialize the dma_address of the new
> > sg list entries instead of the source dma_address entries.
> > 
> > Fixes: 17fd283f3870 ("staging: android: ion: Duplicate sg_table")
> > Signed-off-by: Liam Mark <lmark@codeaurora.org>
> 
> How did you find this bug?  What are the user visible effects of this
> bug?  I'm probably going to ask you to send a v2 patch with a new
> changelog depending on the answers to these questions.

I noticed the bug when reading through the code, I haven’t seen any 
visible effects of this bug.

>From looking at the code I don’t see any obvious ways that this bug would 
introduce any issues with the current code base.

Liam

Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[-- Attachment #2: Type: text/plain, Size: 169 bytes --]

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: android: ion: Initialize dma_address of new sg list

[Laura Abbott]

On 02/12/2018 01:25 PM, Liam Mark wrote:
> 
> On Mon, 12 Feb 2018, Dan Carpenter wrote:
> 
>> On Fri, Feb 09, 2018 at 10:16:56PM -0800, Liam Mark wrote:
>>> Fix the dup_sg_table function to initialize the dma_address of the new
>>> sg list entries instead of the source dma_address entries.
>>>
>>> Fixes: 17fd283f3870 ("staging: android: ion: Duplicate sg_table")
>>> Signed-off-by: Liam Mark <lmark@codeaurora.org>
>>
>> How did you find this bug?  What are the user visible effects of this
>> bug?  I'm probably going to ask you to send a v2 patch with a new
>> changelog depending on the answers to these questions.
> 
> I noticed the bug when reading through the code, I haven’t seen any
> visible effects of this bug.
> 
>  From looking at the code I don’t see any obvious ways that this bug would
> introduce any issues with the current code base.
> 
> Liam
> 

Given the way we duplicate the sg_list this should be harmless but you
are right it's cleaner if we initialize the new list. You can add my
Ack when you update with a new change log clarifying this.

Thanks,
Laura
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: android: ion: Initialize dma_address of new sg list

[Liam Mark]

On Thu, 15 Feb 2018, Laura Abbott wrote:

> On 02/12/2018 01:25 PM, Liam Mark wrote:
> > 
> > On Mon, 12 Feb 2018, Dan Carpenter wrote:
> > 
> > > On Fri, Feb 09, 2018 at 10:16:56PM -0800, Liam Mark wrote:
> > > > Fix the dup_sg_table function to initialize the dma_address of the new
> > > > sg list entries instead of the source dma_address entries.
> > > > 
> > > > Fixes: 17fd283f3870 ("staging: android: ion: Duplicate sg_table")
> > > > Signed-off-by: Liam Mark <lmark@codeaurora.org>
> > > 
> > > How did you find this bug?  What are the user visible effects of this
> > > bug?  I'm probably going to ask you to send a v2 patch with a new
> > > changelog depending on the answers to these questions.
> > 
> > I noticed the bug when reading through the code, I haven?t seen any
> > visible effects of this bug.
> > 
> >  From looking at the code I don?t see any obvious ways that this bug
> would
> > introduce any issues with the current code base.
> > 
> > Liam
> > 
> 
> Given the way we duplicate the sg_list this should be harmless but you
> are right it's cleaner if we initialize the new list. You can add my
> Ack when you update with a new change log clarifying this.
> 
> Thanks,
> Laura
> 

Will do.

Thanks,
Liam

Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: android: ion: Initialize dma_address of new sg list

[Greg KH]

On Fri, Feb 09, 2018 at 10:16:56PM -0800, Liam Mark wrote:
> Fix the dup_sg_table function to initialize the dma_address of the new
> sg list entries instead of the source dma_address entries.
> 
> Fixes: 17fd283f3870 ("staging: android: ion: Duplicate sg_table")

So this should be sent to the stable trees as well, right?

Please add that when you resend...

thanks,

greg k-h
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: android: ion: Initialize dma_address of new sg list

[Liam Mark]

On Fri, 16 Feb 2018, Greg KH wrote:

> On Fri, Feb 09, 2018 at 10:16:56PM -0800, Liam Mark wrote:
> > Fix the dup_sg_table function to initialize the dma_address of the new
> > sg list entries instead of the source dma_address entries.
> > 
> > Fixes: 17fd283f3870 ("staging: android: ion: Duplicate sg_table")
> 
> So this should be sent to the stable trees as well, right?
> 
> Please add that when you resend...

My understanding was that I should only send this to the stable branch if 
it fixes a real bug.

Both myself and Laura can't see any actual problems that result from this 
issue, the change is more to help future proof the code.

My commit message wasn't clear about this and could have mislead people 
into thinking there was a bug.
I will update the commit message to make it clear that this issue doesn't 
currently result in any problem.

Do you still want me to send it to the stable trees?

Thanks,
Liam

Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: android: ion: Initialize dma_address of new sg list

[Greg KH]

On Fri, Feb 16, 2018 at 09:57:12AM -0800, Liam Mark wrote:
> On Fri, 16 Feb 2018, Greg KH wrote:
> 
> > On Fri, Feb 09, 2018 at 10:16:56PM -0800, Liam Mark wrote:
> > > Fix the dup_sg_table function to initialize the dma_address of the new
> > > sg list entries instead of the source dma_address entries.
> > > 
> > > Fixes: 17fd283f3870 ("staging: android: ion: Duplicate sg_table")
> > 
> > So this should be sent to the stable trees as well, right?
> > 
> > Please add that when you resend...
> 
> My understanding was that I should only send this to the stable branch if 
> it fixes a real bug.
> 
> Both myself and Laura can't see any actual problems that result from this 
> issue, the change is more to help future proof the code.
> 
> My commit message wasn't clear about this and could have mislead people 
> into thinking there was a bug.
> I will update the commit message to make it clear that this issue doesn't 
> currently result in any problem.
> 
> Do you still want me to send it to the stable trees?

If it's not a bugfix, no need to.  But please clean up your changelog
text as it implies that it is a bugfix...

thanks,

greg k-h