All of lore.kernel.org
 help / color / mirror / Atom feed
From: Darren Hart <dvhltc@us.ibm.com>
To: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
Cc: Peter Zijlstra <peterz@infradead.org>,
	Mark Seaborn <mrs@mythic-beasts.com>,
	linux-kernel@vger.kernel.org, Ingo Molnar <mingo@elte.hu>,
	Thomas Gleixner <tglx@linutronix.de>,
	"hugh.dickins" <hugh.dickins@tiscali.co.uk>
Subject: Re: futex() on vdso makes process unkillable
Date: Tue, 26 Jan 2010 06:21:46 -0800	[thread overview]
Message-ID: <4B5EFA7A.9080800@us.ibm.com> (raw)
In-Reply-To: <20100126113312.5AA3.A69D9226@jp.fujitsu.com>

KOSAKI Motohiro wrote:
>> Peter Zijlstra wrote:
>>> On Mon, 2010-01-25 at 16:27 +0900, KOSAKI Motohiro wrote:
>> <snip>
>>>> Futex should work both file anon anon. however I personally think 
>>>> vdso is not file nor anon. it is special mappings. nobody defined
>>>> futex spec on special mappings. (yes, undefined).
>>>>
>>>> Personally, I think EINVAL or EFAULT are best result of vdso futexing, like as
>>>> futexing againt kernel address. but I guess another person have another thinking.
>>>>
>>>> I'd like to hear futex folks's opinion.
>>> Well, my opinion is we should remove the vdso, its ugly as hell :-)
>>>
>>> But I think it would make most sense to extend its definition in the
>>> direction of it being a file (for all intents and purposes its a special
>>> DSO -- which unfortunately isn't present in any filesystem).
>>>
>>> [ For all intents and purposes processes can already communicate through
>>> futexes on the libc space, so being able to do so through the vsdo
>>> really doesn't add anything ]
>>>
>>> So the problem is that the VDSO pages do not have a page->mapping
>>> because they lack the actual filesystem part of files, so even if (with
>>> the recent zero-page patch from Kosaki-san) you make private COWs of the
>>> VDSO, you'll get stuck in that loop.
>>>
>>> So the prettiest solution is to simply place the vdso in an actual
>>> filesystem and slowly migrate towards letting userspace map it as a
>>> regular DSO -- /sys/lib{32,64}/libkernel.so like.
>>>
>>> [ that has the bonus of getting rid of install_special_mapping() ]
>>>
>>> The ugly solution is special casing the vdso in get_futex_key().
>> I like the creating-a-real-file solution. However, for now (and for 
>> stable), I think Kosaki's suggestion of EINVAL or EFAULT is a good 
>> stop-gap. EINVAL might play the best with existing glibc implementations.
> 
> May I confirm your mention?
> 
> If we can accept EFAULT, we don't need any change. my previous futex patch
> already did. because 1) VDSO is alwasys read-only mapped 2) write mode
> get_user_pages_fast() against read-only pte/vma return EFAULT.
> 
> Current linus and stable tree don't cause Mark's original problem. instead, just
> return EFAULT. (Well, I'm sorry. my previous mail was unclear. I wrote v2.6.31 test
> result)
> 
> If you can't accept EFAULT, we need to add vdso specific logic into get_futex_key().
> Is this your intention?

That was my intention, but after looking at the glibc source, I don't 
see any reason for EINVAL over EFAULT. I apparently mis-remembered 
something there. EFAULT is fine.

-- 
Darren Hart
IBM Linux Technology Center
Real-Time Linux Team

      parent reply	other threads:[~2010-01-26 14:22 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-01-24  0:04 futex() on vdso makes process unkillable Mark Seaborn
2010-01-25  3:37 ` KOSAKI Motohiro
2010-01-25  7:27   ` KOSAKI Motohiro
2010-01-25  9:26     ` Peter Zijlstra
2010-01-25 17:37       ` Darren Hart
2010-01-26  2:41         ` KOSAKI Motohiro
2010-01-26  7:52           ` Peter Zijlstra
2010-01-26  8:33           ` Thomas Gleixner
2010-01-26 14:21           ` Darren Hart [this message]

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=4B5EFA7A.9080800@us.ibm.com \
    --to=dvhltc@us.ibm.com \
    --cc=hugh.dickins@tiscali.co.uk \
    --cc=kosaki.motohiro@jp.fujitsu.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=mrs@mythic-beasts.com \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    /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.