* Re: [PATCH v2 1/3] KVM: x86: add support for user wait instructions
From: Wanpeng Li @ 2019-05-28 6:11 UTC (permalink / raw)
To: Tao Xu
Cc: Peter Zijlstra, Paolo Bonzini, Radim Krcmar, Jonathan Corbet,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin,
Sean Christopherson, the arch/x86 maintainers, kvm, linux-doc,
LKML, jingqi.liu
In-Reply-To: <43e2a62a-e992-2138-f038-1e4b2fb79ad1@intel.com>
On Tue, 28 May 2019 at 13:16, Tao Xu <tao3.xu@intel.com> wrote:
>
>
> On 27/05/2019 18:30, Peter Zijlstra wrote:
> > On Fri, May 24, 2019 at 03:56:35PM +0800, Tao Xu wrote:
> >> This patch adds support for UMONITOR, UMWAIT and TPAUSE instructions
> >> in kvm, and by default dont't expose it to kvm and provide a capability
> >> to enable it.
> >
> > I'm thinking this should be conditional on the guest being a 1:1 guest,
> > and I also seem to remember we have bits for that already -- they were
> > used to disable paravirt spinlocks for example.
> >
>
> Hi Peter,
>
> I am wondering if "1:1 guest" means different guests in the same host
> should have different settings on user wait instructions?
>
> User wait instructions(UMONITOR, UMWAIT and TPAUSE) can use in guest
> only when the VMCS Secondary Processor-Based VM-Execution Control bit 26
> is 1, otherwise any execution of TPAUSE, UMONITOR, or UMWAIT causes a #UD.
>
> So with a capability to enable it, we use qemu kvm_vm_ioctl_enable_cap()
> to enable it. The qemu link is blew:
> https://lists.gnu.org/archive/html/qemu-devel/2019-05/msg05810.html
>
> By using different QEMU parameters, different guests in the same host
> would have different features with or without user wait instructions.
>
> About "disable paravirt spinlocks" case, I am wondering if it uses
Please refer to a4429e53c9 (KVM: Introduce paravirtualization hints
and KVM_HINTS_DEDICATED) and b2798ba0b87 (KVM: X86: Choose qspinlock
when dedicated physical CPUs are available).
> kernel parameters? If it uses kernel parameters, different guests in the
> same host may have same settings on user wait instructions.
>
> Or when we uses kernel parameters to disable user wait instructions, for
> a host chooses to enable user wait instructions, we should do some work
> on QEMU to choose disable or enable user wait instructions?
>
> Thanks
>
> Tao
^ permalink raw reply
* Re: [PATCH v2 1/3] KVM: x86: add support for user wait instructions
From: Tao Xu @ 2019-05-28 7:19 UTC (permalink / raw)
To: Wanpeng Li
Cc: Peter Zijlstra, Paolo Bonzini, Radim Krcmar, Jonathan Corbet,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin,
Sean Christopherson, the arch/x86 maintainers, kvm, linux-doc,
LKML, jingqi.liu
In-Reply-To: <CANRm+CwnJoj0EwWoFC44SXVUTLdE+iFGovaMr4Yf=OzbaW36sA@mail.gmail.com>
On 28/05/2019 14:11, Wanpeng Li wrote:
> On Tue, 28 May 2019 at 13:16, Tao Xu <tao3.xu@intel.com> wrote:
>>
>>
>> On 27/05/2019 18:30, Peter Zijlstra wrote:
>>> On Fri, May 24, 2019 at 03:56:35PM +0800, Tao Xu wrote:
>>>> This patch adds support for UMONITOR, UMWAIT and TPAUSE instructions
>>>> in kvm, and by default dont't expose it to kvm and provide a capability
>>>> to enable it.
>>>
>>> I'm thinking this should be conditional on the guest being a 1:1 guest,
>>> and I also seem to remember we have bits for that already -- they were
>>> used to disable paravirt spinlocks for example.
>>>
>>
>> Hi Peter,
>>
>> I am wondering if "1:1 guest" means different guests in the same host
>> should have different settings on user wait instructions?
>>
>> User wait instructions(UMONITOR, UMWAIT and TPAUSE) can use in guest
>> only when the VMCS Secondary Processor-Based VM-Execution Control bit 26
>> is 1, otherwise any execution of TPAUSE, UMONITOR, or UMWAIT causes a #UD.
>>
>> So with a capability to enable it, we use qemu kvm_vm_ioctl_enable_cap()
>> to enable it. The qemu link is blew:
>> https://lists.gnu.org/archive/html/qemu-devel/2019-05/msg05810.html
>>
>> By using different QEMU parameters, different guests in the same host
>> would have different features with or without user wait instructions.
>>
>> About "disable paravirt spinlocks" case, I am wondering if it uses
>
> Please refer to a4429e53c9 (KVM: Introduce paravirtualization hints
> and KVM_HINTS_DEDICATED) and b2798ba0b87 (KVM: X86: Choose qspinlock
> when dedicated physical CPUs are available)
Hi Wanpeng,
Thank you! This information really helped me. After I read the code in
KVM/QEMU, I was wondering that with qemu command-line "-cpu
host,+kvm-hint-dedicated", then in KVM,
"kvm_hint_has_feature(KVM_HINTS_DEDICATED)" will be true, am I right?
Tao
^ permalink raw reply
* Re: [PATCH v2 2/2] tracing/kprobe: Add kprobe_event= boot parameter
From: Anders Roxell @ 2019-05-28 12:23 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: Steven Rostedt, Ingo Molnar, Naveen N . Rao, Anil S Keshavamurthy,
David S . Miller, Linux Kernel Mailing List, linux-doc
In-Reply-To: <155851395498.15728.830529496248543583.stgit@devnote2>
On Wed, 22 May 2019 at 10:32, Masami Hiramatsu <mhiramat@kernel.org> wrote:
>
> Add kprobe_event= boot parameter to define kprobe events
> at boot time.
> The definition syntax is similar to tracefs/kprobe_events
> interface, but use ',' and ';' instead of ' ' and '\n'
> respectively. e.g.
>
> kprobe_event=p,vfs_read,$arg1,$arg2
>
> This puts a probe on vfs_read with argument1 and 2, and
> enable the new event.
>
> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
I built an arm64 kernel from todays linux-next tag next-20190528 and
ran in to this issue when I booted it up in qemu:
[ 9.068772][ T1] CPU: 0 PID: 1 Comm: swapper/0 Not tainted
5.2.0-rc2-next-20190528-00019-g9a6008710716 #8
[ 9.072893][ T1] Hardware name: linux,dummy-virt (DT)
[ 9.075143][ T1] pstate: 80400005 (Nzcv daif +PAN -UAO)
[ 9.077528][ T1] pc : kprobe_target+0x0/0x30
[ 9.079479][ T1] lr : init_test_probes+0x134/0x540
[ 9.081611][ T1] sp : ffff80003f51fbe0
[ 9.083331][ T1] x29: ffff80003f51fbe0 x28: ffff200013c17820
[ 9.085906][ T1] x27: ffff200015d3ab40 x26: ffff2000122bb120
[ 9.088491][ T1] x25: 0000000000000000 x24: ffff200013c08ae0
[ 9.091068][ T1] x23: ffff200015d39000 x22: ffff200013a15ac8
[ 9.093667][ T1] x21: 1ffff00007ea3f86 x20: ffff200015d39420
[ 9.096214][ T1] x19: ffff2000122bad20 x18: 0000000000001400
[ 9.098831][ T1] x17: 0000000000000000 x16: ffff80003f510040
[ 9.101410][ T1] x15: 0000000000001480 x14: 1ffff00007ea3ea2
[ 9.103963][ T1] x13: 00000000f1f1f1f1 x12: ffff040002782e0d
[ 9.106549][ T1] x11: 1fffe40002782e0c x10: ffff040002782e0c
[ 9.109120][ T1] x9 : 1fffe40002782e0c x8 : dfff200000000000
[ 9.111676][ T1] x7 : ffff040002782e0d x6 : ffff200013c17067
[ 9.114234][ T1] x5 : ffff80003f510040 x4 : 0000000000000000
[ 9.116843][ T1] x3 : ffff200010427508 x2 : 0000000000000000
[ 9.119409][ T1] x1 : ffff200010426e10 x0 : 0000000000a6326b
[ 9.121980][ T1] Call trace:
[ 9.123380][ T1] kprobe_target+0x0/0x30
[ 9.125205][ T1] init_kprobes+0x2b8/0x300
[ 9.127074][ T1] do_one_initcall+0x4c0/0xa68
[ 9.129076][ T1] kernel_init_freeable+0x3c4/0x4e4
[ 9.131234][ T1] kernel_init+0x14/0x1fc
[ 9.133032][ T1] ret_from_fork+0x10/0x18
[ 9.134908][ T1] Code: a9446bf9 f9402bfb a8d87bfd d65f03c0 (d4200080)
[ 9.137845][ T1] ---[ end trace 49243ee03446b072 ]---
[ 9.140114][ T1] Kernel panic - not syncing: Fatal exception
[ 9.142684][ T1] ---[ end Kernel panic - not syncing: Fatal exception ]---
I bisected down to this commit as the one that introduces this issue.
I'm unsure why this causes the call trace though, any ideas?
Cheers,
Anders
^ permalink raw reply
* Re: [PATCH RFC 0/5] Remove some notrace RCU APIs
From: Paul E. McKenney @ 2019-05-28 12:24 UTC (permalink / raw)
To: Joel Fernandes
Cc: Steven Rostedt, linux-kernel, Benjamin Herrenschmidt, Ingo Molnar,
Jonathan Corbet, Josh Triplett, kvm-ppc, Lai Jiangshan, linux-doc,
linuxppc-dev, Mathieu Desnoyers, Michael Ellerman, Miguel Ojeda,
Paul Mackerras, rcu
In-Reply-To: <20190525181407.GA220326@google.com>
On Sat, May 25, 2019 at 02:14:07PM -0400, Joel Fernandes wrote:
> On Sat, May 25, 2019 at 08:50:35AM -0700, Paul E. McKenney wrote:
> > On Sat, May 25, 2019 at 10:19:54AM -0400, Joel Fernandes wrote:
> > > On Sat, May 25, 2019 at 07:08:26AM -0400, Steven Rostedt wrote:
> > > > On Sat, 25 May 2019 04:14:44 -0400
> > > > Joel Fernandes <joel@joelfernandes.org> wrote:
> > > >
> > > > > > I guess the difference between the _raw_notrace and just _raw variants
> > > > > > is that _notrace ones do a rcu_check_sparse(). Don't we want to keep
> > > > > > that check?
> > > > >
> > > > > This is true.
> > > > >
> > > > > Since the users of _raw_notrace are very few, is it worth keeping this API
> > > > > just for sparse checking? The API naming is also confusing. I was expecting
> > > > > _raw_notrace to do fewer checks than _raw, instead of more. Honestly, I just
> > > > > want to nuke _raw_notrace as done in this series and later we can introduce a
> > > > > sparse checking version of _raw if need-be. The other option could be to
> > > > > always do sparse checking for _raw however that used to be the case and got
> > > > > changed in http://lists.infradead.org/pipermail/linux-afs/2016-July/001016.html
> > > >
> > > > What if we just rename _raw to _raw_nocheck, and _raw_notrace to _raw ?
> > >
> > > That would also mean changing 160 usages of _raw to _raw_nocheck in the
> > > kernel :-/.
> > >
> > > The tracing usage of _raw_notrace is only like 2 or 3 users. Can we just call
> > > rcu_check_sparse directly in the calling code for those and eliminate the APIs?
> > >
> > > I wonder what Paul thinks about the matter as well.
> >
> > My thought is that it is likely that a goodly number of the current uses
> > of _raw should really be some form of _check, with lockdep expressions
> > spelled out. Not that working out what exactly those lockdep expressions
> > should be is necessarily a trivial undertaking. ;-)
>
> Yes, currently where I am a bit stuck is the rcu_dereference_raw()
> cannot possibly know what SRCU domain it is under, so lockdep cannot check if
> an SRCU lock is held without the user also passing along the SRCU domain. I
> am trying to change lockdep to see if it can check if *any* srcu domain lock
> is held (regardless of which one) and complain if none are. This is at least
> better than no check at all.
>
> However, I think it gets tricky for mutexes. If you have something like:
> mutex_lock(some_mutex);
> p = rcu_dereference_raw(gp);
> mutex_unlock(some_mutex);
>
> This might be a perfectly valid invocation of _raw, however my checks (patch
> is still cooking) trigger a lockdep warning becase _raw cannot know that this
> is Ok. lockdep thinks it is not in a reader section. This then gets into the
> territory of a new rcu_derference_raw_protected(gp, assert_held(some_mutex))
> which sucks because its yet another API. To circumvent this issue, can we
> just have callers of rcu_dereference_raw ensure that they call
> rcu_read_lock() if they are protecting dereferences by a mutex? That would
> make things a lot easier and also may be Ok since rcu_read_lock is quite
> cheap.
Why not just rcu_dereference_protected(lockdep_is_held(some_mutex))?
The API is already there, and no need for spurious readers.
> > That aside, if we are going to change the name of an API that is
> > used 160 places throughout the tree, we would need to have a pretty
> > good justification. Without such a justification, it will just look
> > like pointless churn to the various developers and maintainers on the
> > receiving end of the patches.
>
> Actually, the API name change is not something I want to do, it is Steven
> suggestion. My suggestion is let us just delete _raw_notrace and just use the
> _raw API for tracing, since _raw doesn't do any tracing anyway. Steve pointed
> that _raw_notrace does sparse checking unlike _raw, but I think that isn't an
> issue since _raw doesn't do such checking at the moment anyway.. (if possible
> check my cover letter again for details/motivation of this series).
Understood, but regardless of who suggested it, if we are to go through
with it, good justification will be required. ;-)
Thanx, Paul
> thanks!
>
> - Joel
>
> > Thanx, Paul
> >
> > > thanks, Steven!
> > >
> >
>
^ permalink raw reply
* Re: [PATCH v2 2/2] tracing/kprobe: Add kprobe_event= boot parameter
From: Steven Rostedt @ 2019-05-28 12:36 UTC (permalink / raw)
To: Anders Roxell
Cc: Masami Hiramatsu, Ingo Molnar, Naveen N . Rao,
Anil S Keshavamurthy, David S . Miller, Linux Kernel Mailing List,
linux-doc
In-Reply-To: <CADYN=9KHQPQTgy==TYZ5iD_zViPbHq4hgOUwuX69aQWV6vZOQg@mail.gmail.com>
On Tue, 28 May 2019 14:23:43 +0200
Anders Roxell <anders.roxell@linaro.org> wrote:
> On Wed, 22 May 2019 at 10:32, Masami Hiramatsu <mhiramat@kernel.org> wrote:
> >
> > Add kprobe_event= boot parameter to define kprobe events
> > at boot time.
> > The definition syntax is similar to tracefs/kprobe_events
> > interface, but use ',' and ';' instead of ' ' and '\n'
> > respectively. e.g.
> >
> > kprobe_event=p,vfs_read,$arg1,$arg2
> >
> > This puts a probe on vfs_read with argument1 and 2, and
> > enable the new event.
> >
> > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
>
> I built an arm64 kernel from todays linux-next tag next-20190528 and
> ran in to this issue when I booted it up in qemu:
>
This is partial output from the crash. There should be more output
before this that says what happened to cause the crash.
-- Steve
> [ 9.068772][ T1] CPU: 0 PID: 1 Comm: swapper/0 Not tainted
> 5.2.0-rc2-next-20190528-00019-g9a6008710716 #8
> [ 9.072893][ T1] Hardware name: linux,dummy-virt (DT)
> [ 9.075143][ T1] pstate: 80400005 (Nzcv daif +PAN -UAO)
> [ 9.077528][ T1] pc : kprobe_target+0x0/0x30
> [ 9.079479][ T1] lr : init_test_probes+0x134/0x540
> [ 9.081611][ T1] sp : ffff80003f51fbe0
> [ 9.083331][ T1] x29: ffff80003f51fbe0 x28: ffff200013c17820
> [ 9.085906][ T1] x27: ffff200015d3ab40 x26: ffff2000122bb120
> [ 9.088491][ T1] x25: 0000000000000000 x24: ffff200013c08ae0
> [ 9.091068][ T1] x23: ffff200015d39000 x22: ffff200013a15ac8
> [ 9.093667][ T1] x21: 1ffff00007ea3f86 x20: ffff200015d39420
> [ 9.096214][ T1] x19: ffff2000122bad20 x18: 0000000000001400
> [ 9.098831][ T1] x17: 0000000000000000 x16: ffff80003f510040
> [ 9.101410][ T1] x15: 0000000000001480 x14: 1ffff00007ea3ea2
> [ 9.103963][ T1] x13: 00000000f1f1f1f1 x12: ffff040002782e0d
> [ 9.106549][ T1] x11: 1fffe40002782e0c x10: ffff040002782e0c
> [ 9.109120][ T1] x9 : 1fffe40002782e0c x8 : dfff200000000000
> [ 9.111676][ T1] x7 : ffff040002782e0d x6 : ffff200013c17067
> [ 9.114234][ T1] x5 : ffff80003f510040 x4 : 0000000000000000
> [ 9.116843][ T1] x3 : ffff200010427508 x2 : 0000000000000000
> [ 9.119409][ T1] x1 : ffff200010426e10 x0 : 0000000000a6326b
> [ 9.121980][ T1] Call trace:
> [ 9.123380][ T1] kprobe_target+0x0/0x30
> [ 9.125205][ T1] init_kprobes+0x2b8/0x300
> [ 9.127074][ T1] do_one_initcall+0x4c0/0xa68
> [ 9.129076][ T1] kernel_init_freeable+0x3c4/0x4e4
> [ 9.131234][ T1] kernel_init+0x14/0x1fc
> [ 9.133032][ T1] ret_from_fork+0x10/0x18
> [ 9.134908][ T1] Code: a9446bf9 f9402bfb a8d87bfd d65f03c0 (d4200080)
> [ 9.137845][ T1] ---[ end trace 49243ee03446b072 ]---
> [ 9.140114][ T1] Kernel panic - not syncing: Fatal exception
> [ 9.142684][ T1] ---[ end Kernel panic - not syncing: Fatal exception ]---
>
> I bisected down to this commit as the one that introduces this issue.
> I'm unsure why this causes the call trace though, any ideas?
>
> Cheers,
> Anders
^ permalink raw reply
* Re: [PATCH v7 1/1] iommu: enhance IOMMU dma mode build options
From: Leizhen (ThunderTown) @ 2019-05-28 13:05 UTC (permalink / raw)
To: Joerg Roedel
Cc: Jean-Philippe Brucker, John Garry, Robin Murphy, Will Deacon,
Jonathan Corbet, linux-doc, Sebastian Ott, Gerald Schaefer,
Martin Schwidefsky, Heiko Carstens, Benjamin Herrenschmidt,
Paul Mackerras, Michael Ellerman, Tony Luck, Fenghua Yu,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, H . Peter Anvin,
David Woodhouse, iommu, linux-kernel, linux-s390, linuxppc-dev,
x86, linux-ia64, Hanjun Guo
In-Reply-To: <20190527142140.GH8420@8bytes.org>
On 2019/5/27 22:21, Joerg Roedel wrote:
> Hi Zhen Lei,
>
> On Mon, May 20, 2019 at 09:59:47PM +0800, Zhen Lei wrote:
>> arch/ia64/kernel/pci-dma.c | 2 +-
>> arch/powerpc/platforms/powernv/pci-ioda.c | 3 ++-
>> arch/s390/pci/pci_dma.c | 2 +-
>> arch/x86/kernel/pci-dma.c | 7 ++---
>> drivers/iommu/Kconfig | 44 ++++++++++++++++++++++++++-----
>> drivers/iommu/amd_iommu_init.c | 3 ++-
>> drivers/iommu/intel-iommu.c | 2 +-
>> drivers/iommu/iommu.c | 3 ++-
>> 8 files changed, 48 insertions(+), 18 deletions(-)
>
> This needs Acks from the arch maintainers of ia64, powerpc, s390 and
> x86, at least.
>
> It is easier for them if you split it up into the Kconfig change and
> separete patches per arch and per iommu driver. Then collect the Acks on
> the individual patches.
OK, thanks. I will do it tomorrow.
>
> Thanks,
>
> Joerg
>
> .
>
--
Thanks!
BestRegards
^ permalink raw reply
* Re: [PATCH v2 2/2] tracing/kprobe: Add kprobe_event= boot parameter
From: Anders Roxell @ 2019-05-28 13:39 UTC (permalink / raw)
To: Steven Rostedt
Cc: Masami Hiramatsu, Ingo Molnar, Naveen N . Rao,
Anil S Keshavamurthy, David S . Miller, Linux Kernel Mailing List,
linux-doc
In-Reply-To: <20190528083629.3c100256@gandalf.local.home>
On Tue, 28 May 2019 at 14:36, Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Tue, 28 May 2019 14:23:43 +0200
> Anders Roxell <anders.roxell@linaro.org> wrote:
>
> > On Wed, 22 May 2019 at 10:32, Masami Hiramatsu <mhiramat@kernel.org> wrote:
> > >
> > > Add kprobe_event= boot parameter to define kprobe events
> > > at boot time.
> > > The definition syntax is similar to tracefs/kprobe_events
> > > interface, but use ',' and ';' instead of ' ' and '\n'
> > > respectively. e.g.
> > >
> > > kprobe_event=p,vfs_read,$arg1,$arg2
> > >
> > > This puts a probe on vfs_read with argument1 and 2, and
> > > enable the new event.
> > >
> > > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> >
> > I built an arm64 kernel from todays linux-next tag next-20190528 and
> > ran in to this issue when I booted it up in qemu:
> >
>
> This is partial output from the crash. There should be more output
> before this that says what happened to cause the crash.
Oh I'm sorry.
Here is the full output
[ 0.000000][ T0] Booting Linux on physical CPU 0x0000000000 [0x410fd034]
[ 0.000000][ T0] Linux version
5.2.0-rc2-next-20190528-00019-g9a6008710716 (anders@compilator) (gcc
version 8.3.0 (Debian 8.3.0-2)) #8 SMP PREEMPT Tue May 28 11:47:57
CEST 2019
[ 0.000000][ T0] Machine model: linux,dummy-virt
[ 0.000000][ T0] earlycon: pl11 at MMIO 0x0000000009000000 (options '')
[ 0.000000][ T0] printk: bootconsole [pl11] enabled
[ 0.000000][ T0] efi: Getting EFI parameters from FDT:
[ 0.000000][ T0] efi: UEFI not found.
[ 0.000000][ T0] cma: Reserved 32 MiB at 0x00000000be000000
[ 0.000000][ T0] kmemleak: Kernel memory leak detector disabled
[ 0.000000][ T0] NUMA: No NUMA configuration found
[ 0.000000][ T0] NUMA: Faking a node at [mem
0x0000000040000000-0x00000000bfffffff]
[ 0.000000][ T0] NUMA: NODE_DATA [mem 0xbdbfd480-0xbdbfefff]
[ 0.000000][ T0] Zone ranges:
[ 0.000000][ T0] DMA32 [mem 0x0000000040000000-0x00000000bfffffff]
[ 0.000000][ T0] Normal empty
[ 0.000000][ T0] Movable zone start for each node
[ 0.000000][ T0] Early memory node ranges
[ 0.000000][ T0] node 0: [mem 0x0000000040000000-0x00000000bfffffff]
[ 0.000000][ T0] Initmem setup node 0 [mem
0x0000000040000000-0x00000000bfffffff]
[ 0.000000][ T0] kasan: KernelAddressSanitizer initialized
[ 0.000000][ T0] psci: probing for conduit method from DT.
[ 0.000000][ T0] psci: PSCIv1.0 detected in firmware.
[ 0.000000][ T0] psci: Using standard PSCI v0.2 function IDs
[ 0.000000][ T0] psci: Trusted OS migration not required
[ 0.000000][ T0] psci: SMC Calling Convention v1.1
[ 0.000000][ T0] percpu: Embedded 505 pages/cpu s2030744 r8192
d29544 u2068480
[ 0.000000][ T0] Detected VIPT I-cache on CPU0
[ 0.000000][ T0] CPU features: detected: ARM erratum 845719
[ 0.000000][ T0] CPU features: GIC system register CPU interface
present but disabled by higher exception level
[ 0.000000][ T0] node[0] zonelist: 0:DMA32
[ 0.000000][ T0] Built 1 zonelists, mobility grouping on. Total
pages: 516096
[ 0.000000][ T0] Policy zone: DMA32
[ 0.000000][ T0] Kernel command line: root=/dev/root
rootfstype=9p rootflags=trans=virtio console=ttyAMA0,38400n8
earlycon=pl011,0x9000000 initcall_debug softlockup_panic=0
security=none
[ 0.000000][ T0] Memory: 942464K/2097152K available (33532K
kernel code, 24756K rwdata, 16956K rodata, 6144K init, 22215K bss,
420912K reserved, 32768K cma-reserved)
[ 0.000000][ T0] random: get_random_u64 called from
kmem_cache_open+0x2c/0x580 with crng_init=0
[ 0.000000][ T0] random: get_random_u64 called from
cache_random_seq_create+0x6c/0x1a8 with crng_init=0
[ 0.000000][ T0] SLUB: HWalign=64, Order=0-3, MinObjects=0,
CPUs=1, Nodes=1
[ 0.000000][ T0] random: get_random_u64 called from
kmem_cache_open+0x2c/0x580 with crng_init=0
[ 0.000000][ T0] ODEBUG: selftest passed
[ 0.000000][ T0] Running RCU self tests
[ 0.000000][ T0] rcu: Preemptible hierarchical RCU implementation.
[ 0.000000][ T0] rcu: RCU event tracing is enabled.
[ 0.000000][ T0] rcu: RCU dyntick-idle grace-period
acceleration is enabled.
[ 0.000000][ T0] rcu: RCU lockdep checking is enabled.
[ 0.000000][ T0] rcu: RCU restricting CPUs from NR_CPUS=256
to nr_cpu_ids=1.
[ 0.000000][ T0] rcu: RCU priority boosting: priority 1 delay 500 ms.
[ 0.000000][ T0] rcu: RCU callback double-/use-after-free
debug enabled.
[ 0.000000][ T0] rcu: RCU debug extended QS entry/exit.
[ 0.000000][ T0] Tasks RCU enabled.
[ 0.000000][ T0] rcu: RCU calculated value of
scheduler-enlistment delay is 25 jiffies.
[ 0.000000][ T0] rcu: Adjusting geometry for rcu_fanout_leaf=16,
nr_cpu_ids=1
[ 0.000000][ T0] NR_IRQS: 64, nr_irqs: 64, preallocated irqs: 0
[ 0.000000][ T0] GICv2m: range[mem 0x08020000-0x08020fff], SPI[80:143]
[ 0.000000][ T0] arch_timer: cp15 timer(s) running at 100.00MHz (virt).
[ 0.000000][ T0] clocksource: arch_sys_counter: mask:
0xffffffffffffff max_cycles: 0x171024e7e0, max_idle_ns: 440795205315
ns
[ 0.000041][ T0] sched_clock: 56 bits at 100MHz, resolution
10ns, wraps every 4398046511100ns
[ 0.007303][ T0] Console: colour dummy device 80x25
[ 0.012458][ T0] Lock dependency validator: Copyright (c) 2006
Red Hat, Inc., Ingo Molnar
[ 0.016254][ T0] ... MAX_LOCKDEP_SUBCLASSES: 8
[ 0.018260][ T0] ... MAX_LOCK_DEPTH: 48
[ 0.020472][ T0] ... MAX_LOCKDEP_KEYS: 8191
[ 0.022584][ T0] ... CLASSHASH_SIZE: 4096
[ 0.024891][ T0] ... MAX_LOCKDEP_ENTRIES: 32768
[ 0.027089][ T0] ... MAX_LOCKDEP_CHAINS: 65536
[ 0.029532][ T0] ... CHAINHASH_SIZE: 32768
[ 0.031812][ T0] memory used by lock dependency info: 6747 kB
[ 0.034789][ T0] per task-struct memory footprint: 2688 bytes
[ 0.037446][ T0] ------------------------
[ 0.039583][ T0] | Locking API testsuite:
[ 0.041539][ T0]
----------------------------------------------------------------------------
[ 0.045659][ T0] | spin |wlock
|rlock |mutex | wsem | rsem |
[ 0.049470][ T0]
--------------------------------------------------------------------------
[ 0.053770][ T0] A-A deadlock: ok | ok
| ok | ok | ok | ok | ok |
[ 0.175649][ T0] A-B-B-A deadlock: ok | ok
| ok | ok | ok | ok | ok |
[ 0.304904][ T0] A-B-B-C-C-A deadlock: ok | ok
| ok | ok | ok | ok | ok |
[ 0.441305][ T0] A-B-C-A-B-C deadlock: ok | ok
| ok | ok | ok | ok | ok |
[ 0.577838][ T0] A-B-B-C-C-D-D-A deadlock: ok | ok
| ok | ok | ok | ok | ok |
[ 0.721811][ T0] A-B-C-D-B-D-D-A deadlock: ok | ok
| ok | ok | ok | ok | ok |
[ 0.866090][ T0] A-B-C-D-B-C-D-A deadlock: ok | ok
| ok | ok | ok | ok | ok |
[ 1.009999][ T0] double unlock: ok | ok
| ok | ok | ok | ok | ok |
[ 1.131550][ T0] initialize held: ok | ok
| ok | ok | ok | ok | ok |
[ 1.250065][ T0]
--------------------------------------------------------------------------
[ 1.254034][ T0] recursive read-lock:
| ok | | ok |
[ 1.292322][ T0] recursive read-lock #2:
| ok | | ok |
[ 1.329570][ T0] mixed read-write-lock:
| ok | | ok |
[ 1.366837][ T0] mixed write-read-lock:
| ok | | ok |
[ 1.404193][ T0] mixed read-lock/lock-write ABBA:
|FAILED| | ok |
[ 1.440046][ T0] mixed read-lock/lock-read ABBA:
| ok | | ok |
[ 1.479427][ T0] mixed write-lock/lock-write ABBA:
| ok | | ok |
[ 1.518841][ T0]
--------------------------------------------------------------------------
[ 1.526367][ T0] hard-irqs-on + irq-safe-A/12: ok | ok | ok |
[ 1.579733][ T0] soft-irqs-on + irq-safe-A/12: ok | ok | ok |
[ 1.633140][ T0] hard-irqs-on + irq-safe-A/21: ok | ok | ok |
[ 1.686438][ T0] soft-irqs-on + irq-safe-A/21: ok | ok | ok |
[ 1.739858][ T0] sirq-safe-A => hirqs-on/12: ok | ok | ok |
[ 1.793336][ T0] sirq-safe-A => hirqs-on/21: ok | ok | ok |
[ 1.846724][ T0] hard-safe-A + irqs-on/12: ok | ok | ok |
[ 1.900131][ T0] soft-safe-A + irqs-on/12: ok | ok | ok |
[ 1.953611][ T0] hard-safe-A + irqs-on/21: ok | ok | ok |
[ 2.007020][ T0] soft-safe-A + irqs-on/21: ok | ok | ok |
[ 2.060514][ T0] hard-safe-A + unsafe-B #1/123: ok | ok | ok |
[ 2.117068][ T0] soft-safe-A + unsafe-B #1/123: ok | ok | ok |
[ 2.173630][ T0] hard-safe-A + unsafe-B #1/132: ok | ok | ok |
[ 2.230160][ T0] soft-safe-A + unsafe-B #1/132: ok | ok | ok |
[ 2.286732][ T0] hard-safe-A + unsafe-B #1/213: ok | ok | ok |
[ 2.343298][ T0] soft-safe-A + unsafe-B #1/213: ok | ok | ok |
[ 2.399899][ T0] hard-safe-A + unsafe-B #1/231: ok | ok | ok |
[ 2.456366][ T0] soft-safe-A + unsafe-B #1/231: ok | ok | ok |
[ 2.512893][ T0] hard-safe-A + unsafe-B #1/312: ok | ok | ok |
[ 2.567341][ T0] soft-safe-A + unsafe-B #1/312: ok | ok | ok |
[ 2.621874][ T0] hard-safe-A + unsafe-B #1/321: ok | ok | ok |
[ 2.678319][ T0] soft-safe-A + unsafe-B #1/321: ok | ok | ok |
[ 2.734909][ T0] hard-safe-A + unsafe-B #2/123: ok | ok | ok |
[ 2.793422][ T0] soft-safe-A + unsafe-B #2/123: ok | ok | ok |
[ 2.850039][ T0] hard-safe-A + unsafe-B #2/132: ok | ok | ok |
[ 2.906635][ T0] soft-safe-A + unsafe-B #2/132: ok | ok | ok |
[ 2.963319][ T0] hard-safe-A + unsafe-B #2/213: ok | ok | ok |
[ 3.019876][ T0] soft-safe-A + unsafe-B #2/213: ok | ok | ok |
[ 3.076494][ T0] hard-safe-A + unsafe-B #2/231: ok | ok | ok |
[ 3.133040][ T0] soft-safe-A + unsafe-B #2/231: ok | ok | ok |
[ 3.189646][ T0] hard-safe-A + unsafe-B #2/312: ok | ok | ok |
[ 3.246206][ T0] soft-safe-A + unsafe-B #2/312: ok | ok | ok |
[ 3.302881][ T0] hard-safe-A + unsafe-B #2/321: ok | ok | ok |
[ 3.359434][ T0] soft-safe-A + unsafe-B #2/321: ok | ok | ok |
[ 3.416138][ T0] hard-irq lock-inversion/123: ok | ok | ok |
[ 3.472719][ T0] soft-irq lock-inversion/123: ok | ok | ok |
[ 3.529397][ T0] hard-irq lock-inversion/132: ok | ok | ok |
[ 3.585922][ T0] soft-irq lock-inversion/132: ok | ok | ok |
[ 3.642564][ T0] hard-irq lock-inversion/213: ok | ok | ok |
[ 3.699083][ T0] soft-irq lock-inversion/213: ok | ok | ok |
[ 3.755762][ T0] hard-irq lock-inversion/231: ok | ok | ok |
[ 3.812295][ T0] soft-irq lock-inversion/231: ok | ok | ok |
[ 3.868913][ T0] hard-irq lock-inversion/312: ok | ok | ok |
[ 3.925502][ T0] soft-irq lock-inversion/312: ok | ok | ok |
[ 3.982216][ T0] hard-irq lock-inversion/321: ok | ok | ok |
[ 4.038836][ T0] soft-irq lock-inversion/321: ok | ok | ok |
[ 4.095484][ T0] hard-irq read-recursion/123: ok |
[ 4.115858][ T0] soft-irq read-recursion/123: ok |
[ 4.136275][ T0] hard-irq read-recursion/132: ok |
[ 4.156655][ T0] soft-irq read-recursion/132: ok |
[ 4.177108][ T0] hard-irq read-recursion/213: ok |
[ 4.197524][ T0] soft-irq read-recursion/213: ok |
[ 4.217951][ T0] hard-irq read-recursion/231: ok |
[ 4.238361][ T0] soft-irq read-recursion/231: ok |
[ 4.258769][ T0] hard-irq read-recursion/312: ok |
[ 4.279146][ T0] soft-irq read-recursion/312: ok |
[ 4.299601][ T0] hard-irq read-recursion/321: ok |
[ 4.320000][ T0] soft-irq read-recursion/321: ok |
[ 4.340415][ T0]
--------------------------------------------------------------------------
[ 4.344302][ T0] | Wound/wait tests |
[ 4.346060][ T0] ---------------------
[ 4.347804][ T0] ww api failures: ok | ok | ok |
[ 4.403866][ T0] ww contexts mixing: ok | ok |
[ 4.441325][ T0] finishing ww context: ok | ok
| ok | ok |
[ 4.513923][ T0] locking mismatches: ok | ok | ok |
[ 4.569425][ T0] EDEADLK handling: ok | ok
| ok | ok | ok | ok | ok | ok | ok | ok |
[ 4.756084][ T0] spinlock nest unlocked: ok |
[ 4.775442][ T0] -----------------------------------------------------
[ 4.778554][ T0] |block | try |context|
[ 4.781423][ T0] -----------------------------------------------------
[ 4.784561][ T0] context: ok | ok | ok |
[ 4.841364][ T0] try: ok | ok | ok |
[ 4.895896][ T0] block: ok | ok | ok |
[ 4.950507][ T0] spinlock: ok | ok | ok |
[ 5.008309][ T0] -------------------------------------------------------
[ 5.011499][ T0] Good, all 261 testcases passed! |
[ 5.013625][ T0] ---------------------------------
[ 5.016475][ T0] _warn_unseeded_randomness: 17 callbacks suppressed
[ 5.016589][ T0] random: get_random_u64 called from
kmem_cache_open+0x2c/0x580 with crng_init=0
[ 5.023654][ T0] random: get_random_u64 called from
cache_random_seq_create+0x6c/0x1a8 with crng_init=0
[ 5.028321][ T0] random: get_random_u64 called from
kmem_cache_open+0x2c/0x580 with crng_init=0
[ 5.033170][ T0] Calibrating delay loop (skipped), value
calculated using timer frequency.. 200.00 BogoMIPS (lpj=400000)
[ 5.038026][ T0] pid_max: default: 32768 minimum: 301
[ 5.048372][ T0] LSM: Security Framework initializing
[ 5.051427][ T0] Yama: becoming mindful.
[ 5.054542][ T0] LoadPin: ready to pin (currently enforcing)
[ 5.091650][ T0] Dentry cache hash table entries: 262144 (order:
9, 2097152 bytes)
[ 5.109677][ T0] Inode-cache hash table entries: 131072 (order:
8, 1048576 bytes)
[ 5.115043][ T0] Mount-cache hash table entries: 4096 (order: 3,
32768 bytes)
[ 5.119102][ T0] Mountpoint-cache hash table entries: 4096
(order: 3, 32768 bytes)
[ 5.145756][ T0] *** VALIDATE proc ***
[ 5.161518][ T0] *** VALIDATE cgroup1 ***
[ 5.163416][ T0] *** VALIDATE cgroup2 ***
[ 5.216366][ T1] ASID allocator initialised with 32768 entries
[ 5.233346][ T1] rcu: Hierarchical SRCU implementation.
[ 5.355951][ T1] EFI services will not be available.
[ 5.390252][ T1] smp: Bringing up secondary CPUs ...
[ 5.392549][ T1] smp: Brought up 1 node, 1 CPU
[ 5.395021][ T1] SMP: Total of 1 processors activated.
[ 5.397323][ T1] CPU features: detected: 32-bit EL0 Support
[ 5.400043][ T1] CPU features: detected: CRC32 instructions
[ 5.453529][ T1] CPU features: emulated: Privileged Access Never
(PAN) using TTBR0_EL1 switching
[ 5.457701][ T1] CPU: All CPU(s) started at EL1
[ 5.459924][ T13] alternatives: patching kernel code
[ 7.488419][ T17] node 0 initialised, 175252 pages in 4ms
[ 7.498797][ T17] pgdatinit0 (17) used greatest stack depth:
29312 bytes left
[ 7.622979][ T1] devtmpfs: initialized
[ 7.875584][ T1] Registered cp15_barrier emulation handler
[ 7.878291][ T1] Registered setend emulation handler
[ 7.885484][ T1] workqueue: round-robin CPU selection forced,
expect performance impact
[ 7.921303][ T21] kworker/u2:0 (21) used greatest stack depth:
26864 bytes left
[ 8.098703][ T1] DMA-API: preallocated 65536 debug entries
[ 8.101639][ T1] DMA-API: debugging enabled by kernel config
[ 8.104431][ T1] clocksource: jiffies: mask: 0xffffffff
max_cycles: 0xffffffff, max_idle_ns: 7645041785100000 ns
[ 8.110343][ T1] futex hash table entries: 256 (order: 3, 32768 bytes)
[ 8.315582][ T1] _warn_unseeded_randomness: 73 callbacks suppressed
[ 8.315680][ T1] random: get_random_u64 called from
kmem_cache_open+0x2c/0x580 with crng_init=0
[ 8.322746][ T1] random: get_random_u64 called from
cache_random_seq_create+0x6c/0x1a8 with crng_init=0
[ 8.328032][ T1] random: get_random_u64 called from
kmem_cache_open+0x2c/0x580 with crng_init=0
[ 8.342773][ T1] prandom: seed boundary self test passed
[ 8.362169][ T1] prandom: 100 self tests passed
[ 8.370915][ T1] pinctrl core: initialized pinctrl subsystem
[ 8.429194][ T26] kworker/u2:1 (26) used greatest stack depth:
26320 bytes left
[ 8.495363][ T32] kworker/u2:1 (32) used greatest stack depth:
26272 bytes left
[ 8.743241][ T1]
[ 8.750192][ T1]
*************************************************************
[ 8.760037][ T1] ** NOTICE NOTICE NOTICE NOTICE NOTICE
NOTICE NOTICE **
[ 8.764666][ T1] **
**
[ 8.769961][ T1] ** IOMMU DebugFS SUPPORT HAS BEEN ENABLED IN
THIS KERNEL **
[ 8.774188][ T1] **
**
[ 8.777236][ T1] ** This means that this kernel is built to
expose internal **
[ 8.782049][ T1] ** IOMMU data structures, which may compromise
security on **
[ 8.786179][ T1] ** your system.
**
[ 8.789225][ T1] **
**
[ 8.793520][ T1] ** If you see this message and you are not
debugging the **
[ 8.796556][ T1] ** kernel, report this immediately to your
vendor! **
[ 8.801025][ T1] **
**
[ 8.804360][ T1] ** NOTICE NOTICE NOTICE NOTICE NOTICE
NOTICE NOTICE **
[ 8.807379][ T1]
*************************************************************
[ 8.880289][ T1] DMI not present or invalid.
[ 8.945156][ T1] NET: Registered protocol family 16
[ 8.965688][ T1] audit: initializing netlink subsys (disabled)
[ 8.977767][ T57] audit: type=2000 audit(2.960:1):
state=initialized audit_enabled=0 res=1
[ 9.020354][ T1] Kprobe smoke test: started
[ 9.064132][ T1] Internal error: aarch64 BRK: f2000004 [#1] PREEMPT SMP
[ 9.067084][ T1] Modules linked in:
[ 9.068772][ T1] CPU: 0 PID: 1 Comm: swapper/0 Not tainted
5.2.0-rc2-next-20190528-00019-g9a6008710716 #8
[ 9.072893][ T1] Hardware name: linux,dummy-virt (DT)
[ 9.075143][ T1] pstate: 80400005 (Nzcv daif +PAN -UAO)
[ 9.077528][ T1] pc : kprobe_target+0x0/0x30
[ 9.079479][ T1] lr : init_test_probes+0x134/0x540
[ 9.081611][ T1] sp : ffff80003f51fbe0
[ 9.083331][ T1] x29: ffff80003f51fbe0 x28: ffff200013c17820
[ 9.085906][ T1] x27: ffff200015d3ab40 x26: ffff2000122bb120
[ 9.088491][ T1] x25: 0000000000000000 x24: ffff200013c08ae0
[ 9.091068][ T1] x23: ffff200015d39000 x22: ffff200013a15ac8
[ 9.093667][ T1] x21: 1ffff00007ea3f86 x20: ffff200015d39420
[ 9.096214][ T1] x19: ffff2000122bad20 x18: 0000000000001400
[ 9.098831][ T1] x17: 0000000000000000 x16: ffff80003f510040
[ 9.101410][ T1] x15: 0000000000001480 x14: 1ffff00007ea3ea2
[ 9.103963][ T1] x13: 00000000f1f1f1f1 x12: ffff040002782e0d
[ 9.106549][ T1] x11: 1fffe40002782e0c x10: ffff040002782e0c
[ 9.109120][ T1] x9 : 1fffe40002782e0c x8 : dfff200000000000
[ 9.111676][ T1] x7 : ffff040002782e0d x6 : ffff200013c17067
[ 9.114234][ T1] x5 : ffff80003f510040 x4 : 0000000000000000
[ 9.116843][ T1] x3 : ffff200010427508 x2 : 0000000000000000
[ 9.119409][ T1] x1 : ffff200010426e10 x0 : 0000000000a6326b
[ 9.121980][ T1] Call trace:
[ 9.123380][ T1] kprobe_target+0x0/0x30
[ 9.125205][ T1] init_kprobes+0x2b8/0x300
[ 9.127074][ T1] do_one_initcall+0x4c0/0xa68
[ 9.129076][ T1] kernel_init_freeable+0x3c4/0x4e4
[ 9.131234][ T1] kernel_init+0x14/0x1fc
[ 9.133032][ T1] ret_from_fork+0x10/0x18
[ 9.134908][ T1] Code: a9446bf9 f9402bfb a8d87bfd d65f03c0 (d4200080)
[ 9.137845][ T1] ---[ end trace 49243ee03446b072 ]---
[ 9.140114][ T1] Kernel panic - not syncing: Fatal exception
[ 9.142684][ T1] ---[ end Kernel panic - not syncing: Fatal exception ]---
Cheers,
Anders
>
> -- Steve
>
>
> > [ 9.068772][ T1] CPU: 0 PID: 1 Comm: swapper/0 Not tainted
> > 5.2.0-rc2-next-20190528-00019-g9a6008710716 #8
> > [ 9.072893][ T1] Hardware name: linux,dummy-virt (DT)
> > [ 9.075143][ T1] pstate: 80400005 (Nzcv daif +PAN -UAO)
> > [ 9.077528][ T1] pc : kprobe_target+0x0/0x30
> > [ 9.079479][ T1] lr : init_test_probes+0x134/0x540
> > [ 9.081611][ T1] sp : ffff80003f51fbe0
> > [ 9.083331][ T1] x29: ffff80003f51fbe0 x28: ffff200013c17820
> > [ 9.085906][ T1] x27: ffff200015d3ab40 x26: ffff2000122bb120
> > [ 9.088491][ T1] x25: 0000000000000000 x24: ffff200013c08ae0
> > [ 9.091068][ T1] x23: ffff200015d39000 x22: ffff200013a15ac8
> > [ 9.093667][ T1] x21: 1ffff00007ea3f86 x20: ffff200015d39420
> > [ 9.096214][ T1] x19: ffff2000122bad20 x18: 0000000000001400
> > [ 9.098831][ T1] x17: 0000000000000000 x16: ffff80003f510040
> > [ 9.101410][ T1] x15: 0000000000001480 x14: 1ffff00007ea3ea2
> > [ 9.103963][ T1] x13: 00000000f1f1f1f1 x12: ffff040002782e0d
> > [ 9.106549][ T1] x11: 1fffe40002782e0c x10: ffff040002782e0c
> > [ 9.109120][ T1] x9 : 1fffe40002782e0c x8 : dfff200000000000
> > [ 9.111676][ T1] x7 : ffff040002782e0d x6 : ffff200013c17067
> > [ 9.114234][ T1] x5 : ffff80003f510040 x4 : 0000000000000000
> > [ 9.116843][ T1] x3 : ffff200010427508 x2 : 0000000000000000
> > [ 9.119409][ T1] x1 : ffff200010426e10 x0 : 0000000000a6326b
> > [ 9.121980][ T1] Call trace:
> > [ 9.123380][ T1] kprobe_target+0x0/0x30
> > [ 9.125205][ T1] init_kprobes+0x2b8/0x300
> > [ 9.127074][ T1] do_one_initcall+0x4c0/0xa68
> > [ 9.129076][ T1] kernel_init_freeable+0x3c4/0x4e4
> > [ 9.131234][ T1] kernel_init+0x14/0x1fc
> > [ 9.133032][ T1] ret_from_fork+0x10/0x18
> > [ 9.134908][ T1] Code: a9446bf9 f9402bfb a8d87bfd d65f03c0 (d4200080)
> > [ 9.137845][ T1] ---[ end trace 49243ee03446b072 ]---
> > [ 9.140114][ T1] Kernel panic - not syncing: Fatal exception
> > [ 9.142684][ T1] ---[ end Kernel panic - not syncing: Fatal exception ]---
> >
> > I bisected down to this commit as the one that introduces this issue.
> > I'm unsure why this causes the call trace though, any ideas?
> >
> > Cheers,
> > Anders
>
^ permalink raw reply
* Re: [PATCH v10 12/12] ima: Store the measurement again when appraising a modsig
From: Mimi Zohar @ 2019-05-28 14:09 UTC (permalink / raw)
To: Thiago Jung Bauermann, linux-integrity
Cc: linux-security-module, keyrings, linux-crypto, linuxppc-dev,
linux-doc, linux-kernel, Dmitry Kasatkin, James Morris,
Serge E. Hallyn, David Howells, David Woodhouse, Jessica Yu,
Herbert Xu, David S. Miller, Jonathan Corbet, AKASHI, Takahiro
In-Reply-To: <20190418035120.2354-13-bauerman@linux.ibm.com>
Hi Thiago,
On Thu, 2019-04-18 at 00:51 -0300, Thiago Jung Bauermann wrote:
> If the IMA template contains the "modsig" or "d-modsig" field, then the
> modsig should be added to the measurement list when the file is appraised.
>
> And that is what normally happens, but if a measurement rule caused a file
> containing a modsig to be measured before a different rule causes it to be
> appraised, the resulting measurement entry will not contain the modsig
> because it is only fetched during appraisal. When the appraisal rule
> triggers, it won't store a new measurement containing the modsig because
> the file was already measured.
>
> We need to detect that situation and store an additional measurement with
> the modsig. This is done by adding an IMA_MEASURE action flag if we read a
> modsig and the IMA template contains a modsig field.
With the new per policy rule "template" support being added, this
patch needs to be modified so that the per policy "template" format is
checked. ima_template_has_modsig() should be called with the
template_desc being used.
thanks,
Mimi
> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> index 8e6475854351..f91ed4189f98 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -282,9 +282,17 @@ static int process_measurement(struct file *file, const struct cred *cred,
> /* read 'security.ima' */
> xattr_len = ima_read_xattr(file_dentry(file), &xattr_value);
>
> - /* Read the appended modsig if allowed by the policy. */
> - if (iint->flags & IMA_MODSIG_ALLOWED)
> - ima_read_modsig(func, buf, size, &modsig);
> + /*
> + * Read the appended modsig, if allowed by the policy, and allow
> + * an additional measurement list entry, if needed, based on the
> + * template format.
> + */
> + if (iint->flags & IMA_MODSIG_ALLOWED) {
> + rc = ima_read_modsig(func, buf, size, &modsig);
> +
> + if (!rc && ima_template_has_modsig())
> + action |= IMA_MEASURE;
> + }
>
^ permalink raw reply
* Purchase_rfq
From: aishatu @ 2019-05-28 14:42 UTC (permalink / raw)
To: linux-doc
[-- Attachment #1: INQUIRY_4178916..xlsx --]
[-- Type: application/octet-stream, Size: 9404 bytes --]
^ permalink raw reply
* [PATCH 1/3] lib/test_kasan: Add bitops tests
From: Marco Elver @ 2019-05-28 16:32 UTC (permalink / raw)
To: peterz, aryabinin, dvyukov, glider, andreyknvl
Cc: corbet, tglx, mingo, bp, hpa, x86, arnd, jpoimboe, linux-doc,
linux-kernel, linux-arch, kasan-dev, Marco Elver
This adds bitops tests to the test_kasan module. In a follow-up patch,
support for bitops instrumentation will be added.
Signed-off-by: Marco Elver <elver@google.com>
---
lib/test_kasan.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 70 insertions(+), 3 deletions(-)
diff --git a/lib/test_kasan.c b/lib/test_kasan.c
index 7de2702621dc..f67f3b52251d 100644
--- a/lib/test_kasan.c
+++ b/lib/test_kasan.c
@@ -11,16 +11,17 @@
#define pr_fmt(fmt) "kasan test: %s " fmt, __func__
+#include <linux/bitops.h>
#include <linux/delay.h>
+#include <linux/kasan.h>
#include <linux/kernel.h>
-#include <linux/mman.h>
#include <linux/mm.h>
+#include <linux/mman.h>
+#include <linux/module.h>
#include <linux/printk.h>
#include <linux/slab.h>
#include <linux/string.h>
#include <linux/uaccess.h>
-#include <linux/module.h>
-#include <linux/kasan.h>
/*
* Note: test functions are marked noinline so that their names appear in
@@ -623,6 +624,71 @@ static noinline void __init kasan_strings(void)
strnlen(ptr, 1);
}
+static noinline void __init kasan_bitops(void)
+{
+ long bits = 0;
+ const long bit = sizeof(bits) * 8;
+
+ pr_info("within-bounds in set_bit");
+ set_bit(0, &bits);
+
+ pr_info("within-bounds in set_bit");
+ set_bit(bit - 1, &bits);
+
+ pr_info("out-of-bounds in set_bit\n");
+ set_bit(bit, &bits);
+
+ pr_info("out-of-bounds in __set_bit\n");
+ __set_bit(bit, &bits);
+
+ pr_info("out-of-bounds in clear_bit\n");
+ clear_bit(bit, &bits);
+
+ pr_info("out-of-bounds in __clear_bit\n");
+ __clear_bit(bit, &bits);
+
+ pr_info("out-of-bounds in clear_bit_unlock\n");
+ clear_bit_unlock(bit, &bits);
+
+ pr_info("out-of-bounds in __clear_bit_unlock\n");
+ __clear_bit_unlock(bit, &bits);
+
+ pr_info("out-of-bounds in change_bit\n");
+ change_bit(bit, &bits);
+
+ pr_info("out-of-bounds in __change_bit\n");
+ __change_bit(bit, &bits);
+
+ pr_info("out-of-bounds in test_and_set_bit\n");
+ test_and_set_bit(bit, &bits);
+
+ pr_info("out-of-bounds in __test_and_set_bit\n");
+ __test_and_set_bit(bit, &bits);
+
+ pr_info("out-of-bounds in test_and_set_bit_lock\n");
+ test_and_set_bit_lock(bit, &bits);
+
+ pr_info("out-of-bounds in test_and_clear_bit\n");
+ test_and_clear_bit(bit, &bits);
+
+ pr_info("out-of-bounds in __test_and_clear_bit\n");
+ __test_and_clear_bit(bit, &bits);
+
+ pr_info("out-of-bounds in test_and_change_bit\n");
+ test_and_change_bit(bit, &bits);
+
+ pr_info("out-of-bounds in __test_and_change_bit\n");
+ __test_and_change_bit(bit, &bits);
+
+ pr_info("out-of-bounds in test_bit\n");
+ (void)test_bit(bit, &bits);
+
+#if defined(clear_bit_unlock_is_negative_byte)
+ pr_info("out-of-bounds in clear_bit_unlock_is_negative_byte\n");
+ clear_bit_unlock_is_negative_byte(bit, &bits);
+#endif
+}
+
static int __init kmalloc_tests_init(void)
{
/*
@@ -664,6 +730,7 @@ static int __init kmalloc_tests_init(void)
kasan_memchr();
kasan_memcmp();
kasan_strings();
+ kasan_bitops();
kasan_restore_multi_shot(multishot);
--
2.22.0.rc1.257.g3120a18244-goog
^ permalink raw reply related
* [PATCH 2/3] tools/objtool: add kasan_check_* to uaccess whitelist
From: Marco Elver @ 2019-05-28 16:32 UTC (permalink / raw)
To: peterz, aryabinin, dvyukov, glider, andreyknvl
Cc: corbet, tglx, mingo, bp, hpa, x86, arnd, jpoimboe, linux-doc,
linux-kernel, linux-arch, kasan-dev, Marco Elver
In-Reply-To: <20190528163258.260144-1-elver@google.com>
This is a pre-requisite for enabling bitops instrumentation. Some bitops
may safely be used with instrumentation in uaccess regions.
For example, on x86, `test_bit` is used to test a CPU-feature in a
uaccess region: arch/x86/ia32/ia32_signal.c:361
Signed-off-by: Marco Elver <elver@google.com>
---
tools/objtool/check.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 172f99195726..eff0e5209402 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -443,6 +443,8 @@ static void add_ignores(struct objtool_file *file)
static const char *uaccess_safe_builtin[] = {
/* KASAN */
"kasan_report",
+ "kasan_check_read",
+ "kasan_check_write",
"check_memory_region",
/* KASAN out-of-line */
"__asan_loadN_noabort",
--
2.22.0.rc1.257.g3120a18244-goog
^ permalink raw reply related
* [PATCH 3/3] asm-generic, x86: Add bitops instrumentation for KASAN
From: Marco Elver @ 2019-05-28 16:32 UTC (permalink / raw)
To: peterz, aryabinin, dvyukov, glider, andreyknvl
Cc: corbet, tglx, mingo, bp, hpa, x86, arnd, jpoimboe, linux-doc,
linux-kernel, linux-arch, kasan-dev, Marco Elver
In-Reply-To: <20190528163258.260144-1-elver@google.com>
This adds a new header to asm-generic to allow optionally instrumenting
architecture-specific asm implementations of bitops.
This change includes the required change for x86 as reference and
changes the kernel API doc to point to bitops-instrumented.h instead.
Rationale: the functions in x86's bitops.h are no longer the kernel API
functions, but instead the arch_ prefixed functions, which are then
instrumented via bitops-instrumented.h.
Other architectures can similarly add support for asm implementations of
bitops.
The documentation text has been copied/moved, and *no* changes to it
have been made in this patch.
Tested: using lib/test_kasan with bitops tests (pre-requisite patch).
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=198439
Signed-off-by: Marco Elver <elver@google.com>
---
Documentation/core-api/kernel-api.rst | 2 +-
arch/x86/include/asm/bitops.h | 210 ++++----------
include/asm-generic/bitops-instrumented.h | 327 ++++++++++++++++++++++
3 files changed, 380 insertions(+), 159 deletions(-)
create mode 100644 include/asm-generic/bitops-instrumented.h
diff --git a/Documentation/core-api/kernel-api.rst b/Documentation/core-api/kernel-api.rst
index a29c99d13331..65266fa1b706 100644
--- a/Documentation/core-api/kernel-api.rst
+++ b/Documentation/core-api/kernel-api.rst
@@ -51,7 +51,7 @@ The Linux kernel provides more basic utility functions.
Bit Operations
--------------
-.. kernel-doc:: arch/x86/include/asm/bitops.h
+.. kernel-doc:: include/asm-generic/bitops-instrumented.h
:internal:
Bitmap Operations
diff --git a/arch/x86/include/asm/bitops.h b/arch/x86/include/asm/bitops.h
index 8e790ec219a5..8ebf7af9a0f4 100644
--- a/arch/x86/include/asm/bitops.h
+++ b/arch/x86/include/asm/bitops.h
@@ -49,23 +49,8 @@
#define CONST_MASK_ADDR(nr, addr) WBYTE_ADDR((void *)(addr) + ((nr)>>3))
#define CONST_MASK(nr) (1 << ((nr) & 7))
-/**
- * set_bit - Atomically set a bit in memory
- * @nr: the bit to set
- * @addr: the address to start counting from
- *
- * This function is atomic and may not be reordered. See __set_bit()
- * if you do not require the atomic guarantees.
- *
- * Note: there are no guarantees that this function will not be reordered
- * on non x86 architectures, so if you are writing portable code,
- * make sure not to rely on its reordering guarantees.
- *
- * Note that @nr may be almost arbitrarily large; this function is not
- * restricted to acting on a single-word quantity.
- */
static __always_inline void
-set_bit(long nr, volatile unsigned long *addr)
+arch_set_bit(long nr, volatile unsigned long *addr)
{
if (IS_IMMEDIATE(nr)) {
asm volatile(LOCK_PREFIX "orb %1,%0"
@@ -77,33 +62,17 @@ set_bit(long nr, volatile unsigned long *addr)
: : RLONG_ADDR(addr), "Ir" (nr) : "memory");
}
}
+#define arch_set_bit arch_set_bit
-/**
- * __set_bit - Set a bit in memory
- * @nr: the bit to set
- * @addr: the address to start counting from
- *
- * Unlike set_bit(), this function is non-atomic and may be reordered.
- * If it's called on the same region of memory simultaneously, the effect
- * may be that only one operation succeeds.
- */
-static __always_inline void __set_bit(long nr, volatile unsigned long *addr)
+static __always_inline void
+arch___set_bit(long nr, volatile unsigned long *addr)
{
asm volatile(__ASM_SIZE(bts) " %1,%0" : : ADDR, "Ir" (nr) : "memory");
}
+#define arch___set_bit arch___set_bit
-/**
- * clear_bit - Clears a bit in memory
- * @nr: Bit to clear
- * @addr: Address to start counting from
- *
- * clear_bit() is atomic and may not be reordered. However, it does
- * not contain a memory barrier, so if it is used for locking purposes,
- * you should call smp_mb__before_atomic() and/or smp_mb__after_atomic()
- * in order to ensure changes are visible on other processors.
- */
static __always_inline void
-clear_bit(long nr, volatile unsigned long *addr)
+arch_clear_bit(long nr, volatile unsigned long *addr)
{
if (IS_IMMEDIATE(nr)) {
asm volatile(LOCK_PREFIX "andb %1,%0"
@@ -114,27 +83,25 @@ clear_bit(long nr, volatile unsigned long *addr)
: : RLONG_ADDR(addr), "Ir" (nr) : "memory");
}
}
+#define arch_clear_bit arch_clear_bit
-/*
- * clear_bit_unlock - Clears a bit in memory
- * @nr: Bit to clear
- * @addr: Address to start counting from
- *
- * clear_bit() is atomic and implies release semantics before the memory
- * operation. It can be used for an unlock.
- */
-static __always_inline void clear_bit_unlock(long nr, volatile unsigned long *addr)
+static __always_inline void
+arch_clear_bit_unlock(long nr, volatile unsigned long *addr)
{
barrier();
- clear_bit(nr, addr);
+ arch_clear_bit(nr, addr);
}
+#define arch_clear_bit_unlock arch_clear_bit_unlock
-static __always_inline void __clear_bit(long nr, volatile unsigned long *addr)
+static __always_inline void
+arch___clear_bit(long nr, volatile unsigned long *addr)
{
asm volatile(__ASM_SIZE(btr) " %1,%0" : : ADDR, "Ir" (nr) : "memory");
}
+#define arch___clear_bit arch___clear_bit
-static __always_inline bool clear_bit_unlock_is_negative_byte(long nr, volatile unsigned long *addr)
+static __always_inline bool
+arch_clear_bit_unlock_is_negative_byte(long nr, volatile unsigned long *addr)
{
bool negative;
asm volatile(LOCK_PREFIX "andb %2,%1"
@@ -143,48 +110,25 @@ static __always_inline bool clear_bit_unlock_is_negative_byte(long nr, volatile
: "ir" ((char) ~(1 << nr)) : "memory");
return negative;
}
+#define arch_clear_bit_unlock_is_negative_byte \
+ arch_clear_bit_unlock_is_negative_byte
-// Let everybody know we have it
-#define clear_bit_unlock_is_negative_byte clear_bit_unlock_is_negative_byte
-
-/*
- * __clear_bit_unlock - Clears a bit in memory
- * @nr: Bit to clear
- * @addr: Address to start counting from
- *
- * __clear_bit() is non-atomic and implies release semantics before the memory
- * operation. It can be used for an unlock if no other CPUs can concurrently
- * modify other bits in the word.
- */
-static __always_inline void __clear_bit_unlock(long nr, volatile unsigned long *addr)
+static __always_inline void
+arch___clear_bit_unlock(long nr, volatile unsigned long *addr)
{
- __clear_bit(nr, addr);
+ arch___clear_bit(nr, addr);
}
+#define arch___clear_bit_unlock arch___clear_bit_unlock
-/**
- * __change_bit - Toggle a bit in memory
- * @nr: the bit to change
- * @addr: the address to start counting from
- *
- * Unlike change_bit(), this function is non-atomic and may be reordered.
- * If it's called on the same region of memory simultaneously, the effect
- * may be that only one operation succeeds.
- */
-static __always_inline void __change_bit(long nr, volatile unsigned long *addr)
+static __always_inline void
+arch___change_bit(long nr, volatile unsigned long *addr)
{
asm volatile(__ASM_SIZE(btc) " %1,%0" : : ADDR, "Ir" (nr) : "memory");
}
+#define arch___change_bit arch___change_bit
-/**
- * change_bit - Toggle a bit in memory
- * @nr: Bit to change
- * @addr: Address to start counting from
- *
- * change_bit() is atomic and may not be reordered.
- * Note that @nr may be almost arbitrarily large; this function is not
- * restricted to acting on a single-word quantity.
- */
-static __always_inline void change_bit(long nr, volatile unsigned long *addr)
+static __always_inline void
+arch_change_bit(long nr, volatile unsigned long *addr)
{
if (IS_IMMEDIATE(nr)) {
asm volatile(LOCK_PREFIX "xorb %1,%0"
@@ -195,43 +139,24 @@ static __always_inline void change_bit(long nr, volatile unsigned long *addr)
: : RLONG_ADDR(addr), "Ir" (nr) : "memory");
}
}
+#define arch_change_bit arch_change_bit
-/**
- * test_and_set_bit - Set a bit and return its old value
- * @nr: Bit to set
- * @addr: Address to count from
- *
- * This operation is atomic and cannot be reordered.
- * It also implies a memory barrier.
- */
-static __always_inline bool test_and_set_bit(long nr, volatile unsigned long *addr)
+static __always_inline bool
+arch_test_and_set_bit(long nr, volatile unsigned long *addr)
{
return GEN_BINARY_RMWcc(LOCK_PREFIX __ASM_SIZE(bts), *addr, c, "Ir", nr);
}
+#define arch_test_and_set_bit arch_test_and_set_bit
-/**
- * test_and_set_bit_lock - Set a bit and return its old value for lock
- * @nr: Bit to set
- * @addr: Address to count from
- *
- * This is the same as test_and_set_bit on x86.
- */
static __always_inline bool
-test_and_set_bit_lock(long nr, volatile unsigned long *addr)
+arch_test_and_set_bit_lock(long nr, volatile unsigned long *addr)
{
- return test_and_set_bit(nr, addr);
+ return arch_test_and_set_bit(nr, addr);
}
+#define arch_test_and_set_bit_lock arch_test_and_set_bit_lock
-/**
- * __test_and_set_bit - Set a bit and return its old value
- * @nr: Bit to set
- * @addr: Address to count from
- *
- * This operation is non-atomic and can be reordered.
- * If two examples of this operation race, one can appear to succeed
- * but actually fail. You must protect multiple accesses with a lock.
- */
-static __always_inline bool __test_and_set_bit(long nr, volatile unsigned long *addr)
+static __always_inline bool
+arch___test_and_set_bit(long nr, volatile unsigned long *addr)
{
bool oldbit;
@@ -241,37 +166,17 @@ static __always_inline bool __test_and_set_bit(long nr, volatile unsigned long *
: ADDR, "Ir" (nr) : "memory");
return oldbit;
}
+#define arch___test_and_set_bit arch___test_and_set_bit
-/**
- * test_and_clear_bit - Clear a bit and return its old value
- * @nr: Bit to clear
- * @addr: Address to count from
- *
- * This operation is atomic and cannot be reordered.
- * It also implies a memory barrier.
- */
-static __always_inline bool test_and_clear_bit(long nr, volatile unsigned long *addr)
+static __always_inline bool
+arch_test_and_clear_bit(long nr, volatile unsigned long *addr)
{
return GEN_BINARY_RMWcc(LOCK_PREFIX __ASM_SIZE(btr), *addr, c, "Ir", nr);
}
+#define arch_test_and_clear_bit arch_test_and_clear_bit
-/**
- * __test_and_clear_bit - Clear a bit and return its old value
- * @nr: Bit to clear
- * @addr: Address to count from
- *
- * This operation is non-atomic and can be reordered.
- * If two examples of this operation race, one can appear to succeed
- * but actually fail. You must protect multiple accesses with a lock.
- *
- * Note: the operation is performed atomically with respect to
- * the local CPU, but not other CPUs. Portable code should not
- * rely on this behaviour.
- * KVM relies on this behaviour on x86 for modifying memory that is also
- * accessed from a hypervisor on the same CPU if running in a VM: don't change
- * this without also updating arch/x86/kernel/kvm.c
- */
-static __always_inline bool __test_and_clear_bit(long nr, volatile unsigned long *addr)
+static __always_inline bool
+arch___test_and_clear_bit(long nr, volatile unsigned long *addr)
{
bool oldbit;
@@ -281,9 +186,10 @@ static __always_inline bool __test_and_clear_bit(long nr, volatile unsigned long
: ADDR, "Ir" (nr) : "memory");
return oldbit;
}
+#define arch___test_and_clear_bit arch___test_and_clear_bit
-/* WARNING: non atomic and it can be reordered! */
-static __always_inline bool __test_and_change_bit(long nr, volatile unsigned long *addr)
+static __always_inline bool
+arch___test_and_change_bit(long nr, volatile unsigned long *addr)
{
bool oldbit;
@@ -294,19 +200,14 @@ static __always_inline bool __test_and_change_bit(long nr, volatile unsigned lon
return oldbit;
}
+#define arch___test_and_change_bit arch___test_and_change_bit
-/**
- * test_and_change_bit - Change a bit and return its old value
- * @nr: Bit to change
- * @addr: Address to count from
- *
- * This operation is atomic and cannot be reordered.
- * It also implies a memory barrier.
- */
-static __always_inline bool test_and_change_bit(long nr, volatile unsigned long *addr)
+static __always_inline bool
+arch_test_and_change_bit(long nr, volatile unsigned long *addr)
{
return GEN_BINARY_RMWcc(LOCK_PREFIX __ASM_SIZE(btc), *addr, c, "Ir", nr);
}
+#define arch_test_and_change_bit arch_test_and_change_bit
static __always_inline bool constant_test_bit(long nr, const volatile unsigned long *addr)
{
@@ -326,16 +227,7 @@ static __always_inline bool variable_test_bit(long nr, volatile const unsigned l
return oldbit;
}
-#if 0 /* Fool kernel-doc since it doesn't do macros yet */
-/**
- * test_bit - Determine whether a bit is set
- * @nr: bit number to test
- * @addr: Address to start counting from
- */
-static bool test_bit(int nr, const volatile unsigned long *addr);
-#endif
-
-#define test_bit(nr, addr) \
+#define arch_test_bit(nr, addr) \
(__builtin_constant_p((nr)) \
? constant_test_bit((nr), (addr)) \
: variable_test_bit((nr), (addr)))
@@ -504,6 +396,8 @@ static __always_inline int fls64(__u64 x)
#include <asm-generic/bitops/const_hweight.h>
+#include <asm-generic/bitops-instrumented.h>
+
#include <asm-generic/bitops/le.h>
#include <asm-generic/bitops/ext2-atomic-setbit.h>
diff --git a/include/asm-generic/bitops-instrumented.h b/include/asm-generic/bitops-instrumented.h
new file mode 100644
index 000000000000..52140a5626c3
--- /dev/null
+++ b/include/asm-generic/bitops-instrumented.h
@@ -0,0 +1,327 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+/*
+ * This file provides wrappers with sanitizer instrumentation for bit
+ * operations.
+ *
+ * To use this functionality, an arch's bitops.h file needs to define each of
+ * the below bit operations with an arch_ prefix (e.g. arch_set_bit(),
+ * arch___set_bit(), etc.), #define each provided arch_ function, and include
+ * this file after their definitions. For undefined arch_ functions, it is
+ * assumed that they are provided via asm-generic/bitops, which are implicitly
+ * instrumented.
+ */
+#ifndef _ASM_GENERIC_BITOPS_INSTRUMENTED_H
+#define _ASM_GENERIC_BITOPS_INSTRUMENTED_H
+
+#include <linux/kasan-checks.h>
+
+#if !defined(BITOPS_INSTRUMENT_RANGE)
+/*
+ * This may be defined by an arch's bitops.h, in case bitops do not operate on
+ * single bytes only. The default version here is conservative and assumes that
+ * bitops operate only on the byte with the target bit.
+ */
+#define BITOPS_INSTRUMENT_RANGE(addr, nr) \
+ (const volatile char *)(addr) + ((nr) / BITS_PER_BYTE), 1
+#endif
+
+#if defined(arch_set_bit)
+/**
+ * set_bit - Atomically set a bit in memory
+ * @nr: the bit to set
+ * @addr: the address to start counting from
+ *
+ * This function is atomic and may not be reordered. See __set_bit()
+ * if you do not require the atomic guarantees.
+ *
+ * Note: there are no guarantees that this function will not be reordered
+ * on non x86 architectures, so if you are writing portable code,
+ * make sure not to rely on its reordering guarantees.
+ *
+ * Note that @nr may be almost arbitrarily large; this function is not
+ * restricted to acting on a single-word quantity.
+ */
+static inline void set_bit(long nr, volatile unsigned long *addr)
+{
+ kasan_check_write(BITOPS_INSTRUMENT_RANGE(addr, nr));
+ arch_set_bit(nr, addr);
+}
+#endif
+
+#if defined(arch___set_bit)
+/**
+ * __set_bit - Set a bit in memory
+ * @nr: the bit to set
+ * @addr: the address to start counting from
+ *
+ * Unlike set_bit(), this function is non-atomic and may be reordered.
+ * If it's called on the same region of memory simultaneously, the effect
+ * may be that only one operation succeeds.
+ */
+static inline void __set_bit(long nr, volatile unsigned long *addr)
+{
+ kasan_check_write(BITOPS_INSTRUMENT_RANGE(addr, nr));
+ arch___set_bit(nr, addr);
+}
+#endif
+
+#if defined(arch_clear_bit)
+/**
+ * clear_bit - Clears a bit in memory
+ * @nr: Bit to clear
+ * @addr: Address to start counting from
+ *
+ * clear_bit() is atomic and may not be reordered. However, it does
+ * not contain a memory barrier, so if it is used for locking purposes,
+ * you should call smp_mb__before_atomic() and/or smp_mb__after_atomic()
+ * in order to ensure changes are visible on other processors.
+ */
+static inline void clear_bit(long nr, volatile unsigned long *addr)
+{
+ kasan_check_write(BITOPS_INSTRUMENT_RANGE(addr, nr));
+ arch_clear_bit(nr, addr);
+}
+#endif
+
+#if defined(arch___clear_bit)
+/**
+ * __clear_bit - Clears a bit in memory
+ * @nr: the bit to clear
+ * @addr: the address to start counting from
+ *
+ * Unlike clear_bit(), this function is non-atomic and may be reordered.
+ * If it's called on the same region of memory simultaneously, the effect
+ * may be that only one operation succeeds.
+ */
+static inline void __clear_bit(long nr, volatile unsigned long *addr)
+{
+ kasan_check_write(BITOPS_INSTRUMENT_RANGE(addr, nr));
+ arch___clear_bit(nr, addr);
+}
+#endif
+
+#if defined(arch_clear_bit_unlock)
+/**
+ * clear_bit_unlock - Clears a bit in memory
+ * @nr: Bit to clear
+ * @addr: Address to start counting from
+ *
+ * clear_bit_unlock() is atomic and implies release semantics before the memory
+ * operation. It can be used for an unlock.
+ */
+static inline void clear_bit_unlock(long nr, volatile unsigned long *addr)
+{
+ kasan_check_write(BITOPS_INSTRUMENT_RANGE(addr, nr));
+ arch_clear_bit_unlock(nr, addr);
+}
+#endif
+
+#if defined(arch___clear_bit_unlock)
+/**
+ * __clear_bit_unlock - Clears a bit in memory
+ * @nr: Bit to clear
+ * @addr: Address to start counting from
+ *
+ * __clear_bit_unlock() is non-atomic and implies release semantics before the
+ * memory operation. It can be used for an unlock if no other CPUs can
+ * concurrently modify other bits in the word.
+ */
+static inline void __clear_bit_unlock(long nr, volatile unsigned long *addr)
+{
+ kasan_check_write(BITOPS_INSTRUMENT_RANGE(addr, nr));
+ arch___clear_bit_unlock(nr, addr);
+}
+#endif
+
+#if defined(arch_change_bit)
+/**
+ * change_bit - Toggle a bit in memory
+ * @nr: Bit to change
+ * @addr: Address to start counting from
+ *
+ * change_bit() is atomic and may not be reordered.
+ * Note that @nr may be almost arbitrarily large; this function is not
+ * restricted to acting on a single-word quantity.
+ */
+static inline void change_bit(long nr, volatile unsigned long *addr)
+{
+ kasan_check_write(BITOPS_INSTRUMENT_RANGE(addr, nr));
+ arch_change_bit(nr, addr);
+}
+#endif
+
+#if defined(arch___change_bit)
+/**
+ * __change_bit - Toggle a bit in memory
+ * @nr: the bit to change
+ * @addr: the address to start counting from
+ *
+ * Unlike change_bit(), this function is non-atomic and may be reordered.
+ * If it's called on the same region of memory simultaneously, the effect
+ * may be that only one operation succeeds.
+ */
+static inline void __change_bit(long nr, volatile unsigned long *addr)
+{
+ kasan_check_write(BITOPS_INSTRUMENT_RANGE(addr, nr));
+ arch___change_bit(nr, addr);
+}
+#endif
+
+#if defined(arch_test_and_set_bit)
+/**
+ * test_and_set_bit - Set a bit and return its old value
+ * @nr: Bit to set
+ * @addr: Address to count from
+ *
+ * This operation is atomic and cannot be reordered.
+ * It also implies a memory barrier.
+ */
+static inline bool test_and_set_bit(long nr, volatile unsigned long *addr)
+{
+ kasan_check_write(BITOPS_INSTRUMENT_RANGE(addr, nr));
+ return arch_test_and_set_bit(nr, addr);
+}
+#endif
+
+#if defined(arch___test_and_set_bit)
+/**
+ * __test_and_set_bit - Set a bit and return its old value
+ * @nr: Bit to set
+ * @addr: Address to count from
+ *
+ * This operation is non-atomic and can be reordered.
+ * If two examples of this operation race, one can appear to succeed
+ * but actually fail. You must protect multiple accesses with a lock.
+ */
+static inline bool __test_and_set_bit(long nr, volatile unsigned long *addr)
+{
+ kasan_check_write(BITOPS_INSTRUMENT_RANGE(addr, nr));
+ return arch___test_and_set_bit(nr, addr);
+}
+#endif
+
+#if defined(arch_test_and_set_bit_lock)
+/**
+ * test_and_set_bit_lock - Set a bit and return its old value, for lock
+ * @nr: Bit to set
+ * @addr: Address to count from
+ *
+ * This operation is atomic and provides acquire barrier semantics if
+ * the returned value is 0.
+ * It can be used to implement bit locks.
+ */
+static inline bool test_and_set_bit_lock(long nr, volatile unsigned long *addr)
+{
+ kasan_check_write(BITOPS_INSTRUMENT_RANGE(addr, nr));
+ return arch_test_and_set_bit_lock(nr, addr);
+}
+#endif
+
+#if defined(arch_test_and_clear_bit)
+/**
+ * test_and_clear_bit - Clear a bit and return its old value
+ * @nr: Bit to clear
+ * @addr: Address to count from
+ *
+ * This operation is atomic and cannot be reordered.
+ * It also implies a memory barrier.
+ */
+static inline bool test_and_clear_bit(long nr, volatile unsigned long *addr)
+{
+ kasan_check_write(BITOPS_INSTRUMENT_RANGE(addr, nr));
+ return arch_test_and_clear_bit(nr, addr);
+}
+#endif
+
+#if defined(arch___test_and_clear_bit)
+/**
+ * __test_and_clear_bit - Clear a bit and return its old value
+ * @nr: Bit to clear
+ * @addr: Address to count from
+ *
+ * This operation is non-atomic and can be reordered.
+ * If two examples of this operation race, one can appear to succeed
+ * but actually fail. You must protect multiple accesses with a lock.
+ *
+ * Note: the operation is performed atomically with respect to
+ * the local CPU, but not other CPUs. Portable code should not
+ * rely on this behaviour.
+ * KVM relies on this behaviour on x86 for modifying memory that is also
+ * accessed from a hypervisor on the same CPU if running in a VM: don't change
+ * this without also updating arch/x86/kernel/kvm.c
+ */
+static inline bool __test_and_clear_bit(long nr, volatile unsigned long *addr)
+{
+ kasan_check_write(BITOPS_INSTRUMENT_RANGE(addr, nr));
+ return arch___test_and_clear_bit(nr, addr);
+}
+#endif
+
+#if defined(arch_test_and_change_bit)
+/**
+ * test_and_change_bit - Change a bit and return its old value
+ * @nr: Bit to change
+ * @addr: Address to count from
+ *
+ * This operation is atomic and cannot be reordered.
+ * It also implies a memory barrier.
+ */
+static inline bool test_and_change_bit(long nr, volatile unsigned long *addr)
+{
+ kasan_check_write(BITOPS_INSTRUMENT_RANGE(addr, nr));
+ return arch_test_and_change_bit(nr, addr);
+}
+#endif
+
+#if defined(arch___test_and_change_bit)
+/**
+ * __test_and_change_bit - Change a bit and return its old value
+ * @nr: Bit to change
+ * @addr: Address to count from
+ *
+ * This operation is non-atomic and can be reordered.
+ * If two examples of this operation race, one can appear to succeed
+ * but actually fail. You must protect multiple accesses with a lock.
+ */
+static inline bool __test_and_change_bit(long nr, volatile unsigned long *addr)
+{
+ kasan_check_write(BITOPS_INSTRUMENT_RANGE(addr, nr));
+ return arch___test_and_change_bit(nr, addr);
+}
+#endif
+
+#if defined(arch_test_bit)
+/**
+ * test_bit - Determine whether a bit is set
+ * @nr: bit number to test
+ * @addr: Address to start counting from
+ */
+static inline bool test_bit(long nr, const volatile unsigned long *addr)
+{
+ kasan_check_read(BITOPS_INSTRUMENT_RANGE(addr, nr));
+ return arch_test_bit(nr, addr);
+}
+#endif
+
+#if defined(arch_clear_bit_unlock_is_negative_byte)
+/**
+ * clear_bit_unlock_is_negative_byte - Clear a bit in memory and test if bottom
+ * byte is negative, for unlock.
+ * @nr: the bit to clear
+ * @addr: the address to start counting from
+ *
+ * This is a bit of a one-trick-pony for the filemap code, which clears
+ * PG_locked and tests PG_waiters,
+ */
+static inline bool
+clear_bit_unlock_is_negative_byte(long nr, volatile unsigned long *addr)
+{
+ kasan_check_write(BITOPS_INSTRUMENT_RANGE(addr, nr));
+ return arch_clear_bit_unlock_is_negative_byte(nr, addr);
+}
+/* Let everybody know we have it. */
+#define clear_bit_unlock_is_negative_byte clear_bit_unlock_is_negative_byte
+#endif
+
+#endif /* _ASM_GENERIC_BITOPS_INSTRUMENTED_H */
--
2.22.0.rc1.257.g3120a18244-goog
^ permalink raw reply related
* Re: [PATCH 3/3] asm-generic, x86: Add bitops instrumentation for KASAN
From: Mark Rutland @ 2019-05-28 16:50 UTC (permalink / raw)
To: Marco Elver
Cc: peterz, aryabinin, dvyukov, glider, andreyknvl, corbet, tglx,
mingo, bp, hpa, x86, arnd, jpoimboe, linux-doc, linux-kernel,
linux-arch, kasan-dev
In-Reply-To: <20190528163258.260144-3-elver@google.com>
On Tue, May 28, 2019 at 06:32:58PM +0200, Marco Elver wrote:
> This adds a new header to asm-generic to allow optionally instrumenting
> architecture-specific asm implementations of bitops.
>
> This change includes the required change for x86 as reference and
> changes the kernel API doc to point to bitops-instrumented.h instead.
> Rationale: the functions in x86's bitops.h are no longer the kernel API
> functions, but instead the arch_ prefixed functions, which are then
> instrumented via bitops-instrumented.h.
>
> Other architectures can similarly add support for asm implementations of
> bitops.
>
> The documentation text has been copied/moved, and *no* changes to it
> have been made in this patch.
>
> Tested: using lib/test_kasan with bitops tests (pre-requisite patch).
>
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=198439
> Signed-off-by: Marco Elver <elver@google.com>
> ---
> Documentation/core-api/kernel-api.rst | 2 +-
> arch/x86/include/asm/bitops.h | 210 ++++----------
> include/asm-generic/bitops-instrumented.h | 327 ++++++++++++++++++++++
> 3 files changed, 380 insertions(+), 159 deletions(-)
> create mode 100644 include/asm-generic/bitops-instrumented.h
[...]
> +#if !defined(BITOPS_INSTRUMENT_RANGE)
> +/*
> + * This may be defined by an arch's bitops.h, in case bitops do not operate on
> + * single bytes only. The default version here is conservative and assumes that
> + * bitops operate only on the byte with the target bit.
> + */
> +#define BITOPS_INSTRUMENT_RANGE(addr, nr) \
> + (const volatile char *)(addr) + ((nr) / BITS_PER_BYTE), 1
> +#endif
I was under the impression that logically, all the bitops operated on
the entire long the bit happend to be contained in, so checking the
entire long would make more sense to me.
FWIW, arm64's atomic bit ops are all implemented atop of atomic_long_*
functions, which are instrumented, and always checks at the granularity
of a long. I haven't seen splats from that when fuzzing with Syzkaller.
Are you seeing bugs without this?
Thanks,
Mark.
^ permalink raw reply
* Re: [PATCH 1/3] lib/test_kasan: Add bitops tests
From: Mark Rutland @ 2019-05-28 16:50 UTC (permalink / raw)
To: Marco Elver
Cc: peterz, aryabinin, dvyukov, glider, andreyknvl, corbet, tglx,
mingo, bp, hpa, x86, arnd, jpoimboe, linux-doc, linux-kernel,
linux-arch, kasan-dev
In-Reply-To: <20190528163258.260144-1-elver@google.com>
Hi,
On Tue, May 28, 2019 at 06:32:56PM +0200, Marco Elver wrote:
> +static noinline void __init kasan_bitops(void)
> +{
> + long bits = 0;
> + const long bit = sizeof(bits) * 8;
You can use BITS_PER_LONG here.
Thanks,
Mark.
^ permalink raw reply
* Re: [PATCH 2/3] tools/objtool: add kasan_check_* to uaccess whitelist
From: Peter Zijlstra @ 2019-05-28 17:19 UTC (permalink / raw)
To: Marco Elver
Cc: aryabinin, dvyukov, glider, andreyknvl, corbet, tglx, mingo, bp,
hpa, x86, arnd, jpoimboe, linux-doc, linux-kernel, linux-arch,
kasan-dev
In-Reply-To: <20190528163258.260144-2-elver@google.com>
On Tue, May 28, 2019 at 06:32:57PM +0200, Marco Elver wrote:
> This is a pre-requisite for enabling bitops instrumentation. Some bitops
> may safely be used with instrumentation in uaccess regions.
>
> For example, on x86, `test_bit` is used to test a CPU-feature in a
> uaccess region: arch/x86/ia32/ia32_signal.c:361
That one can easily be moved out of the uaccess region. Any else?
> Signed-off-by: Marco Elver <elver@google.com>
> ---
> tools/objtool/check.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/tools/objtool/check.c b/tools/objtool/check.c
> index 172f99195726..eff0e5209402 100644
> --- a/tools/objtool/check.c
> +++ b/tools/objtool/check.c
> @@ -443,6 +443,8 @@ static void add_ignores(struct objtool_file *file)
> static const char *uaccess_safe_builtin[] = {
> /* KASAN */
> "kasan_report",
> + "kasan_check_read",
> + "kasan_check_write",
> "check_memory_region",
> /* KASAN out-of-line */
> "__asan_loadN_noabort",
> --
> 2.22.0.rc1.257.g3120a18244-goog
>
^ permalink raw reply
* Re: [PATCH RFC 0/5] Remove some notrace RCU APIs
From: Joel Fernandes @ 2019-05-28 19:00 UTC (permalink / raw)
To: Paul E. McKenney
Cc: Steven Rostedt, linux-kernel, Benjamin Herrenschmidt, Ingo Molnar,
Jonathan Corbet, Josh Triplett, kvm-ppc, Lai Jiangshan, linux-doc,
linuxppc-dev, Mathieu Desnoyers, Michael Ellerman, Miguel Ojeda,
Paul Mackerras, rcu
In-Reply-To: <20190528122447.GS28207@linux.ibm.com>
On Tue, May 28, 2019 at 05:24:47AM -0700, Paul E. McKenney wrote:
> On Sat, May 25, 2019 at 02:14:07PM -0400, Joel Fernandes wrote:
> > On Sat, May 25, 2019 at 08:50:35AM -0700, Paul E. McKenney wrote:
> > > On Sat, May 25, 2019 at 10:19:54AM -0400, Joel Fernandes wrote:
> > > > On Sat, May 25, 2019 at 07:08:26AM -0400, Steven Rostedt wrote:
> > > > > On Sat, 25 May 2019 04:14:44 -0400
> > > > > Joel Fernandes <joel@joelfernandes.org> wrote:
> > > > >
> > > > > > > I guess the difference between the _raw_notrace and just _raw variants
> > > > > > > is that _notrace ones do a rcu_check_sparse(). Don't we want to keep
> > > > > > > that check?
> > > > > >
> > > > > > This is true.
> > > > > >
> > > > > > Since the users of _raw_notrace are very few, is it worth keeping this API
> > > > > > just for sparse checking? The API naming is also confusing. I was expecting
> > > > > > _raw_notrace to do fewer checks than _raw, instead of more. Honestly, I just
> > > > > > want to nuke _raw_notrace as done in this series and later we can introduce a
> > > > > > sparse checking version of _raw if need-be. The other option could be to
> > > > > > always do sparse checking for _raw however that used to be the case and got
> > > > > > changed in http://lists.infradead.org/pipermail/linux-afs/2016-July/001016.html
> > > > >
> > > > > What if we just rename _raw to _raw_nocheck, and _raw_notrace to _raw ?
> > > >
> > > > That would also mean changing 160 usages of _raw to _raw_nocheck in the
> > > > kernel :-/.
> > > >
> > > > The tracing usage of _raw_notrace is only like 2 or 3 users. Can we just call
> > > > rcu_check_sparse directly in the calling code for those and eliminate the APIs?
> > > >
> > > > I wonder what Paul thinks about the matter as well.
> > >
> > > My thought is that it is likely that a goodly number of the current uses
> > > of _raw should really be some form of _check, with lockdep expressions
> > > spelled out. Not that working out what exactly those lockdep expressions
> > > should be is necessarily a trivial undertaking. ;-)
> >
> > Yes, currently where I am a bit stuck is the rcu_dereference_raw()
> > cannot possibly know what SRCU domain it is under, so lockdep cannot check if
> > an SRCU lock is held without the user also passing along the SRCU domain. I
> > am trying to change lockdep to see if it can check if *any* srcu domain lock
> > is held (regardless of which one) and complain if none are. This is at least
> > better than no check at all.
> >
> > However, I think it gets tricky for mutexes. If you have something like:
> > mutex_lock(some_mutex);
> > p = rcu_dereference_raw(gp);
> > mutex_unlock(some_mutex);
> >
> > This might be a perfectly valid invocation of _raw, however my checks (patch
> > is still cooking) trigger a lockdep warning becase _raw cannot know that this
> > is Ok. lockdep thinks it is not in a reader section. This then gets into the
> > territory of a new rcu_derference_raw_protected(gp, assert_held(some_mutex))
> > which sucks because its yet another API. To circumvent this issue, can we
> > just have callers of rcu_dereference_raw ensure that they call
> > rcu_read_lock() if they are protecting dereferences by a mutex? That would
> > make things a lot easier and also may be Ok since rcu_read_lock is quite
> > cheap.
>
> Why not just rcu_dereference_protected(lockdep_is_held(some_mutex))?
> The API is already there, and no need for spurious readers.
Hmm, so I gave a bad example, here is a better example:
fib_get_table calls hlist_for_each_entry_rcu()
hlist_for_each_entry_rcu calls rcu_dereference_raw().
This is perfectly Ok to be called under rtnl_mutex. However rcu_dererence_raw
in hlist_for_each_entry_rcu has no way of knowing that the rtnl_mutex held is
sufficient for the protection since it is not directly called by the caller.
I am almost sure I saw other examples of rcu_dereference_raw being called
this way as well.
I was trying to make an "automatic" lockdep check for all this, but it is
quite hard to do so without passing down lockdep experessions down a call
chain thus complicating all such callchains.
Further I don't think code can trivially be converted from
rcu_dereference_raw to rcu_dereference_protected even if the protection being
offered is known, since the former does not do sparse checking and the latter
might trigger false sparse checks in case the pointer in concern is protected
both by RCU and non-RCU methods. I believe this is why you removed sparse
checking from rcu_dereference_raw as well:
http://lists.infradead.org/pipermail/linux-afs/2016-July/001016.html
> > > That aside, if we are going to change the name of an API that is
> > > used 160 places throughout the tree, we would need to have a pretty
> > > good justification. Without such a justification, it will just look
> > > like pointless churn to the various developers and maintainers on the
> > > receiving end of the patches.
> >
> > Actually, the API name change is not something I want to do, it is Steven
> > suggestion. My suggestion is let us just delete _raw_notrace and just use the
> > _raw API for tracing, since _raw doesn't do any tracing anyway. Steve pointed
> > that _raw_notrace does sparse checking unlike _raw, but I think that isn't an
> > issue since _raw doesn't do such checking at the moment anyway.. (if possible
> > check my cover letter again for details/motivation of this series).
>
> Understood, but regardless of who suggested it, if we are to go through
> with it, good justification will be required. ;-)
Ok ;-). About the names of the APIs, I thought of leaving rcu_dereference_raw
and its callers intact, and just rename:
* hlist_for_each_entry_rcu_notrace
* rcu_dereference_raw_notrace
to:
* hlist_for_each_entry_rcu_sparse
* rcu_dereference_raw_sparse
The _sparse would stand for "sparse checking". However I am open to better
names..
Such renaming would avoid confusion and keep the fact about sparse checking
less ambiguous.
thanks!
- Joel
^ permalink raw reply
* Re: [PATCH v10 01/12] MODSIGN: Export module signature definitions
From: Thiago Jung Bauermann @ 2019-05-28 19:03 UTC (permalink / raw)
To: Mimi Zohar
Cc: linux-integrity, linux-security-module, keyrings, linux-crypto,
linuxppc-dev, linux-doc, linux-kernel, Dmitry Kasatkin,
James Morris, Serge E. Hallyn, David Howells, David Woodhouse,
Jessica Yu, Herbert Xu, David S. Miller, Jonathan Corbet,
AKASHI, Takahiro
In-Reply-To: <1557416528.10635.62.camel@linux.ibm.com>
Mimi Zohar <zohar@linux.ibm.com> writes:
> On Thu, 2019-04-18 at 00:51 -0300, Thiago Jung Bauermann wrote:
>> IMA will use the module_signature format for append signatures, so export
>> the relevant definitions and factor out the code which verifies that the
>> appended signature trailer is valid.
>>
>> Also, create a CONFIG_MODULE_SIG_FORMAT option so that IMA can select it
>> and be able to use mod_check_sig() without having to depend on either
>> CONFIG_MODULE_SIG or CONFIG_MODULES.
>>
>> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
>> Cc: Jessica Yu <jeyu@kernel.org>
>
> Just a couple minor questions/comments below.
>
> Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
Thanks for your review and your comments!
>> diff --git a/init/Kconfig b/init/Kconfig
>> index 4592bf7997c0..a71019553ee1 100644
>> --- a/init/Kconfig
>> +++ b/init/Kconfig
>> @@ -1906,7 +1906,7 @@ config MODULE_SRCVERSION_ALL
>> config MODULE_SIG
>> bool "Module signature verification"
>> depends on MODULES
>> - select SYSTEM_DATA_VERIFICATION
>> + select MODULE_SIG_FORMAT
>> help
>> Check modules for valid signatures upon load: the signature
>> is simply appended to the module. For more information see
>> @@ -2036,6 +2036,10 @@ config TRIM_UNUSED_KSYMS
>>
>> endif # MODULES
>>
>> +config MODULE_SIG_FORMAT
>> + def_bool n
>> + select SYSTEM_DATA_VERIFICATION
>
> Normally Kconfigs, in the same file, are defined before they are used.
> I'm not sure if that is required or just a convention.
I think it's a convention, because it seemed to work in the current way.
For the next version I moved the config MODULE_SIG_FORMAT definition to
just before "menuconfig MODULES"
>> diff --git a/kernel/module_signature.c b/kernel/module_signature.c
>> new file mode 100644
>> index 000000000000..6d5e59f27f55
>> --- /dev/null
>> +++ b/kernel/module_signature.c
>> @@ -0,0 +1,45 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * Module signature checker
>> + *
>> + * Copyright (C) 2012 Red Hat, Inc. All Rights Reserved.
>> + * Written by David Howells (dhowells@redhat.com)
>> + */
>> +
>> +#include <linux/errno.h>
>> +#include <linux/printk.h>
>> +#include <linux/module_signature.h>
>> +#include <asm/byteorder.h>
>> +
>> +/**
>> + * mod_check_sig - check that the given signature is sane
>> + *
>> + * @ms: Signature to check.
>> + * @file_len: Size of the file to which @ms is appended.
>
> "name" is missing.
Fixed.
--
Thiago Jung Bauermann
IBM Linux Technology Center
^ permalink raw reply
* Re: [PATCH v10 11/12] ima: Define ima-modsig template
From: Thiago Jung Bauermann @ 2019-05-28 19:09 UTC (permalink / raw)
To: Mimi Zohar
Cc: linux-integrity, linux-security-module, keyrings, linux-crypto,
linuxppc-dev, linux-doc, linux-kernel, Dmitry Kasatkin,
James Morris, Serge E. Hallyn, David Howells, David Woodhouse,
Jessica Yu, Herbert Xu, David S. Miller, Jonathan Corbet,
AKASHI, Takahiro
In-Reply-To: <1557442889.10635.88.camel@linux.ibm.com>
Mimi Zohar <zohar@linux.ibm.com> writes:
> On Thu, 2019-04-18 at 00:51 -0300, Thiago Jung Bauermann wrote:
>> Define new "d-modsig" template field which holds the digest that is
>> expected to match the one contained in the modsig, and also new "modsig"
>> template field which holds the appended file signature.
>>
>> Add a new "ima-modsig" defined template descriptor with the new fields as
>> well as the ones from the "ima-sig" descriptor.
>>
>> Change ima_store_measurement() to accept a struct modsig * argument so that
>> it can be passed along to the templates via struct ima_event_data.
>>
>> Suggested-by: Mimi Zohar <zohar@linux.ibm.com>
>> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
>
> Thanks, Roberto. Just some thoughts inline below.
>
> Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
Thanks!
>> +/*
>> + * Validating the appended signature included in the measurement list requires
>> + * the file hash calculated without the appended signature (i.e., the 'd-modsig'
>> + * field). Therefore, notify the user if they have the 'modsig' field but not
>> + * the 'd-modsig' field in the template.
>> + */
>> +static void check_current_template_modsig(void)
>> +{
>> +#define MSG "template with 'modsig' field also needs 'd-modsig' field\n"
>> + struct ima_template_desc *template;
>> + bool has_modsig, has_dmodsig;
>> + static bool checked;
>> + int i;
>> +
>> + /* We only need to notify the user once. */
>> + if (checked)
>> + return;
>> +
>> + has_modsig = has_dmodsig = false;
>> + template = ima_template_desc_current();
>> + for (i = 0; i < template->num_fields; i++) {
>> + if (!strcmp(template->fields[i]->field_id, "modsig"))
>> + has_modsig = true;
>> + else if (!strcmp(template->fields[i]->field_id, "d-modsig"))
>> + has_dmodsig = true;
>> + }
>> +
>> + if (has_modsig && !has_dmodsig)
>> + pr_notice(MSG);
>> +
>> + checked = true;
>> +#undef MSG
>> +}
>> +
>
> There was some recent discussion about supporting per IMA policy rule
> template formats. This feature will allow just the kexec kernel image
> to require ima-modsig. When per policy rule template formats support
> is upstreamed, this function will need to be updated.
Indeed. Thanks for the clarification. For the next iteration I rebased
on top of Matthew Garret's "IMA: Allow profiles to define the desired
IMA template" patch. I'm currently adapting this check accordingly.
>> @@ -389,3 +425,25 @@ int ima_eventsig_init(struct ima_event_data *event_data,
>> return ima_write_template_field_data(xattr_value, event_data->xattr_len,
>> DATA_FMT_HEX, field_data);
>> }
>> +
>> +int ima_eventmodsig_init(struct ima_event_data *event_data,
>> + struct ima_field_data *field_data)
>> +{
>> + const void *data;
>> + u32 data_len;
>> + int rc;
>> +
>> + if (!event_data->modsig)
>> + return 0;
>> +
>> + /*
>> + * The xattr_value for IMA_MODSIG is a runtime structure containing
>> + * pointers. Get its raw data instead.
>> + */
>
> "xattr_value"? The comment needs some clarification.
Oops, forgot to update this comment. This is the new version:
/*
* modsig is a runtime structure containing pointers. Get its raw data
* instead.
*/
--
Thiago Jung Bauermann
IBM Linux Technology Center
^ permalink raw reply
* Re: [PATCH v10 12/12] ima: Store the measurement again when appraising a modsig
From: Thiago Jung Bauermann @ 2019-05-28 19:14 UTC (permalink / raw)
To: Mimi Zohar
Cc: linux-integrity, linux-security-module, keyrings, linux-crypto,
linuxppc-dev, linux-doc, linux-kernel, Dmitry Kasatkin,
James Morris, Serge E. Hallyn, David Howells, David Woodhouse,
Jessica Yu, Herbert Xu, David S. Miller, Jonathan Corbet,
AKASHI, Takahiro
In-Reply-To: <1559052560.4090.14.camel@linux.ibm.com>
Mimi Zohar <zohar@linux.ibm.com> writes:
> Hi Thiago,
>
> On Thu, 2019-04-18 at 00:51 -0300, Thiago Jung Bauermann wrote:
>> If the IMA template contains the "modsig" or "d-modsig" field, then the
>> modsig should be added to the measurement list when the file is appraised.
>>
>> And that is what normally happens, but if a measurement rule caused a file
>> containing a modsig to be measured before a different rule causes it to be
>> appraised, the resulting measurement entry will not contain the modsig
>> because it is only fetched during appraisal. When the appraisal rule
>> triggers, it won't store a new measurement containing the modsig because
>> the file was already measured.
>>
>> We need to detect that situation and store an additional measurement with
>> the modsig. This is done by adding an IMA_MEASURE action flag if we read a
>> modsig and the IMA template contains a modsig field.
>
> With the new per policy rule "template" support being added, this
> patch needs to be modified so that the per policy "template" format is
> checked. ima_template_has_modsig() should be called with the
> template_desc being used.
Right. Thanks for point out what needs to be done. After rebasing on top
of Matthew Garret's "IMA: Allow profiles to define the desired IMA
template" patch I changed ima_template_has_modsig() to check the
template_desc obtained from process_measurement().
--
Thiago Jung Bauermann
IBM Linux Technology Center
^ permalink raw reply
* Re: [PATCH v10 09/12] ima: Implement support for module-style appended signatures
From: Thiago Jung Bauermann @ 2019-05-28 19:23 UTC (permalink / raw)
To: Mimi Zohar
Cc: linux-integrity, linux-security-module, keyrings, linux-crypto,
linuxppc-dev, linux-doc, linux-kernel, Dmitry Kasatkin,
James Morris, Serge E. Hallyn, David Howells, David Woodhouse,
Jessica Yu, Herbert Xu, David S. Miller, Jonathan Corbet,
AKASHI, Takahiro
In-Reply-To: <1557442868.10635.87.camel@linux.ibm.com>
Mimi Zohar <zohar@linux.ibm.com> writes:
> Hi Thiago,
>
>> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
>> index fca7a3f23321..a7a20a8c15c1 100644
>> --- a/security/integrity/ima/ima_policy.c
>> +++ b/security/integrity/ima/ima_policy.c
>> @@ -1144,6 +1144,12 @@ void ima_delete_rules(void)
>> }
>> }
>>
>> +#define __ima_hook_stringify(str) (#str),
>> +
>> +const char *const func_tokens[] = {
>> + __ima_hooks(__ima_hook_stringify)
>> +};
>> +
>> #ifdef CONFIG_IMA_READ_POLICY
>> enum {
>> mask_exec = 0, mask_write, mask_read, mask_append
>> @@ -1156,12 +1162,6 @@ static const char *const mask_tokens[] = {
>> "MAY_APPEND"
>> };
>>
>> -#define __ima_hook_stringify(str) (#str),
>> -
>> -static const char *const func_tokens[] = {
>> - __ima_hooks(__ima_hook_stringify)
>> -};
>> -
>> void *ima_policy_start(struct seq_file *m, loff_t *pos)
>> {
>> loff_t l = *pos;
>
> Is moving this something left over from previous versions or there is
> a need for this change?
Well, it's not a strong need, but it's still relevant in the current
version. I use func_tokens in ima_read_modsig() in order to be able to
mention the hook name in mod_check_sig()'s error message:
In ima_read_modsig():
rc = mod_check_sig(sig, buf_len, func_tokens[func]);
And in mod_check_sig():
pr_err("%s: Module is not signed with expected PKCS#7 message\n",
name);
If you think it's not worth it to expose func_tokens, I can make
ima_read_modsig() pass a more generic const string such as "IMA modsig"
for example.
> Other than this, the patch looks good.
Nice!
--
Thiago Jung Bauermann
IBM Linux Technology Center
^ permalink raw reply
* Re: [PATCH v10 09/12] ima: Implement support for module-style appended signatures
From: Thiago Jung Bauermann @ 2019-05-28 19:27 UTC (permalink / raw)
To: Mimi Zohar
Cc: linux-integrity, linux-security-module, keyrings, linux-crypto,
linuxppc-dev, linux-doc, linux-kernel, Dmitry Kasatkin,
James Morris, Serge E. Hallyn, David Howells, David Woodhouse,
Jessica Yu, Herbert Xu, David S. Miller, Jonathan Corbet,
AKASHI, Takahiro
In-Reply-To: <1557835765.4139.9.camel@linux.ibm.com>
Mimi Zohar <zohar@linux.ibm.com> writes:
> Hi Thiago,
>
> On Thu, 2019-04-18 at 00:51 -0300, Thiago Jung Bauermann wrote:
>>
>> @@ -326,6 +356,10 @@ int ima_appraise_measurement(enum ima_hooks func,
>> case INTEGRITY_UNKNOWN:
>> break;
>> case INTEGRITY_NOXATTRS:/* No EVM protected xattrs. */
>> +/* It's fine not to have xattrs when using a modsig. */
>> +if (try_modsig)
>> +break;
>> +/* fall through */
>> case INTEGRITY_NOLABEL:/* No security.evm xattr. */
>> cause = "missing-HMAC";
>> goto out;
>> @@ -340,6 +374,14 @@ int ima_appraise_measurement(enum ima_hooks func,
>> rc = xattr_verify(func, iint, xattr_value, xattr_len, &status,
>> &cause);
>>
>> +/*
>> + * If we have a modsig and either no imasig or the imasig's key isn't
>> + * known, then try verifying the modsig.
>> + */
>> +if (status != INTEGRITY_PASS && try_modsig &&
>> + (!xattr_value || rc == -ENOKEY))
>> +rc = modsig_verify(func, modsig, &status, &cause);
>
> EVM protects other security xattrs, not just security.ima, if they
> exist. As a result, evm_verifyxattr() could pass based on the other
> security xattrs.
Indeed! It doesn't make sense to test for status != INTEGRITY_PASS here.
Not sure what I was thinking. Thanks for spotting it. With your other
comments about this if clause, this code now reads:
/*
* If we have a modsig and either no imasig or the imasig's key isn't
* known, then try verifying the modsig.
*/
if (try_modsig &&
(!xattr_value || xattr_value->type == IMA_XATTR_DIGEST_NG ||
rc == -ENOKEY))
rc = modsig_verify(func, modsig, &status, &cause);
--
Thiago Jung Bauermann
IBM Linux Technology Center
^ permalink raw reply
* Re: [PATCH RFC 0/5] Remove some notrace RCU APIs
From: Paul E. McKenney @ 2019-05-28 20:00 UTC (permalink / raw)
To: Joel Fernandes
Cc: Steven Rostedt, linux-kernel, Benjamin Herrenschmidt, Ingo Molnar,
Jonathan Corbet, Josh Triplett, kvm-ppc, Lai Jiangshan, linux-doc,
linuxppc-dev, Mathieu Desnoyers, Michael Ellerman, Miguel Ojeda,
Paul Mackerras, rcu
In-Reply-To: <20190528190007.GC252809@google.com>
On Tue, May 28, 2019 at 03:00:07PM -0400, Joel Fernandes wrote:
> On Tue, May 28, 2019 at 05:24:47AM -0700, Paul E. McKenney wrote:
> > On Sat, May 25, 2019 at 02:14:07PM -0400, Joel Fernandes wrote:
> > > On Sat, May 25, 2019 at 08:50:35AM -0700, Paul E. McKenney wrote:
> > > > On Sat, May 25, 2019 at 10:19:54AM -0400, Joel Fernandes wrote:
> > > > > On Sat, May 25, 2019 at 07:08:26AM -0400, Steven Rostedt wrote:
> > > > > > On Sat, 25 May 2019 04:14:44 -0400
> > > > > > Joel Fernandes <joel@joelfernandes.org> wrote:
> > > > > >
> > > > > > > > I guess the difference between the _raw_notrace and just _raw variants
> > > > > > > > is that _notrace ones do a rcu_check_sparse(). Don't we want to keep
> > > > > > > > that check?
> > > > > > >
> > > > > > > This is true.
> > > > > > >
> > > > > > > Since the users of _raw_notrace are very few, is it worth keeping this API
> > > > > > > just for sparse checking? The API naming is also confusing. I was expecting
> > > > > > > _raw_notrace to do fewer checks than _raw, instead of more. Honestly, I just
> > > > > > > want to nuke _raw_notrace as done in this series and later we can introduce a
> > > > > > > sparse checking version of _raw if need-be. The other option could be to
> > > > > > > always do sparse checking for _raw however that used to be the case and got
> > > > > > > changed in http://lists.infradead.org/pipermail/linux-afs/2016-July/001016.html
> > > > > >
> > > > > > What if we just rename _raw to _raw_nocheck, and _raw_notrace to _raw ?
> > > > >
> > > > > That would also mean changing 160 usages of _raw to _raw_nocheck in the
> > > > > kernel :-/.
> > > > >
> > > > > The tracing usage of _raw_notrace is only like 2 or 3 users. Can we just call
> > > > > rcu_check_sparse directly in the calling code for those and eliminate the APIs?
> > > > >
> > > > > I wonder what Paul thinks about the matter as well.
> > > >
> > > > My thought is that it is likely that a goodly number of the current uses
> > > > of _raw should really be some form of _check, with lockdep expressions
> > > > spelled out. Not that working out what exactly those lockdep expressions
> > > > should be is necessarily a trivial undertaking. ;-)
> > >
> > > Yes, currently where I am a bit stuck is the rcu_dereference_raw()
> > > cannot possibly know what SRCU domain it is under, so lockdep cannot check if
> > > an SRCU lock is held without the user also passing along the SRCU domain. I
> > > am trying to change lockdep to see if it can check if *any* srcu domain lock
> > > is held (regardless of which one) and complain if none are. This is at least
> > > better than no check at all.
> > >
> > > However, I think it gets tricky for mutexes. If you have something like:
> > > mutex_lock(some_mutex);
> > > p = rcu_dereference_raw(gp);
> > > mutex_unlock(some_mutex);
> > >
> > > This might be a perfectly valid invocation of _raw, however my checks (patch
> > > is still cooking) trigger a lockdep warning becase _raw cannot know that this
> > > is Ok. lockdep thinks it is not in a reader section. This then gets into the
> > > territory of a new rcu_derference_raw_protected(gp, assert_held(some_mutex))
> > > which sucks because its yet another API. To circumvent this issue, can we
> > > just have callers of rcu_dereference_raw ensure that they call
> > > rcu_read_lock() if they are protecting dereferences by a mutex? That would
> > > make things a lot easier and also may be Ok since rcu_read_lock is quite
> > > cheap.
> >
> > Why not just rcu_dereference_protected(lockdep_is_held(some_mutex))?
> > The API is already there, and no need for spurious readers.
>
> Hmm, so I gave a bad example, here is a better example:
>
> fib_get_table calls hlist_for_each_entry_rcu()
> hlist_for_each_entry_rcu calls rcu_dereference_raw().
>
> This is perfectly Ok to be called under rtnl_mutex. However rcu_dererence_raw
> in hlist_for_each_entry_rcu has no way of knowing that the rtnl_mutex held is
> sufficient for the protection since it is not directly called by the caller.
Agreed, and this just happens to be one of the use cases that led to
rcu_dereference_raw(). The calling code (in this case, FIB) simply has
no idea what the synchronization strategy might be.
> I am almost sure I saw other examples of rcu_dereference_raw being called
> this way as well.
And I am OK with this sort of use case. The ones I am less happy with
are the ones where there really is a lockdep expression that could be
constructed.
> I was trying to make an "automatic" lockdep check for all this, but it is
> quite hard to do so without passing down lockdep experessions down a call
> chain thus complicating all such callchains.
Understood! Not an easy task.
> Further I don't think code can trivially be converted from
> rcu_dereference_raw to rcu_dereference_protected even if the protection being
> offered is known, since the former does not do sparse checking and the latter
> might trigger false sparse checks in case the pointer in concern is protected
> both by RCU and non-RCU methods. I believe this is why you removed sparse
> checking from rcu_dereference_raw as well:
>
> http://lists.infradead.org/pipermail/linux-afs/2016-July/001016.html
Good point!
> > > > That aside, if we are going to change the name of an API that is
> > > > used 160 places throughout the tree, we would need to have a pretty
> > > > good justification. Without such a justification, it will just look
> > > > like pointless churn to the various developers and maintainers on the
> > > > receiving end of the patches.
> > >
> > > Actually, the API name change is not something I want to do, it is Steven
> > > suggestion. My suggestion is let us just delete _raw_notrace and just use the
> > > _raw API for tracing, since _raw doesn't do any tracing anyway. Steve pointed
> > > that _raw_notrace does sparse checking unlike _raw, but I think that isn't an
> > > issue since _raw doesn't do such checking at the moment anyway.. (if possible
> > > check my cover letter again for details/motivation of this series).
> >
> > Understood, but regardless of who suggested it, if we are to go through
> > with it, good justification will be required. ;-)
>
> Ok ;-). About the names of the APIs, I thought of leaving rcu_dereference_raw
> and its callers intact, and just rename:
>
> * hlist_for_each_entry_rcu_notrace
> * rcu_dereference_raw_notrace
>
> to:
> * hlist_for_each_entry_rcu_sparse
> * rcu_dereference_raw_sparse
>
> The _sparse would stand for "sparse checking". However I am open to better
> names..
>
> Such renaming would avoid confusion and keep the fact about sparse checking
> less ambiguous.
Let's give people a few days to propose different names, and if nothing
compelling, those names look good. There are not very many of them, so
the penalty for having to rename is quite low.
Thanx, Paul
^ permalink raw reply
* Re: [PATCH v10 09/12] ima: Implement support for module-style appended signatures
From: Mimi Zohar @ 2019-05-28 20:06 UTC (permalink / raw)
To: Thiago Jung Bauermann
Cc: linux-integrity, linux-security-module, keyrings, linux-crypto,
linuxppc-dev, linux-doc, linux-kernel, Dmitry Kasatkin,
James Morris, Serge E. Hallyn, David Howells, David Woodhouse,
Jessica Yu, Herbert Xu, David S. Miller, Jonathan Corbet,
AKASHI, Takahiro
In-Reply-To: <87zhn65qor.fsf@morokweng.localdomain>
On Tue, 2019-05-28 at 16:23 -0300, Thiago Jung Bauermann wrote:
> Mimi Zohar <zohar@linux.ibm.com> writes:
>
> > Hi Thiago,
> >
> >> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
> >> index fca7a3f23321..a7a20a8c15c1 100644
> >> --- a/security/integrity/ima/ima_policy.c
> >> +++ b/security/integrity/ima/ima_policy.c
> >> @@ -1144,6 +1144,12 @@ void ima_delete_rules(void)
> >> }
> >> }
> >>
> >> +#define __ima_hook_stringify(str) (#str),
> >> +
> >> +const char *const func_tokens[] = {
> >> + __ima_hooks(__ima_hook_stringify)
> >> +};
> >> +
> >> #ifdef CONFIG_IMA_READ_POLICY
> >> enum {
> >> mask_exec = 0, mask_write, mask_read, mask_append
> >> @@ -1156,12 +1162,6 @@ static const char *const mask_tokens[] = {
> >> "MAY_APPEND"
> >> };
> >>
> >> -#define __ima_hook_stringify(str) (#str),
> >> -
> >> -static const char *const func_tokens[] = {
> >> - __ima_hooks(__ima_hook_stringify)
> >> -};
> >> -
> >> void *ima_policy_start(struct seq_file *m, loff_t *pos)
> >> {
> >> loff_t l = *pos;
> >
> > Is moving this something left over from previous versions or there is
> > a need for this change?
>
> Well, it's not a strong need, but it's still relevant in the current
> version. I use func_tokens in ima_read_modsig() in order to be able to
> mention the hook name in mod_check_sig()'s error message:
>
> In ima_read_modsig():
>
> rc = mod_check_sig(sig, buf_len, func_tokens[func]);
>
> And in mod_check_sig():
>
> pr_err("%s: Module is not signed with expected PKCS#7 message\n",
> name);
>
> If you think it's not worth it to expose func_tokens, I can make
> ima_read_modsig() pass a more generic const string such as "IMA modsig"
> for example.
This is fine. I somehow missed moving func_tokens[] outside of the
ifdef was in order to make it independent of "CONFIG_IMA_READ_POLICY".
thanks,
Mimi
^ permalink raw reply
* Re: [PATCH v2 1/1] sched/fair: Fix low cpu usage with high throttling by removing expiration of cpu-local slices
From: Dave Chiluk @ 2019-05-28 22:25 UTC (permalink / raw)
To: Peter Oskolkov
Cc: Phil Auld, Peter Zijlstra, Ingo Molnar, cgroups,
Linux Kernel Mailing List, Brendan Gregg, Kyle Anderson,
Gabriel Munos, John Hammond, Cong Wang, Jonathan Corbet,
linux-doc, Ben Segall
In-Reply-To: <CAFTs51VhpDk9iW5UT62CkPCN3SjgUHHO1nVqhe+ssHMYqou6Bg@mail.gmail.com>
On Fri, May 24, 2019 at 5:07 PM Peter Oskolkov <posk@posk.io> wrote:
> Linux CPU scheduling tail latency is a well-known issue and a major
> pain point in some workloads:
> https://www.google.com/search?q=linux+cpu+scheduling+tail+latency
>
> Even assuming that nobody noticed this particular cause
> of CPU scheduling latencies, it does not mean the problem should be waved
> away. At least it should be documented, if at this point it decided that
> it is difficult to address it in a meaningful way. And, preferably, a way
> to address the issue later on should be discussed and hopefully agreed to.
Pursuing reducing tail latencies for our web application is the
precise reason I created this patch set. Those applications that
previously were responding in 20ms 95% where now taking 220ms. Those
were correctly sized applications prior to 512ac999. After which, they
started seeing massive increases in their latencies due to hitting
throttling with lower than quota amounts of cpu usage.
I'll see if I can rework the documentation. Any specific
suggestions for how that can be worded would be appreciated.
^ permalink raw reply
* Re: [PATCH 1/3] xen: remove tmem driver
From: Boris Ostrovsky @ 2019-05-28 22:44 UTC (permalink / raw)
To: Juergen Gross, linux-kernel, linux-doc
Cc: Jonathan Corbet, Stefano Stabellini, xen-devel
In-Reply-To: <20190527103207.13287-2-jgross@suse.com>
> diff --git a/include/xen/balloon.h b/include/xen/balloon.h
> index 4914b93a23f2..a72ef3f88b39 100644
> --- a/include/xen/balloon.h
> +++ b/include/xen/balloon.h
> @@ -28,14 +28,6 @@ int alloc_xenballooned_pages(int nr_pages, struct page **pages);
> void free_xenballooned_pages(int nr_pages, struct page **pages);
>
> struct device;
This can be removed as well.
Other than that,
Acked-by: Boris Ostriovsky <boris.ostrovsky@oracle.com>
> -#ifdef CONFIG_XEN_SELFBALLOONING
> -extern int register_xen_selfballooning(struct device *dev);
> -#else
> -static inline int register_xen_selfballooning(struct device *dev)
> -{
> - return -ENOSYS;
> -}
> -#endif
>
> #ifdef CONFIG_XEN_BALLOON
> void xen_balloon_init(void);
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox