From owner-freebsd-current@FreeBSD.ORG Wed Nov 26 12:44:41 2008 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id A09F71065677 for ; Wed, 26 Nov 2008 12:44:41 +0000 (UTC) (envelope-from yanefbsd@gmail.com) Received: from wa-out-1112.google.com (wa-out-1112.google.com [209.85.146.177]) by mx1.freebsd.org (Postfix) with ESMTP id 659DD8FC0C for ; Wed, 26 Nov 2008 12:44:41 +0000 (UTC) (envelope-from yanefbsd@gmail.com) Received: by wa-out-1112.google.com with SMTP id m34so221569wag.27 for ; Wed, 26 Nov 2008 04:44:41 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:received:received:cc:message-id:from:to :in-reply-to:content-type:content-transfer-encoding:mime-version :subject:date:references:x-mailer; bh=85okFQbgbbeBq87sX5R/q73KCADpRHtcNwMWFQQr/Sk=; b=E3pRVirw8I3rBO4e4CAN0JiQ5JRHUqpPF0h4nxLlVZqIZvR71uAqWxB1AV3gNzNf7O q+dW+1qFkbRjig5ZUVWPojJI5l8kbG0s0D2IlG9gTsMCoWatwBuPo85NNBTNEYfIFhdl 8uxEIR5DTRXN9l86WfcdW4DCvcff3nL7vtPRg= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=cc:message-id:from:to:in-reply-to:content-type :content-transfer-encoding:mime-version:subject:date:references :x-mailer; b=ftxsGH5ITD8Z30vODsTHef+LmDiKeSY734YfxDDG5LfwxfabboDzWztkNMiTDK1LOA iGhy8txNSDXWKSrtu2xx+0Fi0TIBA4JfZxlNXQ/G7mqN9HZ8wEEx0ciedOAYeG5cx2/8 bhlqK0QVRh4Bd0BzOyd9K1Id0XChO7+Ts9iYI= Received: by 10.115.94.1 with SMTP id w1mr3334092wal.30.1227703481019; Wed, 26 Nov 2008 04:44:41 -0800 (PST) Received: from ?192.168.10.3? (adsl-99-139-48-85.dsl.pltn13.sbcglobal.net [99.139.48.85]) by mx.google.com with ESMTPS id y11sm155490pod.19.2008.11.26.04.44.34 (version=TLSv1/SSLv3 cipher=RC4-MD5); Wed, 26 Nov 2008 04:44:40 -0800 (PST) Message-Id: From: Garrett Cooper To: insomniac In-Reply-To: <20081126132025.07e968b7@beastie> Content-Type: text/plain; charset=US-ASCII; format=flowed; delsp=yes Content-Transfer-Encoding: 7bit Mime-Version: 1.0 (Apple Message framework v929.2) Date: Wed, 26 Nov 2008 04:47:55 -0800 References: <20081126032214.03d8517a@slackware.it> <492CC391.2070207@delphij.net> <20081126132025.07e968b7@beastie> X-Mailer: Apple Mail (2.929.2) Cc: freebsd-current@freebsd.org Subject: Re: Patch for bin/54446 X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 26 Nov 2008 12:44:41 -0000 On Nov 26, 2008, at 4:20 AM, insomniac wrote: > On Tue, 25 Nov 2008 19:33:37 -0800 > Xin LI wrote: > >> I have made a small change: use malloc() here and use strlcpy(). >> Other parts looks just fine. >> >> (BTW I think we need to cc portmgr@ for approval) > > I merged your fix with Garrett Cooper's one (I forgot to free() > before > a return) and also mailed portmgr@ for approval. The new patch is > located at the same link: > > http://insomniac.slackware.it/plist.c.diff > > Moreover, I will fix style(9) in a second step. > > Best regards, > -- > Andrea Barberio Xin Li brought up a really good point (that I remembered when I first looked at the code, but forgot to mention later): calloc is nothing more than a malloc + bzero on many OSes (FreeBSD has moved away from this methodology), so using calloc sparingly is for the best. It really doesn't buy you much here anyhow since you're overwriting everything with a copied string anyhow... As for the potential security issue I mentioned earlier, it could be definitely be done if someone has the knowledge and was running with similar privs or had write access to the parent / child(ren) symlink. One of the ways of solving this issue may be to use flock(2) *shrugs*, as it would block other callers from modifying the file or the symlink (our concern would be symlinks I would think). This kind of race condition situation is part of the driving force between using fstat vs lstat vs stat. HTH, -Garrett