uprobes: Remove redundant lock_page/unlock_page

Since read_opcode() reads from the referenced page and doesnt modify
the page contents nor the page attributes, there is no need to lock
the page.

Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
uprobes: install_breakpoint() should fail if is_swbp_insn() == T

install_breakpoint() returns -EEXIST if is_swbp_insn(orig_insn) == T,
the caller treats this code as success.

This is doubly wrong. The successful return should set UPROBE_COPY_INSN,
but the real problem is that it shouldn't succeed. If the probed insn is
int3 the application should get SIGTRAP, this won't happen with uprobe.

Probably we can fix this, we can add the UPROBE_SHARED_BP flag and teach
handle_swbp/set_orig_insn to handle this case correctly. But this needs
some complications and we have other insns which can't be probed, lets
make a simple fix for now.

I think this needs a cleanup. UPROBE_COPY_INSN should die, copy_insn()
should be called by alloc_uprobe(). arch_uprobe_analyze_insn() depends
on ->mm (ia32_compat) but it is called only once.

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
uprobes: remove the unnecessary initialization in add_utask()

Trivial cleanup. No need to nullify ->active_uprobe after kzalloc().

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
uprobes: simplify the usage of uprobe->pending_list

uprobe->pending_list is only used to create the temporary list,
it has no meaning after we drop uprobes_mmap_hash(inode).

No need to initialize this node or remove it from tmp_list, and
we can use list_for_each_entry().

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
uprobes: rework register_for_each_vma() to make it O(n)

Currently register_for_each_vma() is O(n ** 2) + O(n ** 3), every
time find_next_vma_info() "restarts" the vma_prio_tree_foreach()
loop and each iteration rechecks the whole try_list. This also
means that try_list can grow "indefinitely" if register/unregister
races with munmap/mmap activity even if the number of mapping is
bounded at any time.

With this patch register_for_each_vma() builds the list of mm/vaddr
structures only once and does install_breakpoint() for each entry.

We do not care about the new mappings which can be created after
build_map_info() drops mapping->i_mmap_mutex, uprobe_mmap() should
do its work.

Note that we do not allocate map_info under i_mmap_mutex, this can
deadlock with page reclaim (but see the next patch). So we use 2
lists, "curr" which we are going to return, and "prev" which holds
the already allocated memory. The main loop deques the entry from
"prev" (initially it is empty), and if "prev" becomes empty again
it counts the number of entries we need to pre-allocate outside of
i_mmap_mutex.

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
uprobes: change build_map_info() to try kmalloc(GFP_NOWAIT) first

build_map_info() doesn't allocate the memory under i_mmap_mutex
to avoid the deadlock with page reclaim. But it can try GFP_NOWAIT
first, it should work in the likely case and thus we almost never
need the pre-alloc-and-retry path.

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
uprobes: document uprobe_register() vs uprobe_mmap() race

Because the mind is treacherous and makes us forget we need to write
stuff down.

Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
uprobes: valid_vma() should reject VM_HUGETLB

__replace_page() obviously can't work with the hugetlbfs mappings,
uprobe_register() will likely crash the kernel. Change valid_vma()
to check VM_HUGETLB as well.

As for PageTransHuge() no need to worry, vma->vm_file != NULL.

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
uprobes: __copy_insn() should ensure a_ops->readpage != NULL

__copy_insn() blindly calls read_mapping_page(), this will crash
the kernel if ->readpage == NULL, add the necessary check. For
example, hugetlbfs_aops->readpage is NULL. Perhaps we should change
read_mapping_page() instead.

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
uprobes: copy_insn() shouldn't depend on mm/vma/vaddr

1. copy_insn() doesn't need "addr", it can use uprobe->offset.
   Remove this argument.

2. Change copy_insn/__copy_insn to accept "struct file*" instead
   of vma.

copy_insn() is called only once and mm/vma/vaddr are random, it
shouldn't depend on them.

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
uprobes: copy_insn() should not return -ENOMEM if __copy_insn() fails

copy_insn() returns -ENOMEM if the first __copy_insn() fails,
it should return the correct error code.

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
uprobes: no need to re-check vma_address() in write_opcode()

write_opcode() is called by register_for_each_vma() and uprobe_mmap()
paths. In both cases the caller has already verified this vaddr under
mmap_sem, no need to re-check.

Note also that this check is wrong anyway, we should not truncate
loff_t returned by vma_address() if we do not trust this mapping.

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
uprobes: move BUG_ON(UPROBE_SWBP_INSN_SIZE) from write_opcode() to install_breakpoint()

write_opcode() ensures that UPROBE_SWBP_INSN doesn't cross the
page boundary. This looks a bit confusing, the check does not
depend on vaddr and it is enough to do it only once right after
install_breakpoint()->arch_uprobe_analyze_insn().

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
uprobes: don't use loff_t for the valid virtual address

loff_t looks confusing when it is used for the virtual address.
Change map_info and install_breakpoint/remove_breakpoint paths
to use "unsigned long".

The patch doesn't change vma_address(), it can't return "long"
because it is used to verify the mapping. But probably this
needs some cleanups too.

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
uprobes: __copy_insn() needs "loff_t offset"

1. __copy_insn() needs "loff_t offset", not "unsigned long",
   to read the file.

2. use pgoff_t for "idx" and remove the unnecessary typecast.

3. fix the typo, "&=" is not what we want

4. can't resist, rename off1 to off.

Signed-off-by: Oleg Nesterov <oleg@redhat.com>