From: Al Viro <viro@ZenIV.linux.org.uk>
To: linux-kernel@vger.kernel.org
Cc: linux-fsdevel@vger.kernel.org
Subject: [sigh] binder shite is back ;-/
Date: Thu, 22 Dec 2011 19:58:07 +0000 [thread overview]
Message-ID: <20111222195807.GP23916@ZenIV.linux.org.uk> (raw)
Thesis: binder is a true example of what "Enterprise Quality" means.
Nothing good, that is.
Exhibit 1:
in their ->mmap() they have
if (proc->buffer) {
ret = -EBUSY;
failure_string = "already mapped";
goto err_already_mapped;
}
area = get_vm_area(vma->vm_end - vma->vm_start, VM_IOREMAP);
if (area == NULL) {
ret = -ENOMEM;
failure_string = "get_vm_area";
goto err_get_vm_area_failed;
}
proc->buffer = area->addr;
Note that there's nothing stopping two processes from doing mmap() on the
same file at the same time.
Exhibit 2: while proc->files is set to the ->files of whoever does mmap(),
proc->tsk is set to task_struct of whoever does open(). Then
task_get_unused_fd_flags() does expand ->files after having checked
rlimit on ->tsk.
Exhibit 3:
if (vma)
mm = NULL;
else
mm = get_task_mm(proc->tsk);
if (mm) {
down_write(&mm->mmap_sem);
vma = proc->vma;
}
Note that proc->vma is created by whoever had done mmap(), _not_ whoever
had done open(). Not to mention that there's no reason why either task would
still have the original ->mm...
Exhibit 4: mmap that thing and then fork(). Leaving aside the joy of
getting stack dumps in your face, it will succeed. And then when you
unmap that thing in child (or child exits, for that matter) you'll get
proc->vma = NULL. On a live one.
Exhibit 5: while we are at it, mmap one and then munmap its middle.
As far as I can see, the basic attitude in that code is "surely, nobody
will do anything unexpected to poor li'l me". As far as I can tell,
manipulations of descriptor tables suffer the same kind of braindamage,
etc.
next reply other threads:[~2011-12-22 19:58 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-12-22 19:58 Al Viro [this message]
2011-12-22 20:51 ` [sigh] binder shite is back ;-/ Christoph Hellwig
2011-12-22 21:00 ` Al Viro
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=20111222195807.GP23916@ZenIV.linux.org.uk \
--to=viro@zeniv.linux.org.uk \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
/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