Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932242Ab0F2Rd4 (ORCPT ); Tue, 29 Jun 2010 13:33:56 -0400 Received: from e4.ny.us.ibm.com ([32.97.182.144]:39068 "EHLO e4.ny.us.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756680Ab0F2Rdz (ORCPT ); Tue, 29 Jun 2010 13:33:55 -0400 Date: Tue, 29 Jun 2010 10:33:49 -0700 From: "Paul E. McKenney" To: Oleg Nesterov Cc: Artem Bityutskiy , john stultz , Kees Cook , linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org, Alexander Viro , Andrew Morton , KOSAKI Motohiro , Neil Horman , Roland McGrath , Ingo Molnar , Peter Zijlstra , Thomas Gleixner Subject: Re: [PATCH] sanitize task->comm to avoid leaking escape codes Message-ID: <20100629173349.GH2765@linux.vnet.ibm.com> Reply-To: paulmck@linux.vnet.ibm.com References: <20100623181129.GM5876@outflux.net> <20100623194145.GA19628@redhat.com> <1277787505.3599.25.camel@localhost.localdomain> <20100629133131.GB5237@redhat.com> <20100629162334.GE2765@linux.vnet.ibm.com> <20100629171846.GA18440@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20100629171846.GA18440@redhat.com> User-Agent: Mutt/1.5.20 (2009-06-14) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2202 Lines: 66 On Tue, Jun 29, 2010 at 07:18:46PM +0200, Oleg Nesterov wrote: > On 06/29, Paul E. McKenney wrote: > > > > On Tue, Jun 29, 2010 at 03:31:31PM +0200, Oleg Nesterov wrote: > > > > > So, afaics, set_task_comm()->wmb() buys nothing and should be removed. > > > The last zero char in task_struct->comm[] is always here, at least this > > > guarantees that strcpy(char *dest, tsk->comm) is always safe. > > > > > > (I cc'ed the expert, Paul can correct me) > > > > First, all of the implementations that I can see do task_lock(tsk), which > > should prevent readers from seeing any changes. So I am guessing that > > you guys want to allow readers to get at ->comm without having to acquire > > this lock. > > Ah, sorry for confusion. > > No, we are not trying to invent the lockless get_task_comm(). I'd say > it is not needed, if we really care about the precise ->comm we can > take task->alloc_lock. > > The only problem is that I believe that set_task_comm() wrongly pretends > wmb() can help the lockless reader, it does: > > task_lock(tsk); > > /* > * Threads may access current->comm without holding > * the task lock, so write the string carefully. > * Readers without a lock may see incomplete new > * names but are safe from non-terminating string reads. > */ > memset(tsk->comm, 0, TASK_COMM_LEN); > wmb(); > strlcpy(tsk->comm, buf, sizeof(tsk->comm)); > task_unlock(tsk); > > but afaics this wmb() buys absolutely nothing if we race with the > reader doing, say, > > printk("my name is %s\n", current->comm); > > Afaics, this wmb() > > - can't prevent from printing the mixture of the old/new data > > - is not needed to make strcpy(somewhere, task->comm) safe, > the final char is always '0', we never change it. > > - adds the unnecessary confusion I agree -- I cannot see how this wmb() can help. > > [... snip a lot of good ideas ...] > > Thanks a lot, Paul ;) Glad you liked them. ;-) Thanx, Paul -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/