From owner-svn-src-all@freebsd.org Sun Feb 21 16:47:27 2016 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 089CFAAF6F0; Sun, 21 Feb 2016 16:47:27 +0000 (UTC) (envelope-from mjguzik@gmail.com) Received: from mail-wm0-x22f.google.com (mail-wm0-x22f.google.com [IPv6:2a00:1450:400c:c09::22f]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 94A6A1509; Sun, 21 Feb 2016 16:47:26 +0000 (UTC) (envelope-from mjguzik@gmail.com) Received: by mail-wm0-x22f.google.com with SMTP id a4so132652423wme.1; Sun, 21 Feb 2016 08:47:26 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=date:from:to:cc:subject:message-id:references:mime-version :content-type:content-disposition:content-transfer-encoding :in-reply-to:user-agent; bh=EkA8UInL4aAJfTqbiescXULKMKboXFxWFP4V+OJej+k=; b=vpJlc14N4HlSimDJFcp3xCuQYCRGQy93ca9dxIebdr499txUqbzqOony0mQe7pGr/C JkLFj+ePRTROwi36XPSJy2KIrkqe4aaw9ZaxGhWnY1bb7NFx6ze8g6Zjpia/1hLI8hQr N8YxC6ioW0fFsqrz3J6WkhH/U/LzUz4lIEWq+6AK5B1IqtjUZ1kjGmen01UfERO+N2l1 WZApw8P54DbOfwJni1LVPl6XBa2tPxugb+iHC45GaLZskgFdkLREU6LOInXUyXPxCHu5 2Gu5kR0R19Z/rFPFCrwXX7yYXj2gqHG6XJXnnTdTcmIYQb/x+Qx9KboRiH2B9QllOwTj o3tg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-type:content-disposition :content-transfer-encoding:in-reply-to:user-agent; bh=EkA8UInL4aAJfTqbiescXULKMKboXFxWFP4V+OJej+k=; b=LpHW4H3ahvqPhXR7Sftm5rHLNSiMaIGZ1eWDrObiyH4dZWpEWZGcz84A1yW4aw8fNR mFZmfKW64dKL8UUILlaFW/WWUuU7sE4decxESZuV4cf7M6oKb7ZBOJR6QFcMBREcxbRF Bu0hgTwIMeA+C4Xqzzg+lGEh2ynlgACvSxOfulVxtv0obQNcCl9RjzNAB0eu48fU0eqP URyJx9CGkTfLW84IeSVwAdcCjxR4Nr4Az37IQai5kwgQBemtPO6U9uxFChCoxhak7JRj VD8744+Kft949Lg6v8SiZLSlTB9gZfQ8XjrKbuhQ5RcMcpgNYEqIXj34vue5yhDHEyr0 thCg== X-Gm-Message-State: AG10YOTnHXyqamKyMlafD7EO5OJ9sMGOGKPuq/md2jONPXWdvrHIhz1vIVgC2E2pmxh5YA== X-Received: by 10.194.190.6 with SMTP id gm6mr24148662wjc.115.1456073245152; Sun, 21 Feb 2016 08:47:25 -0800 (PST) Received: from dft-labs.eu (n1x0n-1-pt.tunnel.tserv5.lon1.ipv6.he.net. [2001:470:1f08:1f7::2]) by smtp.gmail.com with ESMTPSA id i5sm20689876wja.23.2016.02.21.08.47.23 (version=TLS1_2 cipher=AES128-SHA bits=128/128); Sun, 21 Feb 2016 08:47:24 -0800 (PST) Date: Sun, 21 Feb 2016 17:47:22 +0100 From: Mateusz Guzik To: Dag-Erling =?utf-8?B?U23DuHJncmF2?= Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org, Szymon =?utf-8?B?xZpsaXdh?= Subject: Re: svn commit: r295856 - head/sys/compat/linprocfs Message-ID: <20160221164722.GB4399@dft-labs.eu> References: <201602211456.u1LEu57C069842@repo.freebsd.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <201602211456.u1LEu57C069842@repo.freebsd.org> User-Agent: Mutt/1.5.21 (2010-09-15) X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.20 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: Sun, 21 Feb 2016 16:47:27 -0000 On Sun, Feb 21, 2016 at 02:56:05PM +0000, Dag-Erling Smørgrav wrote: First of all I received this patch over a month ago and then I forgot a about it, for which I have to apologize. Now back to business. > Author: des > Date: Sun Feb 21 14:56:05 2016 > New Revision: 295856 > URL: https://svnweb.freebsd.org/changeset/base/295856 > > Log: > Implement /proc/$$/limits. > > PR: 207386 > Submitted by: Szymon Śliwa > MFC after: 3 weeks > > Modified: > head/sys/compat/linprocfs/linprocfs.c > > Modified: head/sys/compat/linprocfs/linprocfs.c > ============================================================================== > --- head/sys/compat/linprocfs/linprocfs.c Sun Feb 21 14:50:37 2016 (r295855) > +++ head/sys/compat/linprocfs/linprocfs.c Sun Feb 21 14:56:05 2016 (r295856) > @@ -61,6 +61,7 @@ __FBSDID("$FreeBSD$"); > #include > #include > #include > +#include > #include > #include > #include > @@ -1366,6 +1367,67 @@ linprocfs_dofdescfs(PFS_FILL_ARGS) > return (0); > } > > +/* > + * Filler function for proc/pid/limits > + */ > + > +#define RLIM_NONE -1 > + > +static const struct limit_info { > + const char *desc; > + const char *unit; > + unsigned long long rlim_id; > +} limits_info[] = { > + { "Max cpu time", "seconds", RLIMIT_CPU }, > + { "Max file size", "bytes", RLIMIT_FSIZE }, > + { "Max data size", "bytes", RLIMIT_DATA }, > + { "Max stack size", "bytes", RLIMIT_STACK }, > + { "Max core file size", "bytes", RLIMIT_CORE }, > + { "Max resident set", "bytes", RLIMIT_RSS }, > + { "Max processes", "processes", RLIMIT_NPROC }, > + { "Max open files", "files", RLIMIT_NOFILE }, > + { "Max locked memory", "bytes", RLIMIT_MEMLOCK }, > + { "Max address space", "bytes", RLIMIT_AS }, > + { "Max file locks", "locks", RLIM_INFINITY }, > + { "Max pending signals", "signals", RLIM_INFINITY }, > + { "Max msgqueue size", "bytes", RLIM_NONE }, > + { "Max nice priority", "", RLIM_NONE }, > + { "Max realtime priority", "", RLIM_NONE }, > + { "Max realtime timeout", "us", RLIM_INFINITY }, > + { 0, 0, 0 } > +}; > + > +static int > +linprocfs_doproclimits(PFS_FILL_ARGS) > +{ > + const struct limit_info *li; > + struct rlimit li_rlimits; > + struct plimit *cur_proc_lim; > + 'cur' is a bad part for a name here. For instance 'curthread' refers to the thread executing the code. Now 'cur' strongly implies this is a pointer to limits from the current thread. This also is not the standard name used for pointers to plimit. > + cur_proc_lim = lim_alloc(); I presume this function is called with the process unlocked. If it was locked, bound sleep property of the mtx would conflict with unbodung sleep possibly induced by lim_alloc. But there is no reason to allocate anything. > + lim_copy(cur_proc_lim, p->p_limit); If the process is unlocked, this has to be wrong. The target process may be in the process of changing its limits leading to this code reading a pointer to a buffer which is freed before lim_copy accesses it. The limit structure is maintained with a reference counter. Thus the correct thing to do is to lock the process, grab the reference, unlock the process, do what you need to do with the structure and finally unref it. Interestingly the correct thing to do is implemented here: http://fxr.watson.org/fxr/source/fs/procfs/procfs_rlimit.c#L112 I have not verified correctness of the rest of the patch. > + sbuf_printf(sb, "%-26s%-21s%-21s%-10s\n", "Limit", "Soft Limit", > + "Hard Limit", "Units"); > + for (li = limits_info; li->desc != NULL; ++li) { > + if (li->rlim_id != RLIM_INFINITY && li->rlim_id != RLIM_NONE) > + li_rlimits = cur_proc_lim->pl_rlimit[li->rlim_id]; > + else { > + li_rlimits.rlim_cur = 0; > + li_rlimits.rlim_max = 0; > + } > + if (li->rlim_id == RLIM_INFINITY || > + li_rlimits.rlim_cur == RLIM_INFINITY) > + sbuf_printf(sb, "%-26s%-21s%-21s%-10s\n", > + li->desc, "unlimited", "unlimited", li->unit); > + else > + sbuf_printf(sb, "%-26s%-21ld%-21ld%-10s\n", > + li->desc, (long)li_rlimits.rlim_cur, > + (long)li_rlimits.rlim_max, li->unit); > + } > + lim_free(cur_proc_lim); > + return (0); > +} > + > > /* > * Filler function for proc/sys/kernel/random/uuid > @@ -1504,6 +1566,8 @@ linprocfs_init(PFS_INIT_ARGS) > NULL, NULL, NULL, 0); > pfs_create_file(dir, "auxv", &linprocfs_doauxv, > NULL, &procfs_candebug, NULL, PFS_RD|PFS_RAWRD); > + pfs_create_file(dir, "limits", &linprocfs_doproclimits, > + NULL, NULL, NULL, PFS_RD); > > /* /proc/scsi/... */ > dir = pfs_create_dir(root, "scsi", NULL, NULL, NULL, 0); -- Mateusz Guzik