All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@elte.hu>
To: Neil Horman <nhorman@tuxdriver.com>
Cc: linux-kernel@vger.kernel.org, akpm@linux-foundation.org,
	marcin.slusarz@gmail.com, tglx@linutronix.de, mingo@redhat.com,
	hpa@zytor.com, Linus Torvalds <torvalds@linux-foundation.org>
Subject: Re: [PATCH 0/3] extend get/setrlimit to support setting rlimits external to a process (v7)
Date: Mon, 2 Nov 2009 16:25:20 +0100	[thread overview]
Message-ID: <20091102152520.GG23776@elte.hu> (raw)
In-Reply-To: <20091020005214.GA8886@localhost.localdomain>


* Neil Horman <nhorman@tuxdriver.com> wrote:

> Ok, I give.  I was hoping that some of the requestors of this feature 
> would pipe up and support the use case for the proc file interface to 
> set limits.  clearly they're not that interested, but I still think 
> theres merit in the patch.  So heres version 7 of this patch set.  Its 
> the same as before, but the proc interface has been dropped, leaving 
> only the syscall interface behind.  I've tested the interface on intel 
> 32 and 64 bit, with success
> 
> Summary:
> 
> Its been requested often that we have the ability to read and modify 
> process rlimit values from contexts external to the owning process.  
> Ideally this allows sysadmins to adjust rlimits on long running 
> processes wihout the need to stop and restart those processes, which 
> incurs undesireable downtime.  This patch enables that functionality, 
> It does so in two places.  First it enables process limit setting by 
> writing to the /proc/pid/limits file a string in the format: <limit> 
> <current limit> <max limit> > /proc/<pid>/limits where limit is one of 
> [as,core,cpu,data,fsize,locks,memlock,msgqueue,nice,nofile,nproc,rss,rtprio,rttime]
> 
> Secondly it allows for programatic setting of these limits via 2 new 
> syscalls, getprlimit, and setprlimit, which act in an identical 
> fashion to getrlimit and setrlimit respectively, except that they 
> except a process id as an extra argument, to specify the process id of 
> the rlimit values that you wish to read/write

This looks potentially useful but i think the implementation might be 
too optimistic, from a security POV.

Have you ensured that no rlimit gets propagated during task init into 
some other value - under the previously correct assumption that rlimits 
dont change asynchronously under the feet of tasks?

Also, there's SMP safety: right now all the accesses to 
current->signal->rlim[] are unlocked and assume that if we are executing 
in a syscall those values cannot change. Is this a safe assumption on 
all SMP architectures?

Plus, the locking looks structured in a weird way: why is the 
sighand-lock taken in the procfs code instead of moving it where the 
data structure is updated (the resource limits code).

Also, a patch submission observation: every single patch you submitted 
here had a messed up title that had a 'Re: ' in it, making it hard to 
sort out what is the latest. Some of the patches also had their 
changelog indented. Please use the standard patch submission methods.

So this patch-set needs more work.

Thanks,

	Ingo

  parent reply	other threads:[~2009-11-02 15:25 UTC|newest]

