linux-kernel - Re: [RESEND PATCH] ARM: fix 'unannotated irqs-on' lockdep warning

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

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

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

Message-ID: <20100523194746.GE950@n2100.arm.linux.org.uk>
Date:	Sun, 23 May 2010 20:47:46 +0100
From:	Russell King - ARM Linux <linux@....linux.org.uk>
To:	Ming Lei <tom.leiming@...il.com>
Cc:	linux-arm-kernel@...ts.infradead.org,
	linux-embedded@...r.kernel.org, a.p.zijlstra@...llo.nl,
	linux-kernel@...r.kernel.org
Subject: Re: [RESEND PATCH] ARM: fix 'unannotated irqs-on' lockdep warning

On Sun, May 23, 2010 at 11:07:50PM +0800, Ming Lei wrote:
> 2010/5/23 Russell King - ARM Linux <linux@....linux.org.uk>:
> > On Sun, May 23, 2010 at 09:44:20PM +0800, Ming Lei wrote:
> >> 2010/5/23 Russell King - ARM Linux <linux@....linux.org.uk>:
> >> >>  ENTRY(ret_to_user)
> >> >>  ret_slow_syscall:
> >> >> -     disable_irq                             @ disable interrupts
> >> >> +     disable_irq_notrace                             @ disable interrupts
> >> >
> >> > I think this one does need to be traced - the pending work functions are
> >> > all C code which could call back into lockdep.
> >>
> >> If there are pending works, schedule will be called to give cpu to it,
> >> I wonder why the work function to be scheduled will be run with irq
> >> disabled. Seems we should enable irq again before calling schedule,
> >> not sure.
> >
> > No.  I'm talking about things like do_notify_resume().
> >
> > I think the above should be left as-is, so that as far as lockdep is
> > concerned, IRQs are off while userspace runs.  What happens between
> > returning to userspace and re-entering the kernel has no bearing what
> > so ever on lockdep.
> >
> 
> Oh, trace_ret_hardirqs_on has to be added before returning to user-space to
> remove the warning, like x86 and mips. If you agree, I'd like to post
> a new version patch.

Let me explain again.  We have this series of actions:

- in userspace
- exception happens
- cpu disables interrupts itself
- save state
- enable interrupts, and tell lockdep that IRQs are unmasked
- we process the exception, and ultimately call ret_fast_syscall or
  ret_slow_syscall

Now, what was happening in existing kernels is:

POINT A.
- disable interrupts, and tell lockdep that IRQs are masked
- check for any work pending
  - if work pending, call function - with IRQs still masked
  - go back to point A.
- restore state
- resume userspace, which implicitly re-enables IRQs

This results in a balanced and afaics correct setup.  Lockdep doesn't
care about the state of userspace - it only cares about state (and its
code only ever runs) when we're in kernel mode.

With your change above, what's happening is the above is replaced by:

POINT A.
- disable interrupts, but don't tell lockdep that IRQs are masked
- check for any work pending
  - if work pending, call function - with IRQs still masked
	*but* lockdep believes IRQs are enabled.  Therefore, I believe
	false warnings are probable from things like the scheduler,
	signal handling paths, etc.
  - go back to point A.
- restore state
- resume userspace, which implicitly re-enables IRQs

So can you now see why I believe the above change I've quoted is wrong?

Moreover, I put to you that it's utterly pointless - and a waste of CPU
time - telling lockdep about the IRQ masking when an exception occurs,
and it's also pointless telling lockdep about the IRQ unmasking when we
resume userspace.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/