| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <20120521143701.74ab2d0b.akpm@linux-foundation.org>
Date: Mon, 21 May 2012 14:37:01 -0700
From: Andrew Morton <akpm@...ux-foundation.org>
To: mingo@...hat.com, a.p.zijlstra@...llo.nl,
torvalds@...ux-foundation.org, peterz@...radead.org,
anton@...hat.com, akpm@...ux-foundation.org, rostedt@...dmis.org,
tglx@...utronix.de, oleg@...hat.com, linux-mm@...ck.org,
linux-kernel@...r.kernel.org, hpa@...or.com, jkenisto@...ibm.com,
andi@...stfloor.org, hch@...radead.org, ananth@...ibm.com,
vda.linux@...glemail.com, masami.hiramatsu.pt@...achi.com,
acme@...radead.org, srikar@...ux.vnet.ibm.com,
sfr@...b.auug.org.au, roland@...k.frob.com, mingo@...e.hu
Cc: tip-bot for Srikar Dronamraju <srikar@...ux.vnet.ibm.com>,
linux-tip-commits@...r.kernel.org, mingo@...hat.com,
torvalds@...ux-foundation.org, a.p.zijlstra@...llo.nl,
peterz@...radead.org, anton@...hat.com, rostedt@...dmis.org,
tglx@...utronix.de, oleg@...hat.com, linux-mm@...ck.org,
linux-kernel@...r.kernel.org, hpa@...or.com, jkenisto@...ibm.com,
andi@...stfloor.org, hch@...radead.org, vda.linux@...glemail.com,
ananth@...ibm.com, masami.hiramatsu.pt@...achi.com,
acme@...radead.org, roland@...k.frob.com, sfr@...b.auug.org.au,
mingo@...e.hu
Subject: Re: [tip:perf/uprobes] uprobes, mm, x86: Add the ability to install
and remove uprobes breakpoints
On Fri, 17 Feb 2012 01:58:36 -0800
tip-bot for Srikar Dronamraju <srikar@...ux.vnet.ibm.com> wrote:
> Commit-ID: 2b144498350860b6ee9dc57ff27a93ad488de5dc
> Gitweb: http://git.kernel.org/tip/2b144498350860b6ee9dc57ff27a93ad488de5dc
> Author: Srikar Dronamraju <srikar@...ux.vnet.ibm.com>
> AuthorDate: Thu, 9 Feb 2012 14:56:42 +0530
> Committer: Ingo Molnar <mingo@...e.hu>
> CommitDate: Fri, 17 Feb 2012 10:00:01 +0100
>
> uprobes, mm, x86: Add the ability to install and remove uprobes breakpoints
hm, we seem to have conflicting commits between mainline and linux-next.
During the merge window. Again. Nobody knows why this happens.
static void unmap_single_vma(struct mmu_gather *tlb,
struct vm_area_struct *vma, unsigned long start_addr,
unsigned long end_addr,
struct zap_details *details)
{
unsigned long start = max(vma->vm_start, start_addr);
unsigned long end;
if (start >= vma->vm_end)
return;
end = min(vma->vm_end, end_addr);
if (end <= vma->vm_start)
return;
<<<<<<< HEAD
=======
if (vma->vm_file)
uprobe_munmap(vma, start, end);
if (vma->vm_flags & VM_ACCOUNT)
*nr_accounted += (end - start) >> PAGE_SHIFT;
>>>>>>> linux-next/akpm-base
if (unlikely(is_pfn_mapping(vma)))
untrack_pfn_vma(vma, 0, 0);
It made me look at uprobes. Noticed a few things...
> Add uprobes support to the core kernel, with x86 support.
>
> ...
>
> +static struct rb_root uprobes_tree = RB_ROOT;
> +static DEFINE_SPINLOCK(uprobes_treelock); /* serialize rbtree access */
> +
> +#define UPROBES_HASH_SZ 13
> +/* serialize (un)register */
> +static struct mutex uprobes_mutex[UPROBES_HASH_SZ];
> +#define uprobes_hash(v) (&uprobes_mutex[((unsigned long)(v)) %\
> + UPROBES_HASH_SZ])
> +
> +/* serialize uprobe->pending_list */
> +static struct mutex uprobes_mmap_mutex[UPROBES_HASH_SZ];
> +#define uprobes_mmap_hash(v) (&uprobes_mmap_mutex[((unsigned long)(v)) %\
> + UPROBES_HASH_SZ])
Presumably these locks were hashed for scalability reasons?
If so, this won't be terribly effective when we have multiple mutexes
occupying a single cacheline - the array entries should be padded out.
Of course, that's all a complete waste of space on uniprocessor
machines, but nobody seems to think of that any more ;(
There was no need to code the accessor functions as macros. It is, as
always, better to use a nice C function which takes an argument which
is as strictly typed as possible. ie, it *could* take a void*, but it
would be better if it required an inode*.
If that makes no difference in performance testing then probably we
didn't need to hash it at all and we can go to a single lock and be
nice to uniprocessor.
>
> ...
>
> +static int read_opcode(struct mm_struct *mm, unsigned long vaddr,
> + uprobe_opcode_t *opcode)
> +{
> + struct page *page;
> + void *vaddr_new;
> + int ret;
> +
> + ret = get_user_pages(NULL, mm, vaddr, 1, 0, 0, &page, NULL);
> + if (ret <= 0)
> + return ret;
> +
> + lock_page(page);
> + vaddr_new = kmap_atomic(page);
> + vaddr &= ~PAGE_MASK;
> + memcpy(opcode, vaddr_new + vaddr, uprobe_opcode_sz);
> + kunmap_atomic(vaddr_new);
This is modifying user memory? flush_dcache_page() needed? Or perhaps
we will need different primitives to diddle the instruction memory on
architectures which care.
> + unlock_page(page);
> + put_page(page); /* we did a get_user_pages in the beginning */
> + return 0;
> +}
> +
>
> ...
>
> +int mmap_uprobe(struct vm_area_struct *vma)
> +{
> + struct list_head tmp_list;
> + struct uprobe *uprobe, *u;
> + struct inode *inode;
> + int ret = 0;
> +
> + if (!atomic_read(&uprobe_events) || !valid_vma(vma, true))
> + return ret; /* Bail-out */
> +
> + inode = vma->vm_file->f_mapping->host;
> + if (!inode)
> + return ret;
> +
> + INIT_LIST_HEAD(&tmp_list);
> + mutex_lock(uprobes_mmap_hash(inode));
> + build_probe_list(inode, &tmp_list);
> + list_for_each_entry_safe(uprobe, u, &tmp_list, pending_list) {
> + loff_t vaddr;
> +
> + list_del(&uprobe->pending_list);
> + if (!ret) {
> + vaddr = vma_address(vma, uprobe->offset);
> + if (vaddr < vma->vm_start || vaddr >= vma->vm_end) {
> + put_uprobe(uprobe);
> + continue;
> + }
> + ret = install_breakpoint(vma->vm_mm, uprobe, vma,
> + vaddr);
> + if (ret == -EEXIST)
> + ret = 0;
This now has the comment "Ignore double add:". That is a poor
comment, because it doesn't tell us *why* a double-add is ignored.
> + }
> + put_uprobe(uprobe);
> + }
> +
> + mutex_unlock(uprobes_mmap_hash(inode));
> +
> + return ret;
> +}
> +
>
> ...
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists