Re:[PATCH] x86/mm/mem_encrypt_identity : fix error useage to sizeof

Re:[PATCH] x86/mm/mem_encrypt_identity : fix error useage to sizeof

[Thomas Gleixner]

On Mon, 7 Jan 2019, peng.hao2@zte.com.cn wrote:

> >> Fix error usage to sizeof. It should not use sizeof to pointer.
> >
> >.... because?
> >
> >The commit message needs to explain what the potential issue could be
> >and why it doesn't matter in this case.
> I see the definition of pte_t may be more than sizeof(unsigned long).
> So I think sizeof(pte_t) is safer.

What exactly is the difference between:

	pte_t	*p;

	sizeof(*p)

and

	sizeof(pte_t)

and what is safer about the latter?

Answer: No difference and nothing is safer because it's exactly the same.

In general we use sizeof(*p) simply because when the data type of p changes
you don't have to update the code, it just works and stays correct.

Thanks,

	tglx

Re: [PATCH] x86/mm/mem_encrypt_identity : fix error useage to sizeof

[Juergen Gross]

On 15/01/2019 11:13, Thomas Gleixner wrote:
> On Mon, 7 Jan 2019, peng.hao2@zte.com.cn wrote:
> 
>>>> Fix error usage to sizeof. It should not use sizeof to pointer.
>>>
>>> .... because?
>>>
>>> The commit message needs to explain what the potential issue could be
>>> and why it doesn't matter in this case.
>> I see the definition of pte_t may be more than sizeof(unsigned long).
>> So I think sizeof(pte_t) is safer.
> 
> What exactly is the difference between:
> 
> 	pte_t	*p;
> 
> 	sizeof(*p)
> 
> and
> 
> 	sizeof(pte_t)
> 
> and what is safer about the latter?

Please note that the current code is using sizeof(p) instead of sizeof(*p).

And this is really different for X86_32 PAE.


Juergen

> 
> Answer: No difference and nothing is safer because it's exactly the same.
> 
> In general we use sizeof(*p) simply because when the data type of p changes
> you don't have to update the code, it just works and stays correct.
> 
> Thanks,
> 
> 	tglx
> 

Re: [PATCH] x86/mm/mem_encrypt_identity : fix error useage to sizeof

[Thomas Gleixner]

On Tue, 15 Jan 2019, Juergen Gross wrote:
> On 15/01/2019 11:13, Thomas Gleixner wrote:
> > On Mon, 7 Jan 2019, peng.hao2@zte.com.cn wrote:
> > 
> >>>> Fix error usage to sizeof. It should not use sizeof to pointer.
> >>>
> >>> .... because?
> >>>
> >>> The commit message needs to explain what the potential issue could be
> >>> and why it doesn't matter in this case.
> >> I see the definition of pte_t may be more than sizeof(unsigned long).
> >> So I think sizeof(pte_t) is safer.
> > 
> > What exactly is the difference between:
> > 
> > 	pte_t	*p;
> > 
> > 	sizeof(*p)
> > 
> > and
> > 
> > 	sizeof(pte_t)
> > 
> > and what is safer about the latter?
> 
> Please note that the current code is using sizeof(p) instead of sizeof(*p).

Ooops. That's wrong indeed, but we should not change it to sizeof(pte_t)
and change it to sizeof(*p) instead.

Thanks,

	tglx

Re: [PATCH] x86/mm/mem_encrypt_identity : fix error useage to sizeof

[Juergen Gross]

