* [PATCH] mailbox: pcc: fix channel calculation in get_pcc_channel() @ 2015-12-10 17:28 Alexey Klimov 2015-12-10 18:19 ` Ashwin Chaugule 0 siblings, 1 reply; 9+ messages in thread From: Alexey Klimov @ 2015-12-10 17:28 UTC (permalink / raw) To: jassisinghbrar, ashwin.chaugule; +Cc: alexey.klimov, sudeep.holla, linux-kernel This patch fixes the calculation of pcc_chan for non-zero id. After the compiler ignores the (unsigned long) cast the pcc_mbox_channels pointer is type-cast and then the type-cast offset is added which results in address outside of the range leading to the kernel crashing. We might add braces and make it: pcc_chan = (struct mbox_chan *) ((unsigned long) pcc_mbox_channels + (id * sizeof(*pcc_chan))); but let's go with array approach here and use id as index. Tested on Juno board. Acked-by: Sudeep Holla <sudeep.holla@arm.com> Signed-off-by: Alexey Klimov <alexey.klimov@arm.com> --- drivers/mailbox/pcc.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c index 45d85ae..8f779a1 100644 --- a/drivers/mailbox/pcc.c +++ b/drivers/mailbox/pcc.c @@ -81,16 +81,10 @@ static struct mbox_controller pcc_mbox_ctrl = {}; */ static struct mbox_chan *get_pcc_channel(int id) { - struct mbox_chan *pcc_chan; - if (id < 0 || id > pcc_mbox_ctrl.num_chans) return ERR_PTR(-ENOENT); - pcc_chan = (struct mbox_chan *) - (unsigned long) pcc_mbox_channels + - (id * sizeof(*pcc_chan)); - - return pcc_chan; + return &pcc_mbox_channels[id]; } /** -- 1.9.1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] mailbox: pcc: fix channel calculation in get_pcc_channel() 2015-12-10 17:28 [PATCH] mailbox: pcc: fix channel calculation in get_pcc_channel() Alexey Klimov @ 2015-12-10 18:19 ` Ashwin Chaugule 2016-01-15 18:20 ` Ashwin Chaugule 0 siblings, 1 reply; 9+ messages in thread From: Ashwin Chaugule @ 2015-12-10 18:19 UTC (permalink / raw) To: Alexey Klimov; +Cc: Jassi Brar, Sudeep Holla, lkml On 10 December 2015 at 12:28, Alexey Klimov <alexey.klimov@arm.com> wrote: > This patch fixes the calculation of pcc_chan for non-zero id. > After the compiler ignores the (unsigned long) cast the > pcc_mbox_channels pointer is type-cast and then the type-cast > offset is added which results in address outside of the range > leading to the kernel crashing. > > We might add braces and make it: > > pcc_chan = (struct mbox_chan *) > ((unsigned long) pcc_mbox_channels + > (id * sizeof(*pcc_chan))); > > but let's go with array approach here and use id as index. > > Tested on Juno board. > > Acked-by: Sudeep Holla <sudeep.holla@arm.com> > Signed-off-by: Alexey Klimov <alexey.klimov@arm.com> > --- > drivers/mailbox/pcc.c | 8 +------- > 1 file changed, 1 insertion(+), 7 deletions(-) > > diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c > index 45d85ae..8f779a1 100644 > --- a/drivers/mailbox/pcc.c > +++ b/drivers/mailbox/pcc.c > @@ -81,16 +81,10 @@ static struct mbox_controller pcc_mbox_ctrl = {}; > */ > static struct mbox_chan *get_pcc_channel(int id) > { > - struct mbox_chan *pcc_chan; > - > if (id < 0 || id > pcc_mbox_ctrl.num_chans) > return ERR_PTR(-ENOENT); > > - pcc_chan = (struct mbox_chan *) > - (unsigned long) pcc_mbox_channels + > - (id * sizeof(*pcc_chan)); > - > - return pcc_chan; > + return &pcc_mbox_channels[id]; > } > Strange that we didn't catch this even with a non-zero id. But the change makes sense so.. Acked-by: Ashwin Chaugule <ashwin.chaugule@linaro.org> Thanks, Ashwin. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] mailbox: pcc: fix channel calculation in get_pcc_channel() 2015-12-10 18:19 ` Ashwin Chaugule @ 2016-01-15 18:20 ` Ashwin Chaugule 2016-01-15 18:22 ` Ashwin Chaugule 0 siblings, 1 reply; 9+ messages in thread From: Ashwin Chaugule @ 2016-01-15 18:20 UTC (permalink / raw) To: Alexey Klimov; +Cc: Jassi Brar, Sudeep Holla, lkml Jassi, On 10 December 2015 at 13:19, Ashwin Chaugule <ashwin.chaugule@linaro.org> wrote: > On 10 December 2015 at 12:28, Alexey Klimov <alexey.klimov@arm.com> wrote: >> This patch fixes the calculation of pcc_chan for non-zero id. >> After the compiler ignores the (unsigned long) cast the >> pcc_mbox_channels pointer is type-cast and then the type-cast >> offset is added which results in address outside of the range >> leading to the kernel crashing. >> >> We might add braces and make it: >> >> pcc_chan = (struct mbox_chan *) >> ((unsigned long) pcc_mbox_channels + >> (id * sizeof(*pcc_chan))); >> >> but let's go with array approach here and use id as index. >> >> Tested on Juno board. >> >> Acked-by: Sudeep Holla <sudeep.holla@arm.com> >> Signed-off-by: Alexey Klimov <alexey.klimov@arm.com> >> --- >> drivers/mailbox/pcc.c | 8 +------- >> 1 file changed, 1 insertion(+), 7 deletions(-) >> >> diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c >> index 45d85ae..8f779a1 100644 >> --- a/drivers/mailbox/pcc.c >> +++ b/drivers/mailbox/pcc.c >> @@ -81,16 +81,10 @@ static struct mbox_controller pcc_mbox_ctrl = {}; >> */ >> static struct mbox_chan *get_pcc_channel(int id) >> { >> - struct mbox_chan *pcc_chan; >> - >> if (id < 0 || id > pcc_mbox_ctrl.num_chans) >> return ERR_PTR(-ENOENT); >> >> - pcc_chan = (struct mbox_chan *) >> - (unsigned long) pcc_mbox_channels + >> - (id * sizeof(*pcc_chan)); >> - >> - return pcc_chan; >> + return &pcc_mbox_channels[id]; >> } >> > > > Strange that we didn't catch this even with a non-zero id. But the > change makes sense so.. > > Acked-by: Ashwin Chaugule <ashwin.chaugule@linaro.org> Can you please include this patch in your pull request to Linus? Thanks, Ashwin. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] mailbox: pcc: fix channel calculation in get_pcc_channel() 2016-01-15 18:20 ` Ashwin Chaugule @ 2016-01-15 18:22 ` Ashwin Chaugule 2016-02-01 15:19 ` Alexey Klimov 0 siblings, 1 reply; 9+ messages in thread From: Ashwin Chaugule @ 2016-01-15 18:22 UTC (permalink / raw) To: Alexey Klimov, Jaswinder Singh; +Cc: Jassi Brar, Sudeep Holla, lkml + Jassi (Linaro addr) On 15 January 2016 at 13:20, Ashwin Chaugule <ashwin.chaugule@linaro.org> wrote: > Jassi, > > On 10 December 2015 at 13:19, Ashwin Chaugule > <ashwin.chaugule@linaro.org> wrote: >> On 10 December 2015 at 12:28, Alexey Klimov <alexey.klimov@arm.com> wrote: >>> This patch fixes the calculation of pcc_chan for non-zero id. >>> After the compiler ignores the (unsigned long) cast the >>> pcc_mbox_channels pointer is type-cast and then the type-cast >>> offset is added which results in address outside of the range >>> leading to the kernel crashing. >>> >>> We might add braces and make it: >>> >>> pcc_chan = (struct mbox_chan *) >>> ((unsigned long) pcc_mbox_channels + >>> (id * sizeof(*pcc_chan))); >>> >>> but let's go with array approach here and use id as index. >>> >>> Tested on Juno board. >>> >>> Acked-by: Sudeep Holla <sudeep.holla@arm.com> >>> Signed-off-by: Alexey Klimov <alexey.klimov@arm.com> >>> --- >>> drivers/mailbox/pcc.c | 8 +------- >>> 1 file changed, 1 insertion(+), 7 deletions(-) >>> >>> diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c >>> index 45d85ae..8f779a1 100644 >>> --- a/drivers/mailbox/pcc.c >>> +++ b/drivers/mailbox/pcc.c >>> @@ -81,16 +81,10 @@ static struct mbox_controller pcc_mbox_ctrl = {}; >>> */ >>> static struct mbox_chan *get_pcc_channel(int id) >>> { >>> - struct mbox_chan *pcc_chan; >>> - >>> if (id < 0 || id > pcc_mbox_ctrl.num_chans) >>> return ERR_PTR(-ENOENT); >>> >>> - pcc_chan = (struct mbox_chan *) >>> - (unsigned long) pcc_mbox_channels + >>> - (id * sizeof(*pcc_chan)); >>> - >>> - return pcc_chan; >>> + return &pcc_mbox_channels[id]; >>> } >>> >> >> >> Strange that we didn't catch this even with a non-zero id. But the >> change makes sense so.. >> >> Acked-by: Ashwin Chaugule <ashwin.chaugule@linaro.org> > > Can you please include this patch in your pull request to Linus? > > Thanks, > Ashwin. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] mailbox: pcc: fix channel calculation in get_pcc_channel() 2016-01-15 18:22 ` Ashwin Chaugule @ 2016-02-01 15:19 ` Alexey Klimov 2016-02-01 15:43 ` Jassi Brar 0 siblings, 1 reply; 9+ messages in thread From: Alexey Klimov @ 2016-02-01 15:19 UTC (permalink / raw) To: rjw, linux-acpi Cc: ashwin.chaugule, jaswinder.singh, jassisinghbrar, sudeep.holla, linux-kernel, Alexey Klimov (adding Rafael and linux-acpi) On Fri, Jan 15, 2016 at 6:22 PM, Ashwin Chaugule <ashwin.chaugule@linaro.org> wrote: > + Jassi (Linaro addr) > > On 15 January 2016 at 13:20, Ashwin Chaugule <ashwin.chaugule@linaro.org> wrote: >> Jassi, >> >> On 10 December 2015 at 13:19, Ashwin Chaugule >> <ashwin.chaugule@linaro.org> wrote: >>> On 10 December 2015 at 12:28, Alexey Klimov <alexey.klimov@arm.com> wrote: >>>> This patch fixes the calculation of pcc_chan for non-zero id. >>>> After the compiler ignores the (unsigned long) cast the >>>> pcc_mbox_channels pointer is type-cast and then the type-cast >>>> offset is added which results in address outside of the range >>>> leading to the kernel crashing. >>>> >>>> We might add braces and make it: >>>> >>>> pcc_chan = (struct mbox_chan *) >>>> ((unsigned long) pcc_mbox_channels + >>>> (id * sizeof(*pcc_chan))); >>>> >>>> but let's go with array approach here and use id as index. >>>> >>>> Tested on Juno board. >>>> >>>> Acked-by: Sudeep Holla <sudeep.holla@arm.com> >>>> Signed-off-by: Alexey Klimov <alexey.klimov@arm.com> >>>> --- >>>> drivers/mailbox/pcc.c | 8 +------- >>>> 1 file changed, 1 insertion(+), 7 deletions(-) >>>> >>>> diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c >>>> index 45d85ae..8f779a1 100644 >>>> --- a/drivers/mailbox/pcc.c >>>> +++ b/drivers/mailbox/pcc.c >>>> @@ -81,16 +81,10 @@ static struct mbox_controller pcc_mbox_ctrl = {}; >>>> */ >>>> static struct mbox_chan *get_pcc_channel(int id) >>>> { >>>> - struct mbox_chan *pcc_chan; >>>> - >>>> if (id < 0 || id > pcc_mbox_ctrl.num_chans) >>>> return ERR_PTR(-ENOENT); >>>> >>>> - pcc_chan = (struct mbox_chan *) >>>> - (unsigned long) pcc_mbox_channels + >>>> - (id * sizeof(*pcc_chan)); >>>> - >>>> - return pcc_chan; >>>> + return &pcc_mbox_channels[id]; >>>> } >>>> >>> >>> >>> Strange that we didn't catch this even with a non-zero id. But the >>> change makes sense so.. >>> >>> Acked-by: Ashwin Chaugule <ashwin.chaugule@linaro.org> >> >> Can you please include this patch in your pull request to Linus? Hi Rafael, any chance you can take this fix via your tree? I can resend patch if you want. Looks like Jassi doesn't have time or doesn't care. Best regards, Alexey ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] mailbox: pcc: fix channel calculation in get_pcc_channel() 2016-02-01 15:19 ` Alexey Klimov @ 2016-02-01 15:43 ` Jassi Brar 2016-02-01 20:51 ` Rafael J. Wysocki 0 siblings, 1 reply; 9+ messages in thread From: Jassi Brar @ 2016-02-01 15:43 UTC (permalink / raw) To: Alexey Klimov Cc: Rafael J. Wysocki, linux-acpi, Ashwin Chaugule, Jaswinder Singh, Sudeep Holla, Linux Kernel Mailing List On Mon, Feb 1, 2016 at 8:49 PM, Alexey Klimov <alexey.klimov@arm.com> wrote: > (adding Rafael and linux-acpi) > > On Fri, Jan 15, 2016 at 6:22 PM, Ashwin Chaugule <ashwin.chaugule@linaro.org> wrote: >> + Jassi (Linaro addr) >> >> On 15 January 2016 at 13:20, Ashwin Chaugule <ashwin.chaugule@linaro.org> wrote: >>> Jassi, >>> >>> On 10 December 2015 at 13:19, Ashwin Chaugule >>> <ashwin.chaugule@linaro.org> wrote: >>>> On 10 December 2015 at 12:28, Alexey Klimov <alexey.klimov@arm.com> wrote: >>>>> This patch fixes the calculation of pcc_chan for non-zero id. >>>>> After the compiler ignores the (unsigned long) cast the >>>>> pcc_mbox_channels pointer is type-cast and then the type-cast >>>>> offset is added which results in address outside of the range >>>>> leading to the kernel crashing. >>>>> >>>>> We might add braces and make it: >>>>> >>>>> pcc_chan = (struct mbox_chan *) >>>>> ((unsigned long) pcc_mbox_channels + >>>>> (id * sizeof(*pcc_chan))); >>>>> >>>>> but let's go with array approach here and use id as index. >>>>> >>>>> Tested on Juno board. >>>>> >>>>> Acked-by: Sudeep Holla <sudeep.holla@arm.com> >>>>> Signed-off-by: Alexey Klimov <alexey.klimov@arm.com> >>>>> --- >>>>> drivers/mailbox/pcc.c | 8 +------- >>>>> 1 file changed, 1 insertion(+), 7 deletions(-) >>>>> >>>>> diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c >>>>> index 45d85ae..8f779a1 100644 >>>>> --- a/drivers/mailbox/pcc.c >>>>> +++ b/drivers/mailbox/pcc.c >>>>> @@ -81,16 +81,10 @@ static struct mbox_controller pcc_mbox_ctrl = {}; >>>>> */ >>>>> static struct mbox_chan *get_pcc_channel(int id) >>>>> { >>>>> - struct mbox_chan *pcc_chan; >>>>> - >>>>> if (id < 0 || id > pcc_mbox_ctrl.num_chans) >>>>> return ERR_PTR(-ENOENT); >>>>> >>>>> - pcc_chan = (struct mbox_chan *) >>>>> - (unsigned long) pcc_mbox_channels + >>>>> - (id * sizeof(*pcc_chan)); >>>>> - >>>>> - return pcc_chan; >>>>> + return &pcc_mbox_channels[id]; >>>>> } >>>>> >>>> >>>> >>>> Strange that we didn't catch this even with a non-zero id. But the >>>> change makes sense so.. >>>> >>>> Acked-by: Ashwin Chaugule <ashwin.chaugule@linaro.org> >>> >>> Can you please include this patch in your pull request to Linus? > > Hi Rafael, > > any chance you can take this fix via your tree? > I can resend patch if you want. > Looks like Jassi doesn't have time or doesn't care. > Ouch... sorry. Ashwin's reminder came after I had sent the pull request. And then it just got overlooked. If Rafael doesn't pick it as my punishment, I'll tomorow :) Cheers! ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] mailbox: pcc: fix channel calculation in get_pcc_channel() 2016-02-01 15:43 ` Jassi Brar @ 2016-02-01 20:51 ` Rafael J. Wysocki 2016-02-02 11:12 ` Jassi Brar 0 siblings, 1 reply; 9+ messages in thread From: Rafael J. Wysocki @ 2016-02-01 20:51 UTC (permalink / raw) To: Jassi Brar, Alexey Klimov Cc: Rafael J. Wysocki, ACPI Devel Maling List, Ashwin Chaugule, Jaswinder Singh, Sudeep Holla, Linux Kernel Mailing List On Mon, Feb 1, 2016 at 4:43 PM, Jassi Brar <jassisinghbrar@gmail.com> wrote: > On Mon, Feb 1, 2016 at 8:49 PM, Alexey Klimov <alexey.klimov@arm.com> wrote: >> (adding Rafael and linux-acpi) >> >> On Fri, Jan 15, 2016 at 6:22 PM, Ashwin Chaugule <ashwin.chaugule@linaro.org> wrote: >>> + Jassi (Linaro addr) >>> >>> On 15 January 2016 at 13:20, Ashwin Chaugule <ashwin.chaugule@linaro.org> wrote: [cut] >>>> Can you please include this patch in your pull request to Linus? >> >> Hi Rafael, >> >> any chance you can take this fix via your tree? >> I can resend patch if you want. >> Looks like Jassi doesn't have time or doesn't care. >> > Ouch... sorry. Ashwin's reminder came after I had sent the pull > request. And then it just got overlooked. If Rafael doesn't pick it as > my punishment, I'll tomorow :) I can apply it, but can anyone please point me to the original patch? Or, maybe better, resend it with a CC to linux-acpi and all of the tags collected so far? Thanks, Rafael ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] mailbox: pcc: fix channel calculation in get_pcc_channel() 2016-02-01 20:51 ` Rafael J. Wysocki @ 2016-02-02 11:12 ` Jassi Brar 2016-02-03 1:22 ` Rafael J. Wysocki 0 siblings, 1 reply; 9+ messages in thread From: Jassi Brar @ 2016-02-02 11:12 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Alexey Klimov, Rafael J. Wysocki, ACPI Devel Maling List, Ashwin Chaugule, Jaswinder Singh, Sudeep Holla, Linux Kernel Mailing List On Tue, Feb 2, 2016 at 2:21 AM, Rafael J. Wysocki <rafael@kernel.org> wrote: > On Mon, Feb 1, 2016 at 4:43 PM, Jassi Brar <jassisinghbrar@gmail.com> wrote: >> On Mon, Feb 1, 2016 at 8:49 PM, Alexey Klimov <alexey.klimov@arm.com> wrote: >>> (adding Rafael and linux-acpi) >>> >>> On Fri, Jan 15, 2016 at 6:22 PM, Ashwin Chaugule <ashwin.chaugule@linaro.org> wrote: >>>> + Jassi (Linaro addr) >>>> >>>> On 15 January 2016 at 13:20, Ashwin Chaugule <ashwin.chaugule@linaro.org> wrote: > > [cut] > >>>>> Can you please include this patch in your pull request to Linus? >>> >>> Hi Rafael, >>> >>> any chance you can take this fix via your tree? >>> I can resend patch if you want. >>> Looks like Jassi doesn't have time or doesn't care. >>> >> Ouch... sorry. Ashwin's reminder came after I had sent the pull >> request. And then it just got overlooked. If Rafael doesn't pick it as >> my punishment, I'll tomorow :) > > I can apply it, but can anyone please point me to the original patch? > > Or, maybe better, resend it with a CC to linux-acpi and all of the > tags collected so far? > or even simpler ... I queued it already https://git.linaro.org/landing-teams/working/fujitsu/integration.git/log/refs/heads/mailbox-devel ? Thanks. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] mailbox: pcc: fix channel calculation in get_pcc_channel() 2016-02-02 11:12 ` Jassi Brar @ 2016-02-03 1:22 ` Rafael J. Wysocki 0 siblings, 0 replies; 9+ messages in thread From: Rafael J. Wysocki @ 2016-02-03 1:22 UTC (permalink / raw) To: Jassi Brar Cc: Rafael J. Wysocki, Alexey Klimov, ACPI Devel Maling List, Ashwin Chaugule, Jaswinder Singh, Sudeep Holla, Linux Kernel Mailing List On Tuesday, February 02, 2016 04:42:52 PM Jassi Brar wrote: > On Tue, Feb 2, 2016 at 2:21 AM, Rafael J. Wysocki <rafael@kernel.org> wrote: > > On Mon, Feb 1, 2016 at 4:43 PM, Jassi Brar <jassisinghbrar@gmail.com> wrote: > >> On Mon, Feb 1, 2016 at 8:49 PM, Alexey Klimov <alexey.klimov@arm.com> wrote: > >>> (adding Rafael and linux-acpi) > >>> > >>> On Fri, Jan 15, 2016 at 6:22 PM, Ashwin Chaugule <ashwin.chaugule@linaro.org> wrote: > >>>> + Jassi (Linaro addr) > >>>> > >>>> On 15 January 2016 at 13:20, Ashwin Chaugule <ashwin.chaugule@linaro.org> wrote: > > > > [cut] > > > >>>>> Can you please include this patch in your pull request to Linus? > >>> > >>> Hi Rafael, > >>> > >>> any chance you can take this fix via your tree? > >>> I can resend patch if you want. > >>> Looks like Jassi doesn't have time or doesn't care. > >>> > >> Ouch... sorry. Ashwin's reminder came after I had sent the pull > >> request. And then it just got overlooked. If Rafael doesn't pick it as > >> my punishment, I'll tomorow :) > > > > I can apply it, but can anyone please point me to the original patch? > > > > Or, maybe better, resend it with a CC to linux-acpi and all of the > > tags collected so far? > > > or even simpler ... I queued it already > https://git.linaro.org/landing-teams/working/fujitsu/integration.git/log/refs/heads/mailbox-devel > ? That works for me too. :-) ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2016-02-03 1:21 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-12-10 17:28 [PATCH] mailbox: pcc: fix channel calculation in get_pcc_channel() Alexey Klimov 2015-12-10 18:19 ` Ashwin Chaugule 2016-01-15 18:20 ` Ashwin Chaugule 2016-01-15 18:22 ` Ashwin Chaugule 2016-02-01 15:19 ` Alexey Klimov 2016-02-01 15:43 ` Jassi Brar 2016-02-01 20:51 ` Rafael J. Wysocki 2016-02-02 11:12 ` Jassi Brar 2016-02-03 1:22 ` Rafael J. Wysocki
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®