Messages in this thread | ![/](/images/icornerl.gif) | | From | Johannes Weiner <> | Subject | Re: Dangerous code in cpumask_of_cpu? | Date | Tue, 08 Jul 2008 11:03:10 +0200 |
| |
Johannes Weiner <hannes@saeurebad.de> writes:
> Hi, > > Johannes Weiner <hannes@saeurebad.de> writes: > >> Hi, >> >> Rusty Russell <rusty@rustcorp.com.au> writes: >> >>> Hi Christoph/Mike, >>> >>> Looked at cpumask_of_cpu as introduced in >>> 9f0e8d0400d925c3acd5f4e01dbeb736e4011882 (x86: convert cpumask_of_cpu macro >>> to allocated array), and I don't think it's safe: >>> >>> #define cpumask_of_cpu(cpu) \ >>> (*({ \ >>> typeof(_unused_cpumask_arg_) m; \ >>> if (sizeof(m) == sizeof(unsigned long)) { \ >>> m.bits[0] = 1UL<<(cpu); \ >>> } else { \ >>> cpus_clear(m); \ >>> cpu_set((cpu), m); \ >>> } \ >>> &m; \ >>> })) >>> >>> Referring to &m once out of scope is invalid, and I can't find any evidence >>> that it's legal here. In particular, the change >>> b53e921ba1cff8453dc9a87a84052fa12d5b30bd (generic: reduce stack pressure in >>> sched_affinity) which passes &m to other functions seems highly risky. >>> >>> I'm surprised this hasn't already hit us, but perhaps gcc isn't as clever as >>> it could be? > >> You don't refer to &m outside scope. Look at the character below the >> first e of #define :) > > Oh, well you do access it outside scope, sorry. Me sleepy. > > I guess because we dereference it immediately again, the location is not > clobbered yet. At least in my test case, gcc assembled it to code that > puts the address in eax and derefences it immediately, before eax is > reused:
Gee, just ignore this bs. The address is in eax, not the value.
> static int *foo(void) > { > int x = 42; > return &x; > } > > int main(void) > { > return *foo(); > }
However, this code seems to produce valid assembly with -O2. gcc just warns and fixes it up.
Hannes
| ![\](/images/icornerr.gif) |