RE: [PATCH v4 7/7] ACPI / x86: introduce acpi_os_readable() support

[Zheng, Lv]

Hi,

> From: Chen, Yu C
> Sent: Tuesday, December 15, 2015 2:13 PM
> 
> Hi, Andy
> 
> > From: Andy Lutomirski [mailto:luto@xxxxxxxxxxxxxx]
> > Sent: Tuesday, December 15, 2015 7:28 AM
> >
> > On Wed, Dec 2, 2015 at 6:43 PM, Lv Zheng <lv.zheng@xxxxxxxxx> wrote:
> > > From: Chen Yu <yu.c.chen@xxxxxxxxx>
> > >
> > > This patch implements acpi_os_readable(). The function is used by
> > > ACPICA AML debugger to validate user specified pointers for dumping
> > > the memory as ACPICA descriptor objects.
> > >
> [cut]
> > >
> > > +bool __acpi_memory_readable(void *pointer, size_t length) {
> > > +       unsigned long obj_start, obj_end;
> > > +       unsigned long start_pfn, end_pfn;
> >
> > What does "readable" mean in this context?

[Lv Zheng] 
The function is used by ACPICA "dump" command.
It accepts an arbitrary address, and tries to dump the memory block specified by the address as an acpi_object.
You can try: "help dump" in the interactive mode to confirm.
While acpi_object is actually all created by kmalloc.
So we just check if the specified memory block belongs to the kernel heap.
The readable/writeable is not so meaningful here as the kernel heap should always be both readable and writeable.

We do a lot of checks in this function in order to allow it to:
1. return true if "pointer" belongs to kernel heap when "length" is 0;
2. return false if "pointer" doesn't belong to kernel heap when "length" is 0;
3. return true if "pointer ~ pointer+length-1" belongs to a kernel heap range;
4. return false if "pointer ~ pointer+length-1" doesn't belong to any kernel heap range.

These checks are weak, but can help to avoid panics if users specify wrong pointers for the "dump" command.

> 'readable' means :  the address provided by the user,
> is a dynamically allocated  virtual address -
> because  the  acpi address space are allocated by 'kmalloc',
> acpi debugger must check if this address is a valid 'kmalloc'
> address before accessing it.
> 
> This function does the sanity check that, the vitual address is a:
> 1.   dynamically allocated address (beyond PAGE_OFFSET , but lower
> than high_memory, VMALLOC_START, eg)
> 2.  besides, the physical address must be direct-mapped(so it would not be a
> hole).

[Lv Zheng] 
There is a special case (possibly hackish) on x86_64.
x86_64 kernel maps kernel image twice.
One is called as high map and the other is called as low map.

Since we use __pa() to convert a virtual address,
If the virtual address belongs to the high map range, __pa() which takes care of converting high map addresses actually returns a physical address where there should also be low map mappings ready for it.
Thus the converted PFN from the result of __pa() will be treated as valid.

But this doesn't mean there is a high map for this virtual address.
x86_64 kernel drops several pages from high map in cleanup_highmap().
So accessing a virtual address that belongs to the holes whose page mappings have been dropped in this function could still result in panic due to no mappings.
By enforcing this check, we can avoid such a case.
Actually no acpi_object's virtual address will belong to high map range.

Thanks and best regards
-Lv