Messages in this thread | | | Date | Sun, 26 Nov 2023 11:04:52 +0800 | Subject | Re: [PATCH] prctl: Get private anonymous memory region name | From | Rong Tao <> |
| |
On 11/25/23 23:56, Oleg Nesterov wrote: > On 11/25, Rong Tao wrote: >> +static int prctl_get_vma(unsigned long opt, unsigned long addr, >> + unsigned long buf, unsigned long arg) >> +{ >> + struct mm_struct *mm = current->mm; >> + const char __user *u_buf; >> + int error; >> + >> + switch (opt) { >> + case PR_GET_VMA_ANON_NAME: >> + const struct anon_vma_name *anon_name = NULL; >> + >> + u_buf = (const char __user *)buf; >> + error = 0; >> + >> + mmap_read_lock(mm); >> + anon_name = madvise_get_anon_name(mm, addr); >> + if (!anon_name) { >> + mmap_read_unlock(mm); >> + error = -EFAULT; > may be another error code makes sense to distinguish this case from > the copy_to_user() failure? > >> + break; >> + } >> + >> + if (copy_to_user((char __user *)u_buf, anon_name->name, >> + strlen(anon_name->name) + 1)) >> + error = -EFAULT; > and I guess you can simplify this code a bit, > > anon_name = madvise_get_anon_name(...); > if (!anon_name || copy_to_user(...)) > error = -EFAULT; Thanks a lot, I just modify the code, please review. > > mmap_read_unlock(mm); > anon_vma_name_put(anon_name); // safe if anon_name == NULL; > >> +const struct anon_vma_name *madvise_get_anon_name(struct mm_struct *mm, >> + unsigned long start) >> +{ >> + struct vm_area_struct *vma; >> + struct anon_vma_name *anon_name; >> + >> + vma = find_vma(mm, start); >> + if (vma) { >> + anon_name = anon_vma_name(vma); >> + if (anon_name) { >> + anon_vma_name_get(anon_name); >> + return anon_name; >> + } >> + } >> + >> + return NULL; > Again, afaics this can be simplified, > > struct anon_vma_name *anon_name = NULL; > > vma = find_vma(mm, start); > if (vma) { > anon_name = anon_vma_name(vma); > anon_vma_name_get(anon_name); > } > > return anon_name; > > Oleg. Thanks again.
Rong Tao
>
| |