Thread overview: 107+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-09-28 20:06 [PATCH] proc: augment /proc/pid/limits to allow setting of process limits Neil Horman
2009-09-28 22:44 ` Andrew Morton
2009-09-29  1:14   ` Neil Horman
2009-09-29 20:25   ` [PATCH] proc: augment /proc/pid/limits to allow setting of process limits (v2) Neil Horman
2009-09-29 20:46     ` Andrew Morton
2009-09-30  0:59       ` Neil Horman
2009-10-01 17:15 ` [PATCH 0/3] extend get/setrlimit to support setting rlimits external to a process (v3) Neil Horman
2009-10-01 17:16   ` [PATCH 1/3] " Neil Horman
2009-10-04 12:14     ` Marcin Slusarz
2009-10-04 16:50       ` Neil Horman
2009-10-04 20:04         ` Marcin Slusarz
2009-10-04 23:10           ` Neil Horman
2009-10-04 20:30     ` Marcin Slusarz
2009-10-01 17:21   ` [PATCH 2/3] " Neil Horman
2009-10-01 17:22   ` [PATCH 3/3] " Neil Horman
2009-10-05  0:26   ` [PATCH 0/3] extend get/setrlimit to support setting rlimits external to a process (v4) Neil Horman
2009-10-05  0:53     ` [PATCH 1/3] " Neil Horman
2009-10-08 21:32       ` Marcin Slusarz
2009-10-09  2:00         ` Neil Horman
2009-10-05  0:54     ` [PATCH 2/3] " Neil Horman
2009-10-05  1:57       ` Américo Wang
2009-10-05 12:32         ` Neil Horman
2009-10-05  0:54     ` [PATCH 3/3] " Neil Horman
2009-10-12 16:13   ` [PATCH 0/3] extend get/setrlimit to support setting rlimits external to a process (v5) Neil Horman
2009-10-12 16:20     ` [PATCH 1/3] " Neil Horman
2009-10-12 16:25     ` [PATCH 2/3] " Neil Horman
2009-10-12 16:27     ` [PATCH 3/3] " Neil Horman
2009-10-12 20:13     ` [PATCH 0/3] extend get/setrlimit to support setting rlimits external to a process (v6) Neil Horman
2009-10-12 20:20       ` [PATCH 1/3] " Neil Horman
2009-10-12 20:23       ` [PATCH 2/3] " Neil Horman
2009-10-12 20:25       ` [PATCH 3/3] " Neil Horman
2009-10-20  0:52       ` [PATCH 0/3] extend get/setrlimit to support setting rlimits external to a process (v7) Neil Horman
2009-10-20  0:53         ` [PATCH 1/3] " Neil Horman
2009-10-20  0:54         ` [PATCH 2/3] " Neil Horman
2009-11-02 15:10           ` Ingo Molnar
2009-11-02 17:40             ` Neil Horman
2009-10-20  0:55         ` [PATCH 3/3] " Neil Horman
2009-10-28 14:44         ` [PATCH 0/3] " Neil Horman
2009-10-30 18:24           ` Neil Horman
2009-11-02 15:25         ` Ingo Molnar [this message]
2009-11-02 17:54           ` Neil Horman
2009-11-02 18:51             ` Ingo Molnar
2009-11-03  0:23               ` Neil Horman
2009-11-04 11:26                 ` Ingo Molnar
2009-11-05 20:48                   ` Neil Horman
2009-11-06  9:26                     ` Ingo Molnar
2009-11-06 10:00                       ` Jiri Slaby
2009-11-08 10:36                         ` Ingo Molnar
2009-11-09  0:10                           ` Neil Horman
2009-11-09  8:32                             ` Jiri Slaby
2009-11-09 13:34                               ` Neil Horman
2009-11-09  8:54                       ` Jiri Slaby
2009-11-09  9:01                         ` Ingo Molnar
2009-11-09  9:22                           ` Jiri Slaby
2009-11-09  9:26                             ` Ingo Molnar
2009-11-09 13:35                               ` Neil Horman
2009-11-09 15:56                           ` Jiri Slaby
2009-11-09 16:40                             ` Oleg Nesterov
2009-11-09 17:15                               ` Jiri Slaby
2009-11-09 17:26                                 ` Linus Torvalds
2009-11-09 17:36                                 ` Oleg Nesterov
2009-11-18 14:51                                   ` Jiri Slaby
2009-11-18 14:51                                     ` [PATCH 01/16] core: posix-cpu-timers, cleanup rlimits usage Jiri Slaby
2009-11-18 16:48                                       ` Peter Zijlstra
2009-11-18 14:51                                     ` [PATCH 02/16] core: do security check under task_lock Jiri Slaby
2009-11-18 21:47                                       ` James Morris
2009-11-18 14:51                                     ` [PATCH 03/16] IA64: use ACCESS_ONCE for rlimits Jiri Slaby
2009-11-18 14:51                                       ` Jiri Slaby
2009-11-18 18:56                                       ` Luck, Tony
2009-11-18 18:56                                         ` Luck, Tony
2009-11-18 19:48                                         ` Linus Torvalds
2009-11-18 19:48                                           ` Linus Torvalds
2009-11-19  2:28                                           ` Ingo Molnar
2009-11-19  2:28                                             ` Ingo Molnar
2009-11-18 14:51                                     ` [PATCH 04/16] PPC: " Jiri Slaby
2009-11-18 14:51                                       ` Jiri Slaby
2009-11-18 14:51                                     ` [PATCH 05/16] S390: " Jiri Slaby
2009-11-18 14:51                                     ` [PATCH 06/16] SPARC: " Jiri Slaby
2009-11-18 14:51                                       ` Jiri Slaby
2009-11-18 17:55                                       ` David Miller
2009-11-18 17:55                                         ` David Miller
2009-11-18 18:09                                         ` Linus Torvalds
2009-11-18 18:09                                           ` Linus Torvalds
2009-11-18 14:51                                     ` [PATCH 07/16] X86: " Jiri Slaby
2009-11-18 14:51                                     ` [PATCH 08/16] FS: " Jiri Slaby
2009-11-18 14:51                                     ` [PATCH 09/16] MM: " Jiri Slaby
2009-11-18 14:51                                       ` Jiri Slaby
2009-11-18 15:29                                       ` Linus Torvalds
2009-11-18 15:29                                         ` Linus Torvalds
2009-11-18 14:51                                     ` [PATCH 10/16] core: " Jiri Slaby
     [not found]                                     ` <4B040A03.2020508-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2009-11-18 14:51                                       ` [PATCH 11/16] misc: " Jiri Slaby
2009-11-18 14:51                                         ` Jiri Slaby
2009-11-18 14:51                                     ` [PATCH 12/16] core: rename setrlimit to do_setrlimit Jiri Slaby
2009-11-20  6:10                                       ` Américo Wang
2009-11-18 14:51                                     ` [PATCH 13/16] core: implement getprlimit and setprlimit syscalls Jiri Slaby
2009-11-20 13:14                                       ` Neil Horman
2009-11-18 14:52                                     ` [PATCH 14/16] unistd: add __NR_[get|set]prlimit syscall numbers Jiri Slaby
2009-11-18 14:52                                     ` [PATCH 15/16] COMPAT: add get/put_compat_rlimit Jiri Slaby
2009-12-30 23:55                                       ` Arnd Bergmann
2010-01-06  9:35                                         ` Jiri Slaby
2009-11-18 14:52                                     ` [PATCH 16/16] x86: add ia32 compat prlimit syscalls Jiri Slaby
2009-11-18 23:15                                     ` [PATCH 0/3] extend get/setrlimit to support setting rlimits external to a process (v7) Oleg Nesterov
2009-11-19 15:43                                       ` Jiri Slaby
2009-11-20  2:11                                         ` acct_file_reopen() && do_acct_process() (Was: [PATCH 0/3] extend get/setrlimit to support setting rlimits external to a process (v7)) Oleg Nesterov
2009-11-20 10:27                                           ` Jiri Slaby
2009-10-12 21:58     ` [PATCH 0/3] extend get/setrlimit to support setting rlimits external to a process (v5) Andrew Morton
2009-10-13  0:06       ` Neil Horman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20091102152520.GG23776@elte.hu \
    --to=mingo@elte.hu \
    --cc=akpm@linux-foundation.org \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=marcin.slusarz@gmail.com \
    --cc=mingo@redhat.com \
    --cc=nhorman@tuxdriver.com \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.