* [PATCH] dmaengine: coh901318: fix integer overflow when shifting more than 32 places
@ 2016-09-29 17:57 ` Colin King
0 siblings, 0 replies; 10+ messages in thread
From: Colin King @ 2016-09-29 17:57 UTC (permalink / raw)
To: linux-arm-kernel
From: Colin Ian King <colin.king@canonical.com>
Currently U300_DMA_CHANNELS is set to 40, meaning that the shift of 1 can
be more than 32 places, which leads to a 32 bit integer overflow. Fix this
by casting 1 to a u64 (the same type as started_channels) before shifting
it.
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
drivers/dma/coh901318.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/dma/coh901318.c b/drivers/dma/coh901318.c
index 2835f3e..98611e3 100644
--- a/drivers/dma/coh901318.c
+++ b/drivers/dma/coh901318.c
@@ -1353,7 +1353,7 @@ static ssize_t coh901318_debugfs_read(struct file *file, char __user *buf,
tmp += sprintf(tmp, "DMA -- enabled dma channels\n");
for (i = 0; i < U300_DMA_CHANNELS; i++)
- if (started_channels & (1 << i))
+ if (started_channels & ((u64)1 << i))
tmp += sprintf(tmp, "channel %d\n", i);
tmp += sprintf(tmp, "Pool alloc nbr %d\n", pool_count);
--
2.9.3
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH] dmaengine: coh901318: fix integer overflow when shifting more than 32 places
@ 2016-09-29 17:57 ` Colin King
0 siblings, 0 replies; 10+ messages in thread
From: Colin King @ 2016-09-29 17:57 UTC (permalink / raw)
To: Linus Walleij, Dan Williams, Vinod Koul, linux-arm-kernel,
dmaengine
Cc: linux-kernel
From: Colin Ian King <colin.king@canonical.com>
Currently U300_DMA_CHANNELS is set to 40, meaning that the shift of 1 can
be more than 32 places, which leads to a 32 bit integer overflow. Fix this
by casting 1 to a u64 (the same type as started_channels) before shifting
it.
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
drivers/dma/coh901318.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/dma/coh901318.c b/drivers/dma/coh901318.c
index 2835f3e..98611e3 100644
--- a/drivers/dma/coh901318.c
+++ b/drivers/dma/coh901318.c
@@ -1353,7 +1353,7 @@ static ssize_t coh901318_debugfs_read(struct file *file, char __user *buf,
tmp += sprintf(tmp, "DMA -- enabled dma channels\n");
for (i = 0; i < U300_DMA_CHANNELS; i++)
- if (started_channels & (1 << i))
+ if (started_channels & ((u64)1 << i))
tmp += sprintf(tmp, "channel %d\n", i);
tmp += sprintf(tmp, "Pool alloc nbr %d\n", pool_count);
--
2.9.3
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH] dmaengine: coh901318: fix integer overflow when shifting more than 32 places
2016-09-29 17:57 ` Colin King
@ 2016-09-29 18:06 ` Joe Perches
-1 siblings, 0 replies; 10+ messages in thread
From: Joe Perches @ 2016-09-29 18:06 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, 2016-09-29 at 18:57 +0100, Colin King wrote:
> Currently U300_DMA_CHANNELS is set to 40, meaning that the shift of 1 can
> be more than 32 places, which leads to a 32 bit integer overflow. Fix this
> by casting 1 to a u64 (the same type as started_channels) before shifting
> it.
trivia:
> diff --git a/drivers/dma/coh901318.c b/drivers/dma/coh901318.c
[]
> @@ -1353,7 +1353,7 @@ static ssize_t coh901318_debugfs_read(struct file *file, char __user *buf,
> ? tmp += sprintf(tmp, "DMA -- enabled dma channels\n");
> ?
> ? for (i = 0; i < U300_DMA_CHANNELS; i++)
> - if (started_channels & (1 << i))
> + if (started_channels & ((u64)1 << i))
Using
if (started_channels & (1ULL << i))
would be more common.
It's also how started_channel bits are set and cleared later in the file.
And maybe the for loop should use braces.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] dmaengine: coh901318: fix integer overflow when shifting more than 32 places
@ 2016-09-29 18:06 ` Joe Perches
0 siblings, 0 replies; 10+ messages in thread
From: Joe Perches @ 2016-09-29 18:06 UTC (permalink / raw)
To: Colin King, Linus Walleij, Dan Williams, Vinod Koul,
linux-arm-kernel, dmaengine
Cc: linux-kernel
On Thu, 2016-09-29 at 18:57 +0100, Colin King wrote:
> Currently U300_DMA_CHANNELS is set to 40, meaning that the shift of 1 can
> be more than 32 places, which leads to a 32 bit integer overflow. Fix this
> by casting 1 to a u64 (the same type as started_channels) before shifting
> it.
trivia:
> diff --git a/drivers/dma/coh901318.c b/drivers/dma/coh901318.c
[]
> @@ -1353,7 +1353,7 @@ static ssize_t coh901318_debugfs_read(struct file *file, char __user *buf,
> tmp += sprintf(tmp, "DMA -- enabled dma channels\n");
>
> for (i = 0; i < U300_DMA_CHANNELS; i++)
> - if (started_channels & (1 << i))
> + if (started_channels & ((u64)1 << i))
Using
if (started_channels & (1ULL << i))
would be more common.
It's also how started_channel bits are set and cleared later in the file.
And maybe the for loop should use braces.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] dmaengine: coh901318: fix integer overflow when shifting more than 32 places
2016-09-29 18:06 ` Joe Perches
@ 2016-10-04 12:23 ` Linus Walleij
-1 siblings, 0 replies; 10+ messages in thread
From: Linus Walleij @ 2016-10-04 12:23 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Sep 29, 2016 at 8:06 PM, Joe Perches <joe@perches.com> wrote:
> On Thu, 2016-09-29 at 18:57 +0100, Colin King wrote:
>> Currently U300_DMA_CHANNELS is set to 40, meaning that the shift of 1 can
>> be more than 32 places, which leads to a 32 bit integer overflow. Fix this
>> by casting 1 to a u64 (the same type as started_channels) before shifting
>> it.
>
> trivia:
>
>> diff --git a/drivers/dma/coh901318.c b/drivers/dma/coh901318.c
> []
>> @@ -1353,7 +1353,7 @@ static ssize_t coh901318_debugfs_read(struct file *file, char __user *buf,
>> tmp += sprintf(tmp, "DMA -- enabled dma channels\n");
>>
>> for (i = 0; i < U300_DMA_CHANNELS; i++)
>> - if (started_channels & (1 << i))
>> + if (started_channels & ((u64)1 << i))
>
> Using
>
> if (started_channels & (1ULL << i))
>
> would be more common.
Even better (IMO):
#include <linux/bitops.h>
if (started_channels & BIT(i))
Apparently code is there to avoid the bit 31 problem, mea culpa.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] dmaengine: coh901318: fix integer overflow when shifting more than 32 places
@ 2016-10-04 12:23 ` Linus Walleij
0 siblings, 0 replies; 10+ messages in thread
From: Linus Walleij @ 2016-10-04 12:23 UTC (permalink / raw)
To: Joe Perches
Cc: Colin King, Dan Williams, Vinod Koul,
linux-arm-kernel@lists.infradead.org, dmaengine,
linux-kernel@vger.kernel.org
On Thu, Sep 29, 2016 at 8:06 PM, Joe Perches <joe@perches.com> wrote:
> On Thu, 2016-09-29 at 18:57 +0100, Colin King wrote:
>> Currently U300_DMA_CHANNELS is set to 40, meaning that the shift of 1 can
>> be more than 32 places, which leads to a 32 bit integer overflow. Fix this
>> by casting 1 to a u64 (the same type as started_channels) before shifting
>> it.
>
> trivia:
>
>> diff --git a/drivers/dma/coh901318.c b/drivers/dma/coh901318.c
> []
>> @@ -1353,7 +1353,7 @@ static ssize_t coh901318_debugfs_read(struct file *file, char __user *buf,
>> tmp += sprintf(tmp, "DMA -- enabled dma channels\n");
>>
>> for (i = 0; i < U300_DMA_CHANNELS; i++)
>> - if (started_channels & (1 << i))
>> + if (started_channels & ((u64)1 << i))
>
> Using
>
> if (started_channels & (1ULL << i))
>
> would be more common.
Even better (IMO):
#include <linux/bitops.h>
if (started_channels & BIT(i))
Apparently code is there to avoid the bit 31 problem, mea culpa.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] dmaengine: coh901318: fix integer overflow when shifting more than 32 places
2016-10-04 12:23 ` Linus Walleij
@ 2016-10-04 15:36 ` Vinod Koul
-1 siblings, 0 replies; 10+ messages in thread
From: Vinod Koul @ 2016-10-04 15:36 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Oct 04, 2016 at 02:23:51PM +0200, Linus Walleij wrote:
> On Thu, Sep 29, 2016 at 8:06 PM, Joe Perches <joe@perches.com> wrote:
> > On Thu, 2016-09-29 at 18:57 +0100, Colin King wrote:
> >> Currently U300_DMA_CHANNELS is set to 40, meaning that the shift of 1 can
> >> be more than 32 places, which leads to a 32 bit integer overflow. Fix this
> >> by casting 1 to a u64 (the same type as started_channels) before shifting
> >> it.
> >
> > trivia:
> >
> >> diff --git a/drivers/dma/coh901318.c b/drivers/dma/coh901318.c
> > []
> >> @@ -1353,7 +1353,7 @@ static ssize_t coh901318_debugfs_read(struct file *file, char __user *buf,
> >> tmp += sprintf(tmp, "DMA -- enabled dma channels\n");
> >>
> >> for (i = 0; i < U300_DMA_CHANNELS; i++)
> >> - if (started_channels & (1 << i))
> >> + if (started_channels & ((u64)1 << i))
> >
> > Using
> >
> > if (started_channels & (1ULL << i))
> >
> > would be more common.
>
> Even better (IMO):
>
> #include <linux/bitops.h>
>
> if (started_channels & BIT(i))
>
> Apparently code is there to avoid the bit 31 problem, mea culpa.
I have already applied this one, so feel free to send this as an update :)
--
~Vinod
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] dmaengine: coh901318: fix integer overflow when shifting more than 32 places
@ 2016-10-04 15:36 ` Vinod Koul
0 siblings, 0 replies; 10+ messages in thread
From: Vinod Koul @ 2016-10-04 15:36 UTC (permalink / raw)
To: Linus Walleij
Cc: Joe Perches, Colin King, Dan Williams,
linux-arm-kernel@lists.infradead.org, dmaengine,
linux-kernel@vger.kernel.org
On Tue, Oct 04, 2016 at 02:23:51PM +0200, Linus Walleij wrote:
> On Thu, Sep 29, 2016 at 8:06 PM, Joe Perches <joe@perches.com> wrote:
> > On Thu, 2016-09-29 at 18:57 +0100, Colin King wrote:
> >> Currently U300_DMA_CHANNELS is set to 40, meaning that the shift of 1 can
> >> be more than 32 places, which leads to a 32 bit integer overflow. Fix this
> >> by casting 1 to a u64 (the same type as started_channels) before shifting
> >> it.
> >
> > trivia:
> >
> >> diff --git a/drivers/dma/coh901318.c b/drivers/dma/coh901318.c
> > []
> >> @@ -1353,7 +1353,7 @@ static ssize_t coh901318_debugfs_read(struct file *file, char __user *buf,
> >> tmp += sprintf(tmp, "DMA -- enabled dma channels\n");
> >>
> >> for (i = 0; i < U300_DMA_CHANNELS; i++)
> >> - if (started_channels & (1 << i))
> >> + if (started_channels & ((u64)1 << i))
> >
> > Using
> >
> > if (started_channels & (1ULL << i))
> >
> > would be more common.
>
> Even better (IMO):
>
> #include <linux/bitops.h>
>
> if (started_channels & BIT(i))
>
> Apparently code is there to avoid the bit 31 problem, mea culpa.
I have already applied this one, so feel free to send this as an update :)
--
~Vinod
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] dmaengine: coh901318: fix integer overflow when shifting more than 32 places
2016-10-04 15:36 ` Vinod Koul
@ 2016-10-04 16:01 ` Joe Perches
-1 siblings, 0 replies; 10+ messages in thread
From: Joe Perches @ 2016-10-04 16:01 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, 2016-10-04 at 21:06 +0530, Vinod Koul wrote:
> On Tue, Oct 04, 2016 at 02:23:51PM +0200, Linus Walleij wrote:
> > On Thu, Sep 29, 2016 at 8:06 PM, Joe Perches <joe@perches.com> wrote:
> > > On Thu, 2016-09-29 at 18:57 +0100, Colin King wrote:
> > > > Currently U300_DMA_CHANNELS is set to 40, meaning that the shift of 1 can
> > > > be more than 32 places, which leads to a 32 bit integer overflow. Fix this
> > > > by casting 1 to a u64 (the same type as started_channels) before shifting
> > > > it.
> > > trivia:
> > > > diff --git a/drivers/dma/coh901318.c b/drivers/dma/coh901318.c
> > > []
> > > > @@ -1353,7 +1353,7 @@ static ssize_t coh901318_debugfs_read(struct file *file, char __user *buf,
> > > > ? tmp += sprintf(tmp, "DMA -- enabled dma channels\n");
> > > >
> > > > ? for (i = 0; i < U300_DMA_CHANNELS; i++)
> > > > - if (started_channels & (1 << i))
> > > > + if (started_channels & ((u64)1 << i))
> > > Using
> > > ? if (started_channels & (1ULL << i))
> > > would be more common.
> > Even better (IMO):
> > #include <linux/bitops.h>
> > if (started_channels & BIT(i))
> > Apparently code is there to avoid the bit 31 problem, mea culpa.
> I have already applied this one, so feel free to send this as an update :)
BIT_ULL as it still needs to be u64 not unsigned long.
But if a change is really desired, please use it
consistently in the entire file and not just this
instance.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] dmaengine: coh901318: fix integer overflow when shifting more than 32 places
@ 2016-10-04 16:01 ` Joe Perches
0 siblings, 0 replies; 10+ messages in thread
From: Joe Perches @ 2016-10-04 16:01 UTC (permalink / raw)
To: Vinod Koul, Linus Walleij
Cc: Colin King, Dan Williams, linux-arm-kernel@lists.infradead.org,
dmaengine, linux-kernel@vger.kernel.org
On Tue, 2016-10-04 at 21:06 +0530, Vinod Koul wrote:
> On Tue, Oct 04, 2016 at 02:23:51PM +0200, Linus Walleij wrote:
> > On Thu, Sep 29, 2016 at 8:06 PM, Joe Perches <joe@perches.com> wrote:
> > > On Thu, 2016-09-29 at 18:57 +0100, Colin King wrote:
> > > > Currently U300_DMA_CHANNELS is set to 40, meaning that the shift of 1 can
> > > > be more than 32 places, which leads to a 32 bit integer overflow. Fix this
> > > > by casting 1 to a u64 (the same type as started_channels) before shifting
> > > > it.
> > > trivia:
> > > > diff --git a/drivers/dma/coh901318.c b/drivers/dma/coh901318.c
> > > []
> > > > @@ -1353,7 +1353,7 @@ static ssize_t coh901318_debugfs_read(struct file *file, char __user *buf,
> > > > tmp += sprintf(tmp, "DMA -- enabled dma channels\n");
> > > >
> > > > for (i = 0; i < U300_DMA_CHANNELS; i++)
> > > > - if (started_channels & (1 << i))
> > > > + if (started_channels & ((u64)1 << i))
> > > Using
> > > if (started_channels & (1ULL << i))
> > > would be more common.
> > Even better (IMO):
> > #include <linux/bitops.h>
> > if (started_channels & BIT(i))
> > Apparently code is there to avoid the bit 31 problem, mea culpa.
> I have already applied this one, so feel free to send this as an update :)
BIT_ULL as it still needs to be u64 not unsigned long.
But if a change is really desired, please use it
consistently in the entire file and not just this
instance.
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2016-10-04 16:01 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-09-29 17:57 [PATCH] dmaengine: coh901318: fix integer overflow when shifting more than 32 places Colin King
2016-09-29 17:57 ` Colin King
2016-09-29 18:06 ` Joe Perches
2016-09-29 18:06 ` Joe Perches
2016-10-04 12:23 ` Linus Walleij
2016-10-04 12:23 ` Linus Walleij
2016-10-04 15:36 ` Vinod Koul
2016-10-04 15:36 ` Vinod Koul
2016-10-04 16:01 ` Joe Perches
2016-10-04 16:01 ` Joe Perches
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.