linux-kernel - Re: ERST: how to avoid a dynamic memory allocation in panic case

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1358901594.7408.85.camel@yhuang-dev>
Date:	Wed, 23 Jan 2013 08:39:54 +0800
From:	Huang Ying <ying.huang@...el.com>
To:	Seiji Aguchi <seiji.aguchi@....com>
Cc:	"rjw@...k.pl" <rjw@...k.pl>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-acpi@...r.kernel.org" <linux-acpi@...r.kernel.org>,
	"H. Peter Anvin (hpa@...or.com)" <hpa@...or.com>,
	"robert.moore@...el.com" <robert.moore@...el.com>,
	"trenn@...e.de" <trenn@...e.de>,
	"myron.stowe@...hat.com" <myron.stowe@...hat.com>
Subject: Re: ERST: how to avoid a dynamic memory allocation in panic case

On Tue, 2013-01-22 at 23:30 +0000, Seiji Aguchi wrote:
> [Issue]
> 
> Current erst driver is kicked in panic case.
> On the other hand, a dynamic memory allocation seems to run in it
> with a following code path.
> 
> erst_writer -> erst_write -> __erst_write_to_storage -> many of apei_exec_run -> 
> ctx->ins_table[entry->instruction].run()
> 
> which are functions defined in erst_ins_type table i.e:
> 
> apei_exec_read_register -> apei_read -> acpi_os_read_memory64 -> acpi_os_ioremap -> ioremap_cache
> 
> which possibly involve IOMMU allocator.
> 
> I may cause a failure of  an erst driver if the panic happens in an interrupt context.
> 
> [Idea]
> 
> If we can remove ioremap_cache() from acpi_os_read_memory() as follows,
> It is easy to fix this issue.
> But I'm not sure if it is feasible because I can't see the reason of tying acpi_os_ioremap()  from a git log...
> 
> Any comment?
> 
> <snip>
> @@ -918,10 +918,7 @@ acpi_os_read_memory(acpi_physical_address phys_addr, u64 *value, u32 width)
>  	virt_addr = acpi_map_vaddr_lookup(phys_addr, size);
>  	if (!virt_addr) {
>  		rcu_read_unlock();
> -		virt_addr = acpi_os_ioremap(phys_addr, size);
> -		if (!virt_addr)
> -			return AE_BAD_ADDRESS;
> -		unmap = true;
> +		return AE_BAD_ADDRESS;

No.  We can not do that.  Because some users rely on acpi_os_read_memory
to do ioremap for them.

The correct fixing should be pre-map the io-memory that may be accessed
in erst code patch with acpi_map().

Best Regards,
Huang Ying

>  	}
>  
>  	if (!value)
> <snip>
> 
> Seiji


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/