All of lore.kernel.org
 help / color / mirror / Atom feed
From: Rusty Russell <rusty@rustcorp.com.au>
To: Mike Travis <travis@sgi.com>
Cc: linux-kernel@vger.kernel.org, "H. Anvin" <hpa@zytor.com>,
	Christoph Lameter <clameter@sgi.com>, Ingo Molnar <mingo@elte.hu>
Subject: Dangerous code in cpumask_of_cpu?
Date: Tue, 8 Jul 2008 18:16:40 +1000	[thread overview]
Message-ID: <200807081816.40623.rusty@rustcorp.com.au> (raw)

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?

I don't know what the right answer is, but we might need to go to a pool of 
cpumask_ts, a get_cpumask_of_cpu() which can sleep and a put_cpumask_of_cpu?

Or maybe a gcc guru can refute this?
Rusty.

             reply	other threads:[~2008-07-08  8:17 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-07-08  8:16 Rusty Russell [this message]
2008-07-08  8:35 ` Dangerous code in cpumask_of_cpu? Johannes Weiner
2008-07-08  8:54   ` Johannes Weiner
2008-07-08  9:03     ` Johannes Weiner
2008-07-08  9:28       ` Johannes Weiner
2008-07-08 15:29       ` Mike Travis
2008-07-09  2:22         ` Rusty Russell
2008-07-09 14:42           ` Mike Travis
2008-07-08  9:33     ` Rusty Russell
2008-07-08 10:24   ` Andreas Schwab

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=200807081816.40623.rusty@rustcorp.com.au \
    --to=rusty@rustcorp.com.au \
    --cc=clameter@sgi.com \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=travis@sgi.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.