linux-kernel - Re: [PATCH 0/3] extend get/setrlimit to support setting rlimits external to a process (v7)

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20091102152520.GG23776@elte.hu>
Date:	Mon, 2 Nov 2009 16:25:20 +0100
From:	Ingo Molnar <mingo@...e.hu>
To:	Neil Horman <nhorman@...driver.com>
Cc:	linux-kernel@...r.kernel.org, akpm@...ux-foundation.org,
	marcin.slusarz@...il.com, tglx@...utronix.de, mingo@...hat.com,
	hpa@...or.com, Linus Torvalds <torvalds@...ux-foundation.org>
Subject: Re: [PATCH 0/3] extend get/setrlimit to support setting rlimits
	external to a process (v7)

* Neil Horman <nhorman@...driver.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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/