* Re: xen/mmu: Copy and revector the P2M tree.
@ 2015-07-20 10:03 Dan Carpenter
2015-08-07 0:23 ` Dan Carpenter
0 siblings, 1 reply; 2+ messages in thread
From: Dan Carpenter @ 2015-07-20 10:03 UTC (permalink / raw)
To: konrad.wilk; +Cc: xen-devel
Hello Konrad Rzeszutek Wilk,
The patch 7f9140626c75: "xen/mmu: Copy and revector the P2M tree."
from Jul 26, 2012, leads to the following static checker warning:
arch/x86/xen/mmu.c:1105 xen_cleanhighmap()
warn: potential pointer math issue ('level2_kernel_pgt' is pointer to unsigned long)
arch/x86/xen/mmu.c
1096 #ifdef CONFIG_X86_64
1097 static void __init xen_cleanhighmap(unsigned long vaddr,
1098 unsigned long vaddr_end)
1099 {
1100 unsigned long kernel_end = roundup((unsigned long)_brk_end, PMD_SIZE) - 1;
1101 pmd_t *pmd = level2_kernel_pgt + pmd_index(vaddr);
1102
1103 /* NOTE: The loop is more greedy than the cleanup_highmap variant.
1104 * We include the PMD passed in on _both_ boundaries. */
1105 for (; vaddr <= vaddr_end && (pmd < (level2_kernel_pgt + PAGE_SIZE));
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
This pointer math is weird because we typically think of PAGE_SIZE as
a number of bytes but since level2_kernel_pgt is a pointer to unsigned
long, it looks like this loop can go through more iterations than
intended.
1106 pmd++, vaddr += PMD_SIZE) {
1107 if (pmd_none(*pmd))
1108 continue;
1109 if (vaddr < (unsigned long) _text || vaddr > kernel_end)
1110 set_pmd(pmd, __pmd(0));
1111 }
1112 /* In case we did something silly, we should crash in this function
1113 * instead of somewhere later and be confusing. */
1114 xen_mc_flush();
1115 }
regards,
dan carpenter
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: xen/mmu: Copy and revector the P2M tree.
2015-07-20 10:03 xen/mmu: Copy and revector the P2M tree Dan Carpenter
@ 2015-08-07 0:23 ` Dan Carpenter
0 siblings, 0 replies; 2+ messages in thread
From: Dan Carpenter @ 2015-08-07 0:23 UTC (permalink / raw)
To: konrad.wilk; +Cc: xen-devel
On Mon, Jul 20, 2015 at 01:03:52PM +0300, Dan Carpenter wrote:
> Hello Konrad Rzeszutek Wilk,
>
> The patch 7f9140626c75: "xen/mmu: Copy and revector the P2M tree."
> from Jul 26, 2012, leads to the following static checker warning:
>
> arch/x86/xen/mmu.c:1105 xen_cleanhighmap()
> warn: potential pointer math issue ('level2_kernel_pgt' is pointer to unsigned long)
>
> arch/x86/xen/mmu.c
> 1096 #ifdef CONFIG_X86_64
> 1097 static void __init xen_cleanhighmap(unsigned long vaddr,
> 1098 unsigned long vaddr_end)
> 1099 {
> 1100 unsigned long kernel_end = roundup((unsigned long)_brk_end, PMD_SIZE) - 1;
> 1101 pmd_t *pmd = level2_kernel_pgt + pmd_index(vaddr);
> 1102
> 1103 /* NOTE: The loop is more greedy than the cleanup_highmap variant.
> 1104 * We include the PMD passed in on _both_ boundaries. */
> 1105 for (; vaddr <= vaddr_end && (pmd < (level2_kernel_pgt + PAGE_SIZE));
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> This pointer math is weird because we typically think of PAGE_SIZE as
> a number of bytes but since level2_kernel_pgt is a pointer to unsigned
> long, it looks like this loop can go through more iterations than
> intended.
>
Yeah. This condition doesn't make sense at all. level2_kernel_pgt is
an array that has 512 elements but PAGE_SIZE is 4096 so we would be well
past the end of the array.
I suspect that this works because it is only called when DEBUG in
defined or because of the "vaddr <= vaddr_end" condition.
> 1106 pmd++, vaddr += PMD_SIZE) {
> 1107 if (pmd_none(*pmd))
> 1108 continue;
> 1109 if (vaddr < (unsigned long) _text || vaddr > kernel_end)
> 1110 set_pmd(pmd, __pmd(0));
> 1111 }
> 1112 /* In case we did something silly, we should crash in this function
> 1113 * instead of somewhere later and be confusing. */
> 1114 xen_mc_flush();
> 1115 }
>
> regards,
> dan carpenter
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2015-08-07 0:23 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-07-20 10:03 xen/mmu: Copy and revector the P2M tree Dan Carpenter
2015-08-07 0:23 ` Dan Carpenter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).