On 15/01/2019 11:25, Thomas Gleixner wrote:
> On Tue, 15 Jan 2019, Juergen Gross wrote:
>> On 15/01/2019 11:13, Thomas Gleixner wrote:
>>> On Mon, 7 Jan 2019, peng.hao2@zte.com.cn wrote:
>>>
>>>>>> Fix error usage to sizeof. It should not use sizeof to pointer.
>>>>>
>>>>> .... because?
>>>>>
>>>>> The commit message needs to explain what the potential issue could be
>>>>> and why it doesn't matter in this case.
>>>> I see the definition of pte_t may be more than sizeof(unsigned long).
>>>> So I think sizeof(pte_t) is safer.
>>>
>>> What exactly is the difference between:
>>>
>>> 	pte_t	*p;
>>>
>>> 	sizeof(*p)
>>>
>>> and
>>>
>>> 	sizeof(pte_t)
>>>
>>> and what is safer about the latter?
>>
>> Please note that the current code is using sizeof(p) instead of sizeof(*p).
> 
> Ooops. That's wrong indeed, but we should not change it to sizeof(pte_t)
> and change it to sizeof(*p) instead.

And that's what the patch does.


Juergen

Re: [PATCH] x86/mm/mem_encrypt_identity : fix error useage to sizeof

[Thomas Gleixner]

On Tue, 15 Jan 2019, Thomas Gleixner wrote:

> On Tue, 15 Jan 2019, Juergen Gross wrote:
> > On 15/01/2019 11:13, Thomas Gleixner wrote:
> > > On Mon, 7 Jan 2019, peng.hao2@zte.com.cn wrote:
> > > 
> > >>>> Fix error usage to sizeof. It should not use sizeof to pointer.
> > >>>
> > >>> .... because?
> > >>>
> > >>> The commit message needs to explain what the potential issue could be
> > >>> and why it doesn't matter in this case.
> > >> I see the definition of pte_t may be more than sizeof(unsigned long).
> > >> So I think sizeof(pte_t) is safer.
> > > 
> > > What exactly is the difference between:
> > > 
> > > 	pte_t	*p;
> > > 
> > > 	sizeof(*p)
> > > 
> > > and
> > > 
> > > 	sizeof(pte_t)
> > > 
> > > and what is safer about the latter?
> > 
> > Please note that the current code is using sizeof(p) instead of sizeof(*p).
> 
> Ooops. That's wrong indeed, but we should not change it to sizeof(pte_t)
> and change it to sizeof(*p) instead.

Which is what the patch actually does. Just the above reply:

> > >> So I think sizeof(pte_t) is safer.

confused the hell out of me. -ENOTENOUGHCOFFEE

Re: [PATCH] x86/mm/mem_encrypt_identity : fix error useage to sizeof

[Borislav Petkov]

On Sat, Dec 29, 2018 at 02:34:12PM +0800, Peng Hao wrote:
> Fix error usage to sizeof. It should not use sizeof to pointer.

... because?

The commit message needs to explain what the potential issue could be
and why it doesn't matter in this case.

Thx.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

[PATCH] x86/mm/mem_encrypt_identity : fix error useage to sizeof

[Peng Hao]

Fix error usage to sizeof. It should not use sizeof to pointer.

Signed-off-by: Peng Hao <peng.hao2@zte.com.cn>
---
 arch/x86/mm/mem_encrypt_identity.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/mm/mem_encrypt_identity.c b/arch/x86/mm/mem_encrypt_identity.c
index a19ef1a..4aa9b14 100644
--- a/arch/x86/mm/mem_encrypt_identity.c
+++ b/arch/x86/mm/mem_encrypt_identity.c
@@ -158,8 +158,8 @@ static void __init sme_populate_pgd(struct sme_populate_pgd_data *ppd)
 	pmd = pmd_offset(pud, ppd->vaddr);
 	if (pmd_none(*pmd)) {
 		pte = ppd->pgtable_area;
-		memset(pte, 0, sizeof(pte) * PTRS_PER_PTE);
-		ppd->pgtable_area += sizeof(pte) * PTRS_PER_PTE;
+		memset(pte, 0, sizeof(*pte) * PTRS_PER_PTE);
+		ppd->pgtable_area += sizeof(*pte) * PTRS_PER_PTE;
 		set_pmd(pmd, __pmd(PMD_FLAGS | __pa(pte)));
 	}
 
-- 
1.8.3.1