linux-kernel - Re: Thread flags modified without set_thread

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: <20070301144138.57d6d8c7.akpm@linux-foundation.org>
Date:	Thu, 1 Mar 2007 14:41:38 -0800
From:	Andrew Morton <akpm@...ux-foundation.org>
To:	Mathieu Desnoyers <compudj@...gle.com>
Cc:	Haavard Skinnemoen <hskinnemoen@...el.com>,
	linux-kernel@...r.kernel.org
Subject: Re: Thread flags modified without set_thread_flag() (non
 atomically)

On Thu, 01 Mar 2007 11:59:07 -0800
Mathieu Desnoyers <compudj@...gle.com> wrote:

> Andrew Morton wrote:
> > On Thu, 1 Mar 2007 10:34:51 +0100 Haavard Skinnemoen <hskinnemoen@...el.com> wrote:
> >
> >   
> >>> On Mon, 26 Feb 2007 12:10:37 -0800 Mathieu Desnoyers <compudj@...gle.com> wrote:
> >>>       
> >>>> avr32/kernel/ptrace.c:          ti->flags |= _TIF_BREAKPOINT;
> >>>>         
> >>> No, I don't immediately see anything in the flush_old_exec() code path
> >>> which tells us that nobody else can look up this thread_info (or be holding
> >>> a ref to it) in this context.
> >>>
> >>>
> >>>       
> >>>> avr32/kernel/ptrace.c:                  ti->flags |= TIF_SINGLE_STEP;
> >>>>         
> >>> heh.  Haarvard, you got a bug.
> >>>       
> >> Heh, yeah. That would indeed explain some strange gdb behaviour. It
> >> will only trigger when single-stepping into an exception or interrupt
> >> handler so thanks for pointing it out; I would have had a hard time
> >> figuring it out on my own...
> >>     
> >
> > yup, tricky.
> >
> > If there's a lesson here, it is "don't provide #defines in the header for
> > both versions".
> >
> >   
> 
> Hrm, but the bitmask version is useful (and correctly used) whenever
> the flag is read and tested.
> 
> The proper way to do this would be to change every use the _TIF_* flag 
> in a flag comparison
> for a call to test_ti_thread_flag(). I wonder if gcc optimizes multiple 
> constant test_bit() applying
> on the same variable linked by logical and/or so it becomes a single 
> read and a small set of comparisons.

Well, it's unusual for code to want to test multiple bitflags in the same
operation.  Perhaps thread_info.flags is unusual in that regard.

But one can still do

	if (foo->flags & (1<<bar)|(1<<zot))

Which can get to be a pain if it happens in a lot of places.  But if it
only happens in a few places, this seems a reasonable price to pay, given
the safety gains.

Plus I'm _forever_ having to go into the header file to remember whether
REQ_RW is the bitmask or the bit offset.

-
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/