From: Izik Eidus <ieidus@redhat.com>
To: Anthony Liguori <anthony@codemonkey.ws>
Cc: Chris Wright <chrisw@redhat.com>,
Andrea Arcangeli <aarcange@redhat.com>,
linux-kernel@vger.kernel.org, kvm@vger.kernel.org,
linux-mm@kvack.org, avi@redhat.com, riel@redhat.com,
jeremy@goop.org, mtosatti@redhat.com, hugh@veritas.com,
corbet@lwn.net, yaniv@redhat.com, dmonakhov@openvz.org
Subject: Re: [PATCH 4/4] add ksm kernel shared memory driver.
Date: Thu, 02 Apr 2009 14:23:02 +0300 [thread overview]
Message-ID: <49D4A016.9040506@redhat.com> (raw)
In-Reply-To: <49D424AF.3090806@codemonkey.ws>
Anthony Liguori wrote:
> Chris Wright wrote:
>> * Anthony Liguori (anthony@codemonkey.ws) wrote:
>>
>>> The ioctl() interface is quite bad for what you're doing. You're
>>> telling the kernel extra information about a VA range in
>>> userspace. That's what madvise is for. You're tweaking simple
>>> read/write values of kernel infrastructure. That's what sysfs is for.
>>>
>>
>> I agree re: sysfs (brought it up myself before). As far as madvise vs.
>> ioctl, the one thing that comes from the ioctl is fops->release to
>> automagically unregister memory on exit.
>
> This is precisely why ioctl() is a bad interface. fops->release isn't
> tied to the process but rather tied to the open file. The file can
> stay open long after the process exits either by a fork()'d child
> inheriting the file descriptor or through something more sinister like
> SCM_RIGHTS.
>
> In fact, a common mistake is to leak file descriptors by not closing
> them when exec()'ing a process. Instead of just delaying a close, if
> you rely on this behavior to unregister memory regions, you could
> potentially have badness happen in the kernel if ksm attempted to
> access an invalid memory region.
How could such badness ever happen in the kernel?
Ksm work by virtual addresses!, it fetch the pages by using
get_user_pages(), and the mm struct is protected by get_task_mm(), in
addion we take the down_read(mmap_sem)
So how could ksm ever acces to invalid memory region unless the host
page table or get_task_mm() would stop working!
When someone register memory for scan, we do get_task_mm() when the file
is closed or when he say that he dont want this to be registered anymore
he call the unregister ioctl
You can aurgoment about API, but this is mathamathical thing to say Ksm
is insecure, please show me senario!
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
next prev parent reply other threads:[~2009-04-02 11:27 UTC|newest]
Thread overview: 66+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-03-30 23:59 [PATCH 0/4] ksm - dynamic page sharing driver for linux Izik Eidus
2009-03-30 23:59 ` [PATCH 1/4] MMU_NOTIFIERS: add set_pte_at_notify() Izik Eidus
2009-03-30 23:59 ` [PATCH 2/4] add page_wrprotect(): write protecting page Izik Eidus
2009-03-30 23:59 ` [PATCH 3/4] add replace_page(): change the page pte is pointing to Izik Eidus
2009-03-30 23:59 ` [PATCH 4/4] add ksm kernel shared memory driver Izik Eidus
2009-03-31 2:12 ` Anthony Liguori
2009-03-31 12:24 ` Izik Eidus
2009-03-31 13:31 ` Anthony Liguori
2009-03-31 14:25 ` Andrea Arcangeli
2009-03-31 14:37 ` Anthony Liguori
2009-03-31 15:02 ` Andrea Arcangeli
2009-03-31 15:09 ` Anthony Liguori
2009-03-31 15:18 ` Andrea Arcangeli
2009-03-31 15:54 ` Anthony Liguori
2009-03-31 16:25 ` Andrea Arcangeli
2009-03-31 16:51 ` Anthony Liguori
2009-03-31 17:11 ` Andrea Arcangeli
2009-04-01 22:54 ` Izik Eidus
2009-04-02 0:31 ` Anthony Liguori
2009-04-02 0:48 ` Chris Wright
2009-04-02 1:22 ` Chris Wright
2009-04-02 2:36 ` Anthony Liguori
2009-04-02 5:31 ` [PATCH 5/4] update ksm userspace interfaces Chris Wright
2009-04-02 13:32 ` Izik Eidus
2009-04-02 15:20 ` Chris Wright
2009-04-02 15:56 ` Chris Wright
2009-04-02 15:55 ` Izik Eidus
2009-04-03 10:16 ` Gerd Hoffmann
2009-04-03 10:49 ` Izik Eidus
2009-04-03 11:08 ` Gerd Hoffmann
2009-04-03 16:22 ` Chris Wright
2009-04-02 14:41 ` Andrea Arcangeli
2009-04-02 15:12 ` Chris Wright
2009-04-02 15:25 ` Andrea Arcangeli
2009-04-02 5:48 ` [PATCH 4/4 alternative userspace] add ksm kernel shared memory driver Chris Wright
2009-04-02 5:57 ` Bert Wesarg
2009-04-02 5:59 ` Chris Wright
2009-04-02 6:00 ` Bert Wesarg
2009-04-02 7:09 ` Avi Kivity
2009-04-02 7:24 ` [PATCH 4/4] " Avi Kivity
2009-04-02 9:38 ` Andrea Arcangeli
2009-04-02 11:23 ` Izik Eidus [this message]
2009-03-31 2:15 ` KAMEZAWA Hiroyuki
2009-03-31 12:21 ` Izik Eidus
2009-03-31 23:57 ` KAMEZAWA Hiroyuki
2009-04-01 17:28 ` Izik Eidus
2009-03-31 20:52 ` Andrea Arcangeli
2009-03-31 1:42 ` [PATCH 0/4] ksm - dynamic page sharing driver for linux Anthony Liguori
2009-03-31 12:33 ` Izik Eidus
2009-04-02 19:22 ` Jesper Juhl
2009-04-02 19:38 ` Izik Eidus
2009-04-02 19:39 ` Chris Wright
2009-04-02 19:49 ` Jesper Juhl
-- strict thread matches above, loose matches on Subject: below --
2009-04-04 14:35 [PATCH 0/4] ksm - dynamic page sharing driver for linux v2 Izik Eidus
2009-04-04 14:35 ` [PATCH 1/4] MMU_NOTIFIERS: add set_pte_at_notify() Izik Eidus
2009-04-04 14:35 ` [PATCH 2/4] add page_wrprotect(): write protecting page Izik Eidus
2009-04-04 14:35 ` [PATCH 3/4] add replace_page(): change the page pte is pointing to Izik Eidus
2009-04-04 14:35 ` [PATCH 4/4] add ksm kernel shared memory driver Izik Eidus
2009-04-06 9:13 ` Andrey Panin
2009-04-06 10:58 ` Izik Eidus
2009-04-09 3:58 [PATCH 0/4] ksm - dynamic page sharing driver for linux v3 Izik Eidus
2009-04-09 3:58 ` [PATCH 1/4] MMU_NOTIFIERS: add set_pte_at_notify() Izik Eidus
2009-04-09 3:58 ` [PATCH 2/4] add page_wrprotect(): write protecting page Izik Eidus
2009-04-09 3:58 ` [PATCH 3/4] add replace_page(): change the page pte is pointing to Izik Eidus
2009-04-09 3:58 ` [PATCH 4/4] add ksm kernel shared memory driver Izik Eidus
2009-04-14 22:09 ` Andrew Morton
2009-04-15 22:37 ` Izik Eidus
2009-04-15 22:50 ` Andrew Morton
2009-04-15 23:21 ` Andrea Arcangeli
2009-04-16 0:43 ` Jeremy Fitzhardinge
2009-04-16 0:57 ` Izik Eidus
2009-04-16 11:39 ` Andrea Arcangeli
2009-04-16 16:08 ` Jeremy Fitzhardinge
2009-04-18 14:58 ` Andrea Arcangeli
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=49D4A016.9040506@redhat.com \
--to=ieidus@redhat.com \
--cc=aarcange@redhat.com \
--cc=anthony@codemonkey.ws \
--cc=avi@redhat.com \
--cc=chrisw@redhat.com \
--cc=corbet@lwn.net \
--cc=dmonakhov@openvz.org \
--cc=hugh@veritas.com \
--cc=jeremy@goop.org \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=mtosatti@redhat.com \
--cc=riel@redhat.com \
--cc=yaniv@redhat.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).