All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andi Kleen <andi@firstfloor.org>
To: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: Andi Kleen <andi@firstfloor.org>,
	linux-kernel@vger.kernel.org, paulmck@linux.vnet.ibm.com
Subject: Re: [PATCH] [0/11] SYSCTL: Use RCU to avoid races with string sysctls
Date: Mon, 21 Dec 2009 03:04:28 +0100	[thread overview]
Message-ID: <20091221020427.GA25372@basil.fritz.box> (raw)
In-Reply-To: <m1eimpm6hs.fsf@fess.ebiederm.org>

On Sun, Dec 20, 2009 at 05:59:59PM -0800, Eric W. Biederman wrote:
> Andi Kleen <andi@firstfloor.org> writes:
> 
> > With BKL-less sysctls most of the writable string sysctls are racy. There
> > is no locking on the reader side, so a reader could see an inconsistent
> > string or worse miss the terminating null and walk of beyond it.
> 
> The walk will only extend up to the maximum length of the string.
> So the worst case really is inconsistent data.

It could still miss the 0 byte and walk out, can't it?

> This is an unfortunate corner case.  This is not a regression as this
> has been the way things have worked for years.  So probably 2.6.34
>  material.

The one that's a clear regression is the core pattern one, that 
was protected before by the BKL. A lot of others were always
broken yes.

> 
> > This patch kit adds a new "rcu string" variant to avoid these 
> > problems and convers the racy users. One the writer side the strings are 
> > always copied to new memory and the readers use rcu_read_lock()
> > to get a stable view. For readers who access the string over
> > sleeps the reader copies the string. 
> 
> I will have to look more after the holidays.  This rcu_string looks like
> it introduces allocations on paths that did not use them before, which
> has me wondering a bit.

On the reader side about all of them allocated before, e.g. for
call_usermodehelper.

If the strings were made a bit smaller this could be also 
put on the stack, but I didn't dare for 256 bytes. 

-Andi

-- 
ak@linux.intel.com -- Speaking for myself only.

  reply	other threads:[~2009-12-21  2:04 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-12-21  1:20 [PATCH] [0/11] SYSCTL: Use RCU to avoid races with string sysctls Andi Kleen
2009-12-21  1:20 ` [PATCH] [1/11] Add rcustring ADT for RCU protected strings Andi Kleen
2009-12-22  2:46   ` Paul E. McKenney
2009-12-22 10:05     ` Andi Kleen
2009-12-22 20:59       ` Paul E. McKenney
2009-12-21  1:20 ` [PATCH] [2/11] Add a kernel_address() that works for data too Andi Kleen
2009-12-21  1:20 ` [PATCH] [3/11] SYSCTL: Add proc_rcu_string to manage sysctls using rcu strings Andi Kleen
2009-12-22  2:51   ` Paul E. McKenney
2009-12-22  3:00     ` Eric W. Biederman
2009-12-22  7:44       ` Paul E. McKenney
2009-12-21  1:20 ` [PATCH] [4/11] SYSCTL: Use RCU strings for core_pattern sysctl Andi Kleen
2009-12-21  1:20 ` [PATCH] [5/11] SYSCTL: Add call_usermodehelper_cleanup() Andi Kleen
2009-12-21  1:20 ` [PATCH] [6/11] SYSCTL: Convert modprobe_path to proc_rcu_string() Andi Kleen
2009-12-21  1:20 ` [PATCH] [7/11] SYSCTL: Convert poweroff_command to proc_rcu_string Andi Kleen
2009-12-21  1:20 ` [PATCH] [8/11] SYSCTL: Convert hotplug helper string to proc_rcu_string() Andi Kleen
2009-12-22 19:03   ` Greg KH
2009-12-21  1:20 ` [PATCH] [9/11] SYSCTL: Add a mutex to the page_alloc zone order sysctl Andi Kleen
2009-12-21  1:20 ` [PATCH] [10/11] SYSCTL: Use RCU protected sysctl for ocfs group add helper Andi Kleen
2009-12-21  1:20 ` [PATCH] [11/11] SYSCTL: Convert IRDA text sysctl to RCU Andi Kleen
2009-12-21  1:59 ` [PATCH] [0/11] SYSCTL: Use RCU to avoid races with string sysctls Eric W. Biederman
2009-12-21  2:04   ` Andi Kleen [this message]
2009-12-21  2:31     ` Eric W. Biederman
2009-12-21  3:21       ` Andi Kleen
  -- strict thread matches above, loose matches on Subject: below --
2009-12-21  7:57 Alexey Dobriyan
2009-12-21 10:50 ` Andi Kleen

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=20091221020427.GA25372@basil.fritz.box \
    --to=andi@firstfloor.org \
    --cc=ebiederm@xmission.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=paulmck@linux.vnet.ibm.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.