linux-kernel - Re: [PATCH 3/4] coredump: ELF-FDPIC: enable to omit anonymous shared memory

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]

Date:	Wed, 21 Feb 2007 19:00:44 +0900
From:	"Kawai, Hidehiro" <hidehiro.kawai.ez@...achi.com>
To:	David Howells <dhowells@...hat.com>
Cc:	Andrew Morton <akpm@...l.org>,
	kernel list <linux-kernel@...r.kernel.org>,
	Pavel Machek <pavel@....cz>, Robin Holt <holt@....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@...hat" <haoki@...hat.com>
Subject: Re: [PATCH 3/4] coredump: ELF-FDPIC: enable to omit anonymous shared
    memory

Hi,

Thank you for your reply.

David Howells wrote:

>>I think that locking makes codes complex and generates overhead.
>>So I wouldn't like to use lock as far as possible.  I think passing
>>the flag as an extra argument is the simplest implementation to
>>avoid the core file corruption.
> 
> Actually, I don't think the locking is that hard or that complex.
> 
> 	int do_coredump(long signr, int exit_code, struct pt_regs * regs)
> 	{
> 		<setup vars>
> 
> 		down_read(&coredump_settings_sem);
> 
> 		...
> 
> 	fail:
> 		up_read(&coredump_settings_sem);
> 		return retval;
> 	}
> 
> And:
> 
> 	static ssize_t proc_coredump_omit_anon_shared_write(struct file *file,
> 						    const char __user *buf,
> 						    size_t count,
> 						    loff_t *ppos)
> 	{
> 		<setup vars>
> 
> 		down_write(&coredump_settings_sem);
> 
> 		...
> 
> 	out_no_task:
> 		up_write(&coredump_settings_sem);
> 		return ret;
> 	}

Is coredump_setting_sem a global semaphore?  If so, it prevents concurrent
core dumping.  Additionally, while some process is dumped, writing to
coredump_omit_anon_shared of unrelated process will be blocked.

So we should use per process or per mm locking.  But where should we
place the variable for locking?  Because we don't want to increase the
struct size, we might want to add another bit field in mm_struct:

	struct mm_struct {
		...
 		unsigned char dumpable:2;
		unsigned char coredump_in_progress:1;      /* this */
		unsigned char coredump_omit_anon_shared:1;
		...

And we use it to determine whether core dumping is in progress or not:

 	int do_coredump(long signr, int exit_code, struct pt_regs * regs)
 	{
 		<setup vars>
 
 		spin_lock(&dump_bits_lock);
		current->mm->coredump_in_progress = 1;
		spin_unlock(&dump_bits_lock);
		...

Additionally:

 	static ssize_t proc_coredump_omit_anon_shared_write(struct file *file,
 						    const char __user *buf,
 						    size_t count,
 						    loff_t *ppos)
 	{
 		<setup vars>

		ret = -EBUSY; 
		spin_lock(&dump_bits_lock);
		if (mm->coredump_in_progress) {
			spin_unlock(&dump_bits_lock);
			goto out;
		}
		mm->coredump_omit_anon_shared = (val != 0);
		spin_unlock(&dump_bits_lock);
		...

Do you think which is better this method or passing argument method
used by my patchset?
Or, are there even better way else?

-- 
Hidehiro Kawai
Hitachi, Ltd., Systems Development Laboratory

-
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/