[PATCH v2] xen/arm: Avoid overflow using MIDR_IMPLEMENTOR_MASK

[PATCH v2] xen/arm: Avoid overflow using MIDR_IMPLEMENTOR_MASK

[Michal Orzel]

Value of macro MIDR_IMPLEMENTOR_MASK exceeds the range of integer
and can lead to overflow. Currently there is no issue as it is used
in an expression implicitly casted to u32 in MIDR_IS_CPU_MODEL_RANGE.
To avoid possible problems, fix the macro.

Signed-off-by: Michal Orzel <michal.orzel@arm.com>
Link: https://lore.kernel.org/r/20220426070603.56031-1-michal.orzel@arm.com
Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
Origin: git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git (48e6f22e25a4)
---
Changes since v1:
- add Origin tag as the patch was merged in upstream arm64 linux tree
---
 xen/arch/arm/include/asm/processor.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/arch/arm/include/asm/processor.h b/xen/arch/arm/include/asm/processor.h
index 852b5f3c24..7a1c4c4410 100644
--- a/xen/arch/arm/include/asm/processor.h
+++ b/xen/arch/arm/include/asm/processor.h
@@ -39,7 +39,7 @@
 #define MIDR_VARIANT(midr) \
     (((midr) & MIDR_VARIANT_MASK) >> MIDR_VARIANT_SHIFT)
 #define MIDR_IMPLEMENTOR_SHIFT  24
-#define MIDR_IMPLEMENTOR_MASK   (0xff << MIDR_IMPLEMENTOR_SHIFT)
+#define MIDR_IMPLEMENTOR_MASK   (0xffU << MIDR_IMPLEMENTOR_SHIFT)
 #define MIDR_IMPLEMENTOR(midr) \
     (((midr) & MIDR_IMPLEMENTOR_MASK) >> MIDR_IMPLEMENTOR_SHIFT)
 
-- 
2.25.1

Re: [PATCH v2] xen/arm: Avoid overflow using MIDR_IMPLEMENTOR_MASK

[Catalin Marinas]

On Thu, May 05, 2022 at 01:59:06PM +0200, Michal Orzel wrote:
> Value of macro MIDR_IMPLEMENTOR_MASK exceeds the range of integer
> and can lead to overflow. Currently there is no issue as it is used
> in an expression implicitly casted to u32 in MIDR_IS_CPU_MODEL_RANGE.
> To avoid possible problems, fix the macro.
> 
> Signed-off-by: Michal Orzel <michal.orzel@arm.com>
> Link: https://lore.kernel.org/r/20220426070603.56031-1-michal.orzel@arm.com
> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> Origin: git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git (48e6f22e25a4)
> ---
> Changes since v1:
> - add Origin tag as the patch was merged in upstream arm64 linux tree

