From owner-freebsd-hackers@FreeBSD.ORG Sat Nov 5 15:40:29 2011 Return-Path: Delivered-To: freebsd-hackers@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 3652A106564A; Sat, 5 Nov 2011 15:40:29 +0000 (UTC) (envelope-from to.my.trociny@gmail.com) Received: from mail-fx0-f54.google.com (mail-fx0-f54.google.com [209.85.161.54]) by mx1.freebsd.org (Postfix) with ESMTP id 905B08FC1B; Sat, 5 Nov 2011 15:40:28 +0000 (UTC) Received: by faar19 with SMTP id r19so5374879faa.13 for ; Sat, 05 Nov 2011 08:40:27 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=from:to:cc:subject:references:x-comment-to:sender:date:message-id :user-agent:mime-version:content-type; bh=1WGAzYG7WwSVXRSN1FipqA+gub0PuUjXqN3XCubxbzw=; b=t2VL6I8dLxwWSOFoc89/U+yJrF6rBDTETRecJgLrxgVoEd8jE8tTcZE0qzZb+HZBqQ VKbQaBkwzHx3AbTECJ+RPt00OJprBtSHTpavXNo71lIwHDwNMDYSU7zDCEo0v/B6sySA tlep1rZb/IyJge5C2TbVh/3yzGjYwEiWdBJO0= Received: by 10.223.63.206 with SMTP id c14mr32322899fai.3.1320507627356; Sat, 05 Nov 2011 08:40:27 -0700 (PDT) Received: from localhost ([95.69.173.122]) by mx.google.com with ESMTPS id t28sm1116885faf.13.2011.11.05.08.40.24 (version=TLSv1/SSLv3 cipher=OTHER); Sat, 05 Nov 2011 08:40:25 -0700 (PDT) From: Mikolaj Golub To: Kostik Belousov References: <86y5wkeuw9.fsf@kopusha.home.net> <20111016171005.GB50300@deviant.kiev.zoral.com.ua> <86aa8qozyx.fsf@kopusha.home.net> <20111025082451.GO50300@deviant.kiev.zoral.com.ua> <86aa8k2im0.fsf@kopusha.home.net> <20111031094948.GB50300@deviant.kiev.zoral.com.ua> <86vcr21agm.fsf@kopusha.home.net> <20111105135801.GT50300@deviant.kiev.zoral.com.ua> X-Comment-To: Kostik Belousov Sender: Mikolaj Golub Date: Sat, 05 Nov 2011 17:40:22 +0200 Message-ID: <86ehxmpogp.fsf@kopusha.home.net> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.2 (berkeley-unix) MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: freebsd-hackers@freebsd.org, Robert Watson Subject: Re: "ps -e" without procfs(5) X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 05 Nov 2011 15:40:29 -0000 On Sat, 5 Nov 2011 15:58:01 +0200 Kostik Belousov wrote: KB> + if (error == EFAULT) { KB> + for (i = 0; i < len; i++) { KB> + c = fubyte(sptr + i); KB> + if (c < 0) KB> As a purely stylistical issue, compare with -1. KB> + return (EFAULT); KB> + buf[i] = (char)c; KB> + if (c == '\0') KB> + break; KB> + } KB> + error = 0; KB> + } KB> + return error; KB> Put () around error. Thanks. KB> + /* KB> + * Check that that the address is in user space. KB> + */ KB> + if (vptr + 1 < VM_MIN_ADDRESS + 1 || vptr >= VM_MAXUSER_ADDRESS) KB> + return (ENOEXEC); KB> Why is this needed ? I saw this check in libkvm for ps_argvstr and ps_envstr and decided to add it too. Just some additional check that ps_string fields, which can be overwritten by the user, look reasonable. If you think this is not very useful I remove it. KB> I think that the aux vector must be naturally aligned. You can return KB> ENOEXEC early if vptr is not aligned. Not sure I see what you mean. vptr for auxv is calculated just couple lines above, and I check the result here, in the part common for all vector types. BTW, investigating the cases when I got procstat: sysctl: kern.proc.args: 58002: 8: Exec format error they were because the PROC_VECTOR_MAX limit (512 entries, as it is in linprocfs and libkvm) is small for real world cases: get_proc_vector(pid = rm[47883], type = 0): vsize (3009) > PROC_VECTOR_MAX (512)) get_proc_vector(pid = rm[47883], type = 0): vsize (3009) > PROC_VECTOR_MAX (512)) get_proc_vector(pid = rm[47890], type = 0): vsize (3008) > PROC_VECTOR_MAX (512)) get_proc_vector(pid = rm[47890], type = 0): vsize (3008) > PROC_VECTOR_MAX (512)) get_proc_vector(pid = rm[47897], type = 0): vsize (4511) > PROC_VECTOR_MAX (512)) get_proc_vector(pid = rm[47897], type = 0): vsize (4511) > PROC_VECTOR_MAX (512)) get_proc_vector(pid = rm[47897], type = 0): vsize (4511) > PROC_VECTOR_MAX (512)) get_proc_vector(pid = rm[48044], type = 0): vsize (611) > PROC_VECTOR_MAX (512)) get_proc_vector(pid = rm[52189], type = 0): vsize (772) > PROC_VECTOR_MAX (512)) get_proc_vector(pid = rm[52192], type = 0): vsize (1157) > PROC_VECTOR_MAX (512)) get_proc_vector(pid = rm[55685], type = 0): vsize (1041) > PROC_VECTOR_MAX (512)) get_proc_vector(pid = rm[55687], type = 0): vsize (1040) > PROC_VECTOR_MAX (512)) get_proc_vector(pid = rm[55690], type = 0): vsize (1559) > PROC_VECTOR_MAX (512)) So I am going to change it to ARG_MAX and use independent limit (256 entries) for auxv. KB> Why the blank after the loop statement in get_ps_strings() ? Sorry, what blank you mean? I don't see it in get_ps_strings(). May be you mean the blank line in get_proc_vector() before return? KB> There shall be blank lines after the '{' in proc_getargv() and proc_getenvv(). Ah, sure. KB> Note that using cached pargs is somewhat inconsistent with the digging KB> into ps_strings. KB> procfs_doproccmdline() can benefit from your work. Thanks, I will look at it :-). -- Mikolaj Golub