From owner-svn-src-all@FreeBSD.ORG Thu Feb 11 04:20:39 2010 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 5F1701065672; Thu, 11 Feb 2010 04:20:39 +0000 (UTC) (envelope-from xcllnt@mac.com) Received: from asmtpout024.mac.com (asmtpout024.mac.com [17.148.16.99]) by mx1.freebsd.org (Postfix) with ESMTP id 396838FC13; Thu, 11 Feb 2010 04:20:38 +0000 (UTC) MIME-version: 1.0 Content-type: multipart/mixed; boundary="Boundary_(ID_UnsDNPARCItTutJo6OmNpw)" Received: from macbook-pro.lan.xcllnt.net (mail.xcllnt.net [75.101.29.67]) by asmtp024.mac.com (Sun Java(tm) System Messaging Server 6.3-8.01 (built Dec 16 2008; 32bit)) with ESMTPSA id <0KXN009Y9TED1Q30@asmtp024.mac.com>; Wed, 10 Feb 2010 20:20:38 -0800 (PST) X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 spamscore=0 ipscore=0 phishscore=0 bulkscore=0 adultscore=0 classifier=spam adjust=0 reason=mlx engine=5.0.0-0908210000 definitions=main-1002100279 From: Marcel Moolenaar In-reply-to: <20100210091522.GW9991@deviant.kiev.zoral.com.ua> Date: Wed, 10 Feb 2010 20:20:37 -0800 Message-id: References: <201002090552.o195qZcD074581@svn.freebsd.org> <20100209095722.GQ9991@deviant.kiev.zoral.com.ua> <65DCE552-7EFD-48F2-85A4-EA0F1F0638EE@mac.com> <20100209184043.GV9991@deviant.kiev.zoral.com.ua> <896B58E6-12EA-48AB-86C2-5BA9F0C59512@mac.com> <86989446-64EF-411F-8E25-173DB6AEE10B@mac.com> <20100210091522.GW9991@deviant.kiev.zoral.com.ua> To: Kostik Belousov X-Mailer: Apple Mail (2.1077) Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r203696 - in head: lib/libc/sys sys/kern sys/sys X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 11 Feb 2010 04:20:39 -0000 --Boundary_(ID_UnsDNPARCItTutJo6OmNpw) Content-type: text/plain; charset=us-ascii Content-transfer-encoding: 7BIT On Feb 10, 2010, at 1:15 AM, Kostik Belousov wrote: > > Vnode locks are before vm map locks in global lock order. vn_fullpath() > may need to lock vnodes to call VOP_VPTOCNP(). I think you should (and > can) drop both vm map lock and vmspace reference much earlier. > > Would it be cleaner to use explicitely sized types for compat32 > structure members ? I don't know. I prefer to keep them identical for as much as that's possible. > Comparing ptrace_vm_entry with kinfo_vmentry, I think that it might > be good idea to add fsid and inode number to ptrace_vm_entry, to > give at least some information when vn_fullpath failed. How about the attached new path (includes man page as well)? -- Marcel Moolenaar xcllnt@mac.com --Boundary_(ID_UnsDNPARCItTutJo6OmNpw) Content-type: application/octet-stream; x-unix-mode=0644; name=ptrace.diff Content-transfer-encoding: 7bit Content-disposition: attachment; filename=ptrace.diff Index: lib/libc/sys/ptrace.2 =================================================================== --- lib/libc/sys/ptrace.2 (revision 203696) +++ lib/libc/sys/ptrace.2 (working copy) @@ -343,23 +343,30 @@ which is defined as follows: .Bd -literal struct ptrace_vm_entry { - void *pve_cookie; - u_long pve_start; - u_long pve_end; - u_long pve_offset; - u_int pve_prot; - u_int pve_pathlen; - char *pve_path; + int pve_entry; + int pve_timestamp; + u_long pve_start; + u_long pve_end; + u_long pve_offset; + u_int pve_prot; + u_int pve_pathlen; + long pve_fileid; + uint32_t pve_fsid; + char *pve_path; }; .Ed .Pp The first entry is returned by setting -.Va pve_cookie -to -.Dv NULL . +.Va pve_entry +to zero. Subsequent entries are returned by leaving -.Va pve_cookie +.Va pve_entry unmodified from the value returned by previous requests. +The +.Va pve_timestamp +field can be used to detect changes to the VM map while iterating over the +entries. +The tracing process can then take appropriate action, such as restarting. By setting .Va pve_pathlen to a non-zero value on entry, the pathname of the backing object is returned @@ -434,7 +441,8 @@ .It .Dv PT_VM_ENTRY was given an invalid value for -.Fa pve_cookie . +.Fa pve_entry . +This can also be caused by changes to the VM map of the process. .El .It Bq Er EBUSY .Bl -bullet -compact Index: sys/kern/sys_process.c =================================================================== --- sys/kern/sys_process.c (revision 203724) +++ sys/kern/sys_process.c (working copy) @@ -75,12 +75,15 @@ }; struct ptrace_vm_entry32 { - uint32_t pve_cookie; + int pve_entry; + int pve_timestamp; uint32_t pve_start; uint32_t pve_end; uint32_t pve_offset; u_int pve_prot; u_int pve_pathlen; + int32_t pve_fileid; + u_int pve_fsid; uint32_t pve_path; }; @@ -360,88 +363,141 @@ static int ptrace_vm_entry(struct thread *td, struct proc *p, struct ptrace_vm_entry *pve) { + struct vattr vattr; vm_map_t map; vm_map_entry_t entry; vm_object_t obj, tobj, lobj; + struct vmspace *vm; struct vnode *vp; char *freepath, *fullpath; u_int pathlen; - int error, vfslocked; + int error, index, vfslocked; - map = &p->p_vmspace->vm_map; - entry = map->header.next; - if (pve->pve_cookie != NULL) { - while (entry != &map->header && entry != pve->pve_cookie) + error = 0; + obj = NULL; + + vm = vmspace_acquire_ref(p); + map = &vm->vm_map; + vm_map_lock_read(map); + + do { + entry = map->header.next; + index = 0; + while (index < pve->pve_entry && entry != &map->header) { entry = entry->next; - if (entry != pve->pve_cookie) - return (EINVAL); - entry = entry->next; - } - while (entry != &map->header && (entry->eflags & MAP_ENTRY_IS_SUB_MAP)) - entry = entry->next; - if (entry == &map->header) - return (ENOENT); + index++; + } + if (index != pve->pve_entry) { + error = EINVAL; + break; + } + while (entry != &map->header && + (entry->eflags & MAP_ENTRY_IS_SUB_MAP) != 0) { + entry = entry->next; + index++; + } + if (entry == &map->header) { + error = ENOENT; + break; + } - /* We got an entry. */ - pve->pve_cookie = entry; - pve->pve_start = entry->start; - pve->pve_end = entry->end - 1; - pve->pve_offset = entry->offset; - pve->pve_prot = entry->protection; + /* We got an entry. */ + pve->pve_entry = index + 1; + pve->pve_timestamp = map->timestamp; + pve->pve_start = entry->start; + pve->pve_end = entry->end - 1; + pve->pve_offset = entry->offset; + pve->pve_prot = entry->protection; - /* Backing object's path needed? */ - if (pve->pve_pathlen == 0) - return (0); + /* Backing object's path needed? */ + if (pve->pve_pathlen == 0) + break; - pathlen = pve->pve_pathlen; - pve->pve_pathlen = 0; + pathlen = pve->pve_pathlen; + pve->pve_pathlen = 0; - obj = entry->object.vm_object; - if (obj == NULL) - return (0); + obj = entry->object.vm_object; + if (obj != NULL) + VM_OBJECT_LOCK(obj); + } while (0); - VM_OBJECT_LOCK(obj); - for (lobj = tobj = obj; tobj; tobj = tobj->backing_object) { - if (tobj != obj) - VM_OBJECT_LOCK(tobj); - if (lobj != obj) - VM_OBJECT_UNLOCK(lobj); - lobj = tobj; - pve->pve_offset += tobj->backing_object_offset; - } - if (lobj != NULL) { + vm_map_unlock_read(map); + vmspace_free(vm); + + if (error == 0 && obj != NULL) { + lobj = obj; + for (tobj = obj; tobj != NULL; tobj = tobj->backing_object) { + if (tobj != obj) + VM_OBJECT_LOCK(tobj); + if (lobj != obj) + VM_OBJECT_UNLOCK(lobj); + lobj = tobj; + pve->pve_offset += tobj->backing_object_offset; + } vp = (lobj->type == OBJT_VNODE) ? lobj->handle : NULL; if (vp != NULL) vref(vp); if (lobj != obj) VM_OBJECT_UNLOCK(lobj); VM_OBJECT_UNLOCK(obj); - } else - vp = NULL; - if (vp == NULL) - return (0); + if (vp != NULL) { + freepath = NULL; + fullpath = NULL; + vn_fullpath(td, vp, &fullpath, &freepath); + vfslocked = VFS_LOCK_GIANT(vp->v_mount); + vn_lock(vp, LK_SHARED | LK_RETRY); + if (VOP_GETATTR(vp, &vattr, td->td_ucred) == 0) { + pve->pve_fileid = vattr.va_fileid; + pve->pve_fsid = vattr.va_fsid; + } + vput(vp); + VFS_UNLOCK_GIANT(vfslocked); - freepath = NULL; - fullpath = NULL; - vn_fullpath(td, vp, &fullpath, &freepath); - vfslocked = VFS_LOCK_GIANT(vp->v_mount); - vrele(vp); - VFS_UNLOCK_GIANT(vfslocked); + if (fullpath != NULL) { + pve->pve_pathlen = strlen(fullpath) + 1; + if (pve->pve_pathlen <= pathlen) { + error = copyout(fullpath, pve->pve_path, + pve->pve_pathlen); + } else + error = ENAMETOOLONG; + } + if (freepath != NULL) + free(freepath, M_TEMP); + } + } - error = 0; - if (fullpath != NULL) { - pve->pve_pathlen = strlen(fullpath) + 1; - if (pve->pve_pathlen <= pathlen) { - error = copyout(fullpath, pve->pve_path, - pve->pve_pathlen); - } else - error = ENAMETOOLONG; + return (error); +} + +#ifdef COMPAT_IA32 +static int +ptrace_vm_entry32(struct thread *td, struct proc *p, + struct ptrace_vm_entry32 *pve32) +{ + struct ptrace_vm_entry pve; + int error; + + pve.pve_entry = pve32->pve_entry; + pve.pve_pathlen = pve32->pve_pathlen; + pve.pve_path = (void *)(uintptr_t)pve32->pve_path; + + error = ptrace_vm_entry(td, p, &pve); + if (error == 0) { + pve32->pve_entry = pve.pve_entry; + pve32->pve_timestamp = pve.pve_timestamp; + pve32->pve_start = pve.pve_start; + pve32->pve_end = pve.pve_end; + pve32->pve_offset = pve.pve_offset; + pve32->pve_prot = pve.pve_prot; + pve32->pve_fileid = pve.pve_fileid; + pve32->pve_fsid = pve.pve_fsid; } - if (freepath != NULL) - free(freepath, M_TEMP); + + pve32->pve_pathlen = pve.pve_pathlen; return (error); } +#endif /* COMPAT_IA32 */ /* * Process debugging system call. @@ -1087,14 +1143,12 @@ break; case PT_VM_ENTRY: + PROC_UNLOCK(p); #ifdef COMPAT_IA32 - /* XXX to be implemented. */ - if (wrap32) { - error = EDOOFUS; - break; - } + if (wrap32) + error = ptrace_vm_entry32(td, p, addr); + else #endif - PROC_UNLOCK(p); error = ptrace_vm_entry(td, p, addr); PROC_LOCK(p); break; Index: sys/sys/ptrace.h =================================================================== --- sys/sys/ptrace.h (revision 203724) +++ sys/sys/ptrace.h (working copy) @@ -104,13 +104,16 @@ /* Argument structure for PT_VM_ENTRY. */ struct ptrace_vm_entry { - void *pve_cookie; /* Token used to iterate. */ - u_long pve_start; /* Start VA of range. */ - u_long pve_end; /* End VA of range (incl). */ - u_long pve_offset; /* Offset in backing object. */ - u_int pve_prot; /* Protection of memory range. */ - u_int pve_pathlen; /* Size of path. */ - char *pve_path; /* Path name of object. */ + int pve_entry; /* Entry number used for iteration. */ + int pve_timestamp; /* Generation number of VM map. */ + u_long pve_start; /* Start VA of range. */ + u_long pve_end; /* End VA of range (incl). */ + u_long pve_offset; /* Offset in backing object. */ + u_int pve_prot; /* Protection of memory range. */ + u_int pve_pathlen; /* Size of path. */ + long pve_fileid; /* File ID. */ + uint32_t pve_fsid; /* File system ID. */ + char *pve_path; /* Path name of object. */ }; #ifdef _KERNEL --Boundary_(ID_UnsDNPARCItTutJo6OmNpw) Content-type: text/plain; charset=us-ascii Content-transfer-encoding: 7BIT --Boundary_(ID_UnsDNPARCItTutJo6OmNpw)--