linux-kernel - Re: [PATCH 1/4] coredump: add an interface to control the core dump routine

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20070302093419.GA2001@elf.ucw.cz>
Date:	Fri, 2 Mar 2007 10:34:19 +0100
From:	Pavel Machek <pavel@....cz>
To:	"Kawai, Hidehiro" <hidehiro.kawai.ez@...achi.com>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	kernel list <linux-kernel@...r.kernel.org>,
	Robin Holt <holt@....com>, David Howells <dhowells@...hat.com>,
	Alan Cox <alan@...rguk.ukuu.org.uk>,
	Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>,
	sugita <yumiko.sugita.yf@...achi.com>,
	Satoshi OSHIMA <soshima@...hat.com>,
	Hideo AOKI <haoki@...hat.com>
Subject: Re: [PATCH 1/4] coredump: add an interface to control the core dump routine

Hi!

> This patch adds an interface to set/reset a flag which determines
> anonymous shared memory segments should be dumped or not when a core
> file is generated.
> 
> /proc/<pid>/coredump_omit_anonymous_shared file is provided to access
> the flag. You can change the flag status for a particular process by
> writing to or reading from the file.

Yes. So, you used very different interface interface from ulimit,
which means locking is hard.

Plus, what you are doing can be done in userspace using google
coredumper.

> +	if (down_write_trylock(&coredump_settings_sem)) {
> +		set_coredump_omit_anon_shared(mm, (val != 0));
> +		up_write(&coredump_settings_sem);
> +	} else
> +		ret = -EBUSY;

Now this is an ugly interface. "If user tries to write to /proc file
while it is locked, return him spurious error.


> @@ -75,6 +77,8 @@ extern int suid_dumpable;
>  #define SUID_DUMP_USER		1	/* Dump as user of process */
>  #define SUID_DUMP_ROOT		2	/* Dump as root */
>  
> +extern struct rw_semaphore coredump_settings_sem;
> +
>  /* Stack area protections */
>  #define EXSTACK_DEFAULT   0	/* Whatever the arch defaults to */
>  #define EXSTACK_DISABLE_X 1	/* Disable executable stacks */

Yep, very nice, if you used interface suited for the task (ulimit),
you'd not have to invent new locking like this.

Anyway, I don't care, and you don't listen. You got the design wrong,
and your code can be done in userspace, anyway. This is "NAK" from me,
but unfortunately I'm not in power to decide this. So at least drop me
from Cc in future submissions.

								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
-
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/