Messages in this thread | | | Date | Fri, 2 May 2008 12:37:55 +0400 | From | Alexey Dobriyan <> | Subject | Re: [PATCH] add typecheck on irqsave and friends for correct flags |
| |
On Thu, May 01, 2008 at 07:51:18PM -0400, Steven Rostedt wrote: > There has been several areas in the kernel where an int has been used > for flags in local_irq_save and friends instead of a long. This can > cause some hard to debug problems on some architectures. > > This patch adds a typecheck inside the irqsave and restore functions > to flag these cases.
OK, at least something that uncovers such bugs. Same checks in include/asm-frv/system.h should be removed then.
> --- linux-compile.git.orig/include/linux/irqflags.h > +++ linux-compile.git/include/linux/irqflags.h > @@ -49,18 +49,24 @@ > do { trace_hardirqs_on(); raw_local_irq_enable(); } while (0) > #define local_irq_disable() \ > do { raw_local_irq_disable(); trace_hardirqs_off(); } while (0) > -#define local_irq_save(flags) \ > - do { raw_local_irq_save(flags); trace_hardirqs_off(); } while (0) > +#define local_irq_save(flags) \ > + do { \ > + typecheck(unsigned long, flags); \ > + raw_local_irq_save(flags); \ > + trace_hardirqs_off(); \ > + } while (0) > > -#define local_irq_restore(flags) \ > - do { \ > - if (raw_irqs_disabled_flags(flags)) { \ > - raw_local_irq_restore(flags); \ > - trace_hardirqs_off(); \ > - } else { \ > - trace_hardirqs_on(); \ > - raw_local_irq_restore(flags); \ > - } \ > + > +#define local_irq_restore(flags) \ > + do { \ > + typecheck(unsigned long, flags); \ > + if (raw_irqs_disabled_flags(flags)) { \ > + raw_local_irq_restore(flags); \ > + trace_hardirqs_off(); \ > + } else { \ > + trace_hardirqs_on(); \ > + raw_local_irq_restore(flags); \ > + } \ > } while (0) > #else /* !CONFIG_TRACE_IRQFLAGS_SUPPORT */ > /* > @@ -69,8 +75,16 @@ > */ > # define raw_local_irq_disable() local_irq_disable() > # define raw_local_irq_enable() local_irq_enable() > -# define raw_local_irq_save(flags) local_irq_save(flags) > -# define raw_local_irq_restore(flags) local_irq_restore(flags) > +# define raw_local_irq_save(flags) \ > + do { \ > + typecheck(unsigned long, flags); \ > + local_irq_save(flags); \ > + } while (0) > +# define raw_local_irq_restore(flags) \ > + do { \ > + typecheck(unsigned long, flags); \ > + local_irq_restore(flags); \ > + } while (0) > #endif /* CONFIG_TRACE_IRQFLAGS_SUPPORT */ > > #ifdef CONFIG_TRACE_IRQFLAGS_SUPPORT > @@ -80,7 +94,11 @@ > raw_safe_halt(); \ > } while (0) > > -#define local_save_flags(flags) raw_local_save_flags(flags) > +#define local_save_flags(flags) \ > + do { \ > + typecheck(unsigned long, flags); \ > + raw_local_save_flags(flags); \ > + } while(0) > > #define irqs_disabled() \ > ({ \ > @@ -90,7 +108,11 @@ > raw_irqs_disabled_flags(_flags); \ > }) > > -#define irqs_disabled_flags(flags) raw_irqs_disabled_flags(flags) > +#define irqs_disabled_flags(flags) \ > +({ \ > + typecheck(unsigned long, flags); \ > + raw_irqs_disabled_flags(flags); \ > +}) > #endif /* CONFIG_X86 */ > > #endif > Index: linux-compile.git/include/linux/spinlock.h > =================================================================== > --- linux-compile.git.orig/include/linux/spinlock.h 2008-05-01 16:58:58.000000000 -0400 > +++ linux-compile.git/include/linux/spinlock.h 2008-05-01 19:16:21.000000000 -0400 > @@ -191,23 +191,53 @@ do { \ > > #if defined(CONFIG_SMP) || defined(CONFIG_DEBUG_SPINLOCK) > > -#define spin_lock_irqsave(lock, flags) flags = _spin_lock_irqsave(lock) > -#define read_lock_irqsave(lock, flags) flags = _read_lock_irqsave(lock) > -#define write_lock_irqsave(lock, flags) flags = _write_lock_irqsave(lock) > +#define spin_lock_irqsave(lock, flags) \ > + do { \ > + typecheck(unsigned long, flags); \ > + flags = _spin_lock_irqsave(lock); \ > + } while (0) > +#define read_lock_irqsave(lock, flags) \ > + do { \ > + typecheck(unsigned long, flags); \ > + flags = _read_lock_irqsave(lock); \ > + } while (0) > +#define write_lock_irqsave(lock, flags) \ > + do { \ > + typecheck(unsigned long, flags); \ > + flags = _write_lock_irqsave(lock); \ > + } while (0) > > #ifdef CONFIG_DEBUG_LOCK_ALLOC > -#define spin_lock_irqsave_nested(lock, flags, subclass) \ > - flags = _spin_lock_irqsave_nested(lock, subclass) > +#define spin_lock_irqsave_nested(lock, flags, subclass) \ > + do { \ > + typecheck(unsigned long, flags); \ > + flags = _spin_lock_irqsave_nested(lock, subclass); \ > + } while (0) > #else > -#define spin_lock_irqsave_nested(lock, flags, subclass) \ > - flags = _spin_lock_irqsave(lock) > +#define spin_lock_irqsave_nested(lock, flags, subclass) \ > + do { \ > + typecheck(unsigned long, flags); \ > + flags = _spin_lock_irqsave(lock); \ > + } while (0) > #endif > > #else > > -#define spin_lock_irqsave(lock, flags) _spin_lock_irqsave(lock, flags) > -#define read_lock_irqsave(lock, flags) _read_lock_irqsave(lock, flags) > -#define write_lock_irqsave(lock, flags) _write_lock_irqsave(lock, flags) > +#define spin_lock_irqsave(lock, flags) \ > + do { \ > + typecheck(unsigned long, flags); \ > + _spin_lock_irqsave(lock, flags); \ > + } while (0) > +#define read_lock_irqsave(lock, flags) \ > + do { \ > + typecheck(unsigned long, flags); \ > + _read_lock_irqsave(lock, flags); \ > + } while (0) > +#define write_lock_irqsave(lock, flags) \ > + do { \ > + typecheck(unsigned long, flags); \ > + _write_lock_irqsave(lock, flags); \ > + } while (0) > #define spin_lock_irqsave_nested(lock, flags, subclass) \ > spin_lock_irqsave(lock, flags) > > @@ -260,16 +290,25 @@ do { \ > } while (0) > #endif > > -#define spin_unlock_irqrestore(lock, flags) \ > - _spin_unlock_irqrestore(lock, flags) > +#define spin_unlock_irqrestore(lock, flags) \ > + do { \ > + typecheck(unsigned long, flags); \ > + _spin_unlock_irqrestore(lock, flags); \ > + } while (0) > #define spin_unlock_bh(lock) _spin_unlock_bh(lock) > > -#define read_unlock_irqrestore(lock, flags) \ > - _read_unlock_irqrestore(lock, flags) > +#define read_unlock_irqrestore(lock, flags) \ > + do { \ > + typecheck(unsigned long, flags); \ > + _read_unlock_irqrestore(lock, flags); \ > + } while (0) > #define read_unlock_bh(lock) _read_unlock_bh(lock) > > -#define write_unlock_irqrestore(lock, flags) \ > - _write_unlock_irqrestore(lock, flags) > +#define write_unlock_irqrestore(lock, flags) \ > + do { \ > + typecheck(unsigned long, flags); \ > + _write_unlock_irqrestore(lock, flags); \ > + } while (0) > #define write_unlock_bh(lock) _write_unlock_bh(lock) > > #define spin_trylock_bh(lock) __cond_lock(lock, _spin_trylock_bh(lock))
| |