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: <20170109221831.GC3800@linux.vnet.ibm.com>
Date:   Mon, 9 Jan 2017 14:18:31 -0800
From:   "Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>
To:     Borislav Petkov <bp@...en8.de>
Cc:     "Zheng, Lv" <lv.zheng@...el.com>,
        "Rafael J. Wysocki" <rafael@...nel.org>,
        "Wysocki, Rafael J" <rafael.j.wysocki@...el.com>,
        "Moore, Robert" <robert.moore@...el.com>,
        J?rg R?del <joro@...tes.org>,
        lkml <linux-kernel@...r.kernel.org>,
        Linux ACPI <linux-acpi@...r.kernel.org>
Subject: Re: 174cc7187e6f ACPICA: Tables: Back port
 acpi_get_table_with_size() and early_acpi_os_unmap_memory() from Linux
 kernel

On Mon, Jan 09, 2017 at 10:33:29AM +0100, Borislav Petkov wrote:
> + Paul for comment.
> 
> Leaving in the rest for him.
> 
> On Mon, Jan 09, 2017 at 02:36:33AM +0000, Zheng, Lv wrote:
> > Hi,
> > 
> > > From: linux-acpi-owner@...r.kernel.org [mailto:linux-acpi-owner@...r.kernel.org] On Behalf Of Zheng,
> > > Lv
> > > Subject: RE: 174cc7187e6f ACPICA: Tables: Back port acpi_get_table_with_size() and
> > > early_acpi_os_unmap_memory() from Linux kernel
> > > 
> > > Hi,
> > > 
> > > > From: linux-acpi-owner@...r.kernel.org [mailto:linux-acpi-owner@...r.kernel.org] On Behalf Of
> > > Borislav
> > > > Petkov
> > > > Subject: Re: 174cc7187e6f ACPICA: Tables: Back port acpi_get_table_with_size() and
> > > > early_acpi_os_unmap_memory() from Linux kernel
> > > >
> > > > On Sun, Jan 08, 2017 at 03:20:20AM +0100, Rafael J. Wysocki wrote:
> > > > >  drivers/iommu/amd_iommu_init.c |    2 +-
> > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > >
> > > > > Index: linux-pm/drivers/iommu/amd_iommu_init.c
> > > > > ===================================================================
> > > > > --- linux-pm.orig/drivers/iommu/amd_iommu_init.c
> > > > > +++ linux-pm/drivers/iommu/amd_iommu_init.c
> > > > > @@ -2230,7 +2230,7 @@ static int __init early_amd_iommu_init(v
> > > > >  	 */
> > > > >  	ret = check_ivrs_checksum(ivrs_base);
> > > > >  	if (ret)
> > > > > -		return ret;
> > > > > +		goto out;
> > > > >
> > > > >  	amd_iommu_target_ivhd_type = get_highest_supported_ivhd_type(ivrs_base);
> > > > >  	DUMP_printk("Using IVHD type %#x\n", amd_iommu_target_ivhd_type);
> > > >
> > > > Good catch, this one needs to be applied regardless.
> > > >
> > > > However, it doesn't fix my issue though.
> > > >
> > > > But I think I have it - I went and applied the well-proven debugging
> > > > technique of sprinkling printks around. Here's what I'm seeing:
> > > >
> > > > early_amd_iommu_init()
> > > > |-> acpi_put_table(ivrs_base);
> > > > |-> acpi_tb_put_table(table_desc);
> > > > |-> acpi_tb_invalidate_table(table_desc);
> > > > |-> acpi_tb_release_table(...)
> > > > |-> acpi_os_unmap_memory
> > > > |-> acpi_os_unmap_iomem
> > > > |-> acpi_os_map_cleanup
> > > > |-> synchronize_rcu_expedited	<-- the kernel/rcu/tree_exp.h version with CONFIG_PREEMPT_RCU=y
> > > >
> > > > Now that function goes and sends IPIs, i.e., schedule_work()
> > > > but this is too early - we haven't even done workqueue_init().
> > > > Actually, from looking at the callstack, we do
> > > > kernel_init_freeable->native_smp_prepare_cpus() and workqueue_init()
> > > > comes next.
> > > >
> > > > And this makes sense because the splat rIP points to __queue_work() but
> > > > we haven't done that yet.
> > > >
> > > > So that acpi_put_table() is happening too early. Looks like AMD IOMMU
> > > > should not put the table but WTH do I know?!
> > > >
> > > > In any case, commenting out:
> > > >
> > > >         acpi_put_table(ivrs_base);
> > > >         ivrs_base = NULL;
> > > >
> > > > and the end of early_amd_iommu_init() makes the box boot again.
> > > 
> > > So please help to comment out these 2 lines (with descriptions and do not delete them).
> > > Until acpi_os_unmap_memory() is able to handle such an early case.
> > 
> > IMO, synchronize_rcu_expedited() should be improved:
> > If rcu_init() isn't called or there is nothing to synchronize, schedule_work() shouldn't be invoked.

Indeed it should!

Does the (untested) patch below fix things for you?

If so, does this need to go into 4.10?  (My default workflow would get
it into 4.11 or 4.12, so please speak up if you need it.)

							Thanx, Paul

------------------------------------------------------------------------

commit 1b7feb708241f1662cfd529118468c9f9c0b1449
Author: Paul E. McKenney <paulmck@...ux.vnet.ibm.com>
Date:   Mon Jan 9 14:10:50 2017 -0800

    rcu: Make synchronize_rcu_expedited() safe for early boot
    
    The synchronize_rcu_expedited() function does not check for early-boot
    use, which can result in failures if it is invoked before the scheduler
    has started.  Given that the rcupdate.rcu_expedited kernel parameter
    causes all calls to synchronize_rcu() to be directed instead to
    synchronize_rcu_expedited(), a usage restriction does not make sense.
    
    This commit therefore adds a rcu_scheduler_active check to
    synchronize_rcu_expedited(), so that it is a no-op before the scheduler
    starts.  This behavior is correct because there is only a single CPU
    running during that time.
    
    Reported-by: Lv Zheng <lv.zheng@...el.com>
    Reported-by: Borislav Petkov <bp@...en8.de>
    Signed-off-by: Paul E. McKenney <paulmck@...ux.vnet.ibm.com>

diff --git a/kernel/rcu/tree_exp.h b/kernel/rcu/tree_exp.h
index dfc3ba5a429e..a6c3d86480de 100644
--- a/kernel/rcu/tree_exp.h
+++ b/kernel/rcu/tree_exp.h
@@ -690,6 +690,8 @@ void synchronize_rcu_expedited(void)
 {
 	struct rcu_state *rsp = rcu_state_p;
 
+	if (!rcu_scheduler_active)
+		return;
 	_synchronize_rcu_expedited(rsp, sync_rcu_exp_handler);
 }
 EXPORT_SYMBOL_GPL(synchronize_rcu_expedited);

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