On 03/14/2018 10:29 PM, Oleg Nesterov wrote:
> On 03/13, Ravi Bangoria wrote:
>> +static bool sdt_valid_vma(struct trace_uprobe *tu, struct vm_area_struct 
>> *vma)
>> +{
>> +    unsigned long vaddr = vma_offset_to_vaddr(vma, tu->ref_ctr_offset);
>> +
>> +    return tu->ref_ctr_offset &&
>> +            vma->vm_file &&
>> +            file_inode(vma->vm_file) == tu->inode &&
>> +            vma->vm_flags & VM_WRITE &&
>> +            vma->vm_start <= vaddr &&
>> +            vma->vm_end > vaddr;
>> +}
> Perhaps in this case a simple
>
>               ref_ctr_offset < vma->vm_end - vma->vm_start
>
> check without vma_offset_to_vaddr() makes more sense, but I won't insist.
>

Hmm... I'm not quite sure. Will rethink and get back to you.

>
>> +static void sdt_increment_ref_ctr(struct trace_uprobe *tu)
>> +{
>> +    struct uprobe_map_info *info;
>> +    struct vm_area_struct *vma;
>> +    unsigned long vaddr;
>> +
>> +    uprobe_start_dup_mmap();
>> +    info = uprobe_build_map_info(tu->inode->i_mapping,
>> +                            tu->ref_ctr_offset, false);
> Hmm. This doesn't look right.
>
> If you need to find all mappings (and avoid the races with fork/dup_mmap) you
> need to take this semaphore for writing, uprobe_start_dup_mmap() can't help.

Oops. Yes. Will change it.

Thanks for the review :)
Ravi

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to