Note that there's always a risk that the commit Id will be changed
before it hits mainline (Linus' tree).

-- 
Catalin

Re: [PATCH v2] xen/arm: Avoid overflow using MIDR_IMPLEMENTOR_MASK

[Michal Orzel]

Hi Catalin,

On 05.05.2022 14:13, Catalin Marinas wrote:
> On Thu, May 05, 2022 at 01:59:06PM +0200, Michal Orzel wrote:
>> Value of macro MIDR_IMPLEMENTOR_MASK exceeds the range of integer
>> and can lead to overflow. Currently there is no issue as it is used
>> in an expression implicitly casted to u32 in MIDR_IS_CPU_MODEL_RANGE.
>> To avoid possible problems, fix the macro.
>>
>> Signed-off-by: Michal Orzel <michal.orzel@arm.com>
>> Link: https://lore.kernel.org/r/20220426070603.56031-1-michal.orzel@arm.com
>> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
>> Origin: git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git (48e6f22e25a4)
>> ---
>> Changes since v1:
>> - add Origin tag as the patch was merged in upstream arm64 linux tree
> 
> Note that there's always a risk that the commit Id will be changed
> before it hits mainline (Linus' tree).
> 

This commit is now in linux-next:
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=48e6f22e25a44e43952db5fbb767dea0c9319cb2
so we can be sure that the SHA will stay unmodified there (and will be the same in Linus' tree).

Question to maintainers:
Do you want me to update Origin to point to linux-next?

Cheers,
Michal

Re: [PATCH v2] xen/arm: Avoid overflow using MIDR_IMPLEMENTOR_MASK

[Julien Grall]

Hi,

On 10/05/2022 07:49, Michal Orzel wrote:
> On 05.05.2022 14:13, Catalin Marinas wrote:
>> On Thu, May 05, 2022 at 01:59:06PM +0200, Michal Orzel wrote:
>>> Value of macro MIDR_IMPLEMENTOR_MASK exceeds the range of integer
>>> and can lead to overflow. Currently there is no issue as it is used
>>> in an expression implicitly casted to u32 in MIDR_IS_CPU_MODEL_RANGE.
>>> To avoid possible problems, fix the macro.
>>>
>>> Signed-off-by: Michal Orzel <michal.orzel@arm.com>
>>> Link: https://lore.kernel.org/r/20220426070603.56031-1-michal.orzel@arm.com
>>> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
>>> Origin: git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git (48e6f22e25a4)
>>> ---
>>> Changes since v1:
>>> - add Origin tag as the patch was merged in upstream arm64 linux tree
>>
>> Note that there's always a risk that the commit Id will be changed
>> before it hits mainline (Linus' tree).
>>
> 
> This commit is now in linux-next:
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=48e6f22e25a44e43952db5fbb767dea0c9319cb2
> so we can be sure that the SHA will stay unmodified there (and will be the same in Linus' tree).

AFAIK, linux-next branch is just a merge of all the maintainers branch 
and often rewritten. So there are no guarantee a commit id is valid 
until it reached Linus' tree.

> 
> Question to maintainers:
> Do you want me to update Origin to point to linux-next?
So we have a link to the patch and a name. This should be sufficient to 
find the commit.

Therefore, I would simply on top of Origin:

[The commit ID may be different in linus' tree]

-- 
Julien Grall

Re: [PATCH v2] xen/arm: Avoid overflow using MIDR_IMPLEMENTOR_MASK

[Michal Orzel]

Hi Julien,

On 10.05.2022 10:27, Julien Grall wrote:
> 
> Therefore, I would simply on top of Origin:
> 
> [The commit ID may be different in linus' tree]
> 
Could you please do that on commit as this is just one line of commit change?

Thanks,
Michal

Re: [PATCH v2] xen/arm: Avoid overflow using MIDR_IMPLEMENTOR_MASK

[Catalin Marinas]

On Tue, May 10, 2022 at 09:27:29AM +0100, Julien Grall wrote:
> Hi,
> 
> On 10/05/2022 07:49, Michal Orzel wrote:
> > On 05.05.2022 14:13, Catalin Marinas wrote:
> > > On Thu, May 05, 2022 at 01:59:06PM +0200, Michal Orzel wrote:
> > > > Value of macro MIDR_IMPLEMENTOR_MASK exceeds the range of integer
> > > > and can lead to overflow. Currently there is no issue as it is used
> > > > in an expression implicitly casted to u32 in MIDR_IS_CPU_MODEL_RANGE.
> > > > To avoid possible problems, fix the macro.
> > > > 
> > > > Signed-off-by: Michal Orzel <michal.orzel@arm.com>
> > > > Link: https://lore.kernel.org/r/20220426070603.56031-1-michal.orzel@arm.com
> > > > Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> > > > Origin: git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git (48e6f22e25a4)
> > > > ---
> > > > Changes since v1:
> > > > - add Origin tag as the patch was merged in upstream arm64 linux tree
> > > 
> > > Note that there's always a risk that the commit Id will be changed
> > > before it hits mainline (Linus' tree).
> > > 
> > 
> > This commit is now in linux-next:
> > https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=48e6f22e25a44e43952db5fbb767dea0c9319cb2
> > so we can be sure that the SHA will stay unmodified there (and will be the same in Linus' tree).
> 
> AFAIK, linux-next branch is just a merge of all the maintainers branch and
> often rewritten. So there are no guarantee a commit id is valid until it
> reached Linus' tree.

Indeed. While I try not to rebase it, it may happen occasionally.

> > Question to maintainers:
> > Do you want me to update Origin to point to linux-next?
> 
> So we have a link to the patch and a name. This should be sufficient to find
> the commit.
> 
> Therefore, I would simply on top of Origin:

Yeah, just keep the link to the mailing list. I guess you can drop my
Singed-off-by as well if it appears to be picked from the list rather
than the kernel repo. If you want an ack:

Acked-by: Catalin Marinas <catalin.marinas@arm.com>

-- 
Catalin

Re: [PATCH v2] xen/arm: Avoid overflow using MIDR_IMPLEMENTOR_MASK

[Julien Grall]

Hi Catalin,

On 10/05/2022 09:55, Catalin Marinas wrote:
> On Tue, May 10, 2022 at 09:27:29AM +0100, Julien Grall wrote:
>> Hi,
>>
>> On 10/05/2022 07:49, Michal Orzel wrote:
>>> On 05.05.2022 14:13, Catalin Marinas wrote:
>>>> On Thu, May 05, 2022 at 01:59:06PM +0200, Michal Orzel wrote:
>>>>> Value of macro MIDR_IMPLEMENTOR_MASK exceeds the range of integer
>>>>> and can lead to overflow. Currently there is no issue as it is used
>>>>> in an expression implicitly casted to u32 in MIDR_IS_CPU_MODEL_RANGE.
>>>>> To avoid possible problems, fix the macro.
>>>>>
>>>>> Signed-off-by: Michal Orzel <michal.orzel@arm.com>
>>>>> Link: https://lore.kernel.org/r/20220426070603.56031-1-michal.orzel@arm.com
>>>>> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
>>>>> Origin: git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git (48e6f22e25a4)
>>>>> ---
>>>>> Changes since v1:
>>>>> - add Origin tag as the patch was merged in upstream arm64 linux tree
>>>>
>>>> Note that there's always a risk that the commit Id will be changed
>>>> before it hits mainline (Linus' tree).
>>>>
>>>
>>> This commit is now in linux-next:
>>> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=48e6f22e25a44e43952db5fbb767dea0c9319cb2
>>> so we can be sure that the SHA will stay unmodified there (and will be the same in Linus' tree).
>>
>> AFAIK, linux-next branch is just a merge of all the maintainers branch and
>> often rewritten. So there are no guarantee a commit id is valid until it
>> reached Linus' tree.
> 
> Indeed. While I try not to rebase it, it may happen occasionally.
> 
>>> Question to maintainers:
>>> Do you want me to update Origin to point to linux-next?
>>
>> So we have a link to the patch and a name. This should be sufficient to find
>> the commit.
>>
>> Therefore, I would simply on top of Origin:
> 
> Yeah, just keep the link to the mailing list. I guess you can drop my
> Singed-off-by as well if it appears to be picked from the list rather
> than the kernel repo. If you want an ack:
> 
> Acked-by: Catalin Marinas <catalin.marinas@arm.com>

I have done what you suggested and committed. Thanks!

Cheers,

-- 
Julien Grall