From owner-freebsd-hackers@FreeBSD.ORG Thu Mar 19 10:10:30 2015 Return-Path: Delivered-To: freebsd-hackers@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 849DEEE2 for ; Thu, 19 Mar 2015 10:10:30 +0000 (UTC) Received: from kib.kiev.ua (kib.kiev.ua [IPv6:2001:470:d5e7:1::1]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 0EEA885C for ; Thu, 19 Mar 2015 10:10:29 +0000 (UTC) Received: from tom.home (kostik@localhost [127.0.0.1]) by kib.kiev.ua (8.14.9/8.14.9) with ESMTP id t2JAAKFh062898 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Thu, 19 Mar 2015 12:10:20 +0200 (EET) (envelope-from kostikbel@gmail.com) DKIM-Filter: OpenDKIM Filter v2.9.2 kib.kiev.ua t2JAAKFh062898 Received: (from kostik@localhost) by tom.home (8.14.9/8.14.9/Submit) id t2JAAJKK062891; Thu, 19 Mar 2015 12:10:19 +0200 (EET) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: tom.home: kostik set sender to kostikbel@gmail.com using -f Date: Thu, 19 Mar 2015 12:10:19 +0200 From: Konstantin Belousov To: Tiwei Bie Subject: Re: [PATCH] Finish the task 'Fix corefilename race' Message-ID: <20150319101019.GZ2379@kib.kiev.ua> References: <1426749223-18118-1-git-send-email-btw@mail.ustc.edu.cn> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1426749223-18118-1-git-send-email-btw@mail.ustc.edu.cn> User-Agent: Mutt/1.5.23 (2014-03-12) X-Spam-Status: No, score=-2.0 required=5.0 tests=ALL_TRUSTED,BAYES_00, DKIM_ADSP_CUSTOM_MED,FREEMAIL_FROM,NML_ADSP_CUSTOM_MED autolearn=no autolearn_force=no version=3.4.0 X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on tom.home Cc: freebsd-hackers@freebsd.org, mjguzik@gmail.com X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 19 Mar 2015 10:10:30 -0000 On Thu, Mar 19, 2015 at 03:13:43PM +0800, Tiwei Bie wrote: > Hi, Mateusz! > > I have finished the task: Fix corefilename race [1]. > > Following is my patch: > > --- > sys/kern/kern_sig.c | 22 ++++++++++++++++++++-- > 1 file changed, 20 insertions(+), 2 deletions(-) > > diff --git a/sys/kern/kern_sig.c b/sys/kern/kern_sig.c > index 58d9707..a1421cb 100644 > --- a/sys/kern/kern_sig.c > +++ b/sys/kern/kern_sig.c > @@ -3090,8 +3090,24 @@ static int compress_user_cores = 0; > #endif > > static char corefilename[MAXPATHLEN] = {"%N.core"}; > -SYSCTL_STRING(_kern, OID_AUTO, corefile, CTLFLAG_RWTUN, corefilename, > - sizeof(corefilename), "Process corefile name format string"); > + > +static struct sx corefilename_lock; > +SX_SYSINIT(corefilename_init, &corefilename_lock, "corefilename lock"); > + > +static int > +sysctl_kern_corefile(SYSCTL_HANDLER_ARGS) > +{ > + int error; > + > + sx_xlock(&corefilename_lock); > + error = sysctl_handle_string(oidp, corefilename, MAXPATHLEN, req); > + sx_xunlock(&corefilename_lock); > + > + return (error); > +} > +SYSCTL_PROC(_kern, OID_AUTO, corefile, CTLTYPE_STRING | CTLFLAG_RWTUN | > + CTLFLAG_MPSAFE, 0, 0, sysctl_kern_corefile, "A", > + "Process corefile name format string"); > > /* > * corefile_open(comm, uid, pid, td, compress, vpp, namep) > @@ -3120,6 +3136,7 @@ corefile_open(const char *comm, uid_t uid, pid_t pid, struct thread *td, > name = malloc(MAXPATHLEN, M_TEMP, M_WAITOK | M_ZERO); > indexpos = -1; > (void)sbuf_new(&sb, name, MAXPATHLEN, SBUF_FIXEDLEN); > + sx_slock(&corefilename_lock); > for (i = 0; format[i] != '\0'; i++) { > switch (format[i]) { > case '%': /* Format character */ > @@ -3162,6 +3179,7 @@ corefile_open(const char *comm, uid_t uid, pid_t pid, struct thread *td, > break; > } > } > + sx_sunlock(&corefilename_lock); > free(hostname, M_TEMP); > if (compress) > sbuf_printf(&sb, GZ_SUFFIX); So the race is between somebody setting the core path string and another process coredumping, am I right ? If you, could you try to reuse some existing lock for the task ? It is a waste to have sx dedicated to the task, which is probably never used by 99% of the machines in the world.