linux-kernel - Re: [PATCH] Allow threads to rename siblings via /proc/pid/tasks/tid/comm

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: <87aayy6j8b.fsf@basil.nowhere.org>
Date:	Sat, 07 Nov 2009 23:52:20 +0100
From:	Andi Kleen <andi@...stfloor.org>
To:	john stultz <johnstul@...ibm.com>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	Arjan van de Ven <arjan@...radead.org>,
	lkml <linux-kernel@...r.kernel.org>,
	Mike Fulton <fultonm@...ibm.com>,
	Sean Foley <Sean_Foley@...ibm.com>,
	Darren Hart <dvhltc@...ibm.com>,
	KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>
Subject: Re: [PATCH] Allow threads to rename siblings via /proc/pid/tasks/tid/comm

john stultz <johnstul@...ibm.com> writes:
> -	strlcpy(tsk->comm, buf, sizeof(tsk->comm));
> +
> +	/*
> +	 * Threads may access current->comm without holding
> +	 * the task lock, so write the string carefully
> +	 * to avoid non-terminating reads. Readers without a lock
> +	 * will get the oldname, the newname or an empty string.
> +	 */
> +	tsk->comm[0] = 0;
> +	wmb();
> +	strlcpy(tsk->comm+1, buf+1, sizeof(tsk->comm)-1);
> +	wmb();
> +	tsk->comm[0] = buf[0];

Is this really safe? 

reader                    writer


read comm[0]
                         set comm[0] to 0
                         overwrites comm[1]
read comm[1]
read comm[2]
                         writes comm[2] to 0 
read comm[3]

...
goes beyond the end
                  
Better way probably is to replace tsk->comm with a pointer
and exchange that using xchg. Drawback: 4-8 bytes more per task.

Or perhaps make comm one byte longer and make sure the last
byte is always 0, but the drawback is that a reader can
read random (but at least safe) junk then.

-Andi

-- 
ak@...ux.intel.com -- Speaking for myself only.
--
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/