From owner-freebsd-bugs@FreeBSD.ORG Thu Jan 12 16:48:50 2012 Return-Path: Delivered-To: freebsd-bugs@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id E0325106564A; Thu, 12 Jan 2012 16:48:50 +0000 (UTC) (envelope-from matthewstory@gmail.com) Received: from mail-vw0-f54.google.com (mail-vw0-f54.google.com [209.85.212.54]) by mx1.freebsd.org (Postfix) with ESMTP id 77B4B8FC0C; Thu, 12 Jan 2012 16:48:50 +0000 (UTC) Received: by vbbfp1 with SMTP id fp1so832980vbb.13 for ; Thu, 12 Jan 2012 08:48:49 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; bh=V+98T526bC5uxjAsfRIv1vbK8Vwj+zhvoqz2ttazMFw=; b=Q5ytnaMfu5BNyobILoXEDk7L0GQGD9IbmCWuTcaHWJRlIxoFiLvVus9nEnZrKS9C3x 1MJJWybHCI3PXMufBo3arxrxoMhJ75d6T5RGeFFd4xKJiOgwObwz3XCUCsNc/UYr4otA BKmbJKHpDxN6KMomqbqo1vLOj+psM81MTDUgw= MIME-Version: 1.0 Received: by 10.52.67.38 with SMTP id k6mr2054881vdt.87.1326386928817; Thu, 12 Jan 2012 08:48:48 -0800 (PST) Received: by 10.52.159.69 with HTTP; Thu, 12 Jan 2012 08:48:48 -0800 (PST) In-Reply-To: <201201120815.q0C8FONo062154@red.freebsd.org> References: <201201120815.q0C8FONo062154@red.freebsd.org> Date: Thu, 12 Jan 2012 11:48:48 -0500 Message-ID: From: Matthew Story To: Dirk-Willem van Gulik Content-Type: text/plain; charset=ISO-8859-1 X-Content-Filtered-By: Mailman/MimeDel 2.1.5 Cc: freebsd-bugs@freebsd.org, freebsd-gnats-submit@freebsd.org Subject: Re: conf/164048: /etc/rc.d/hostid is not symlink aware X-BeenThere: freebsd-bugs@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Bug reports List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 12 Jan 2012 16:48:51 -0000 On Thu, Jan 12, 2012 at 3:15 AM, Dirk-Willem van Gulik wrote: [...snip] > # If ${hostid_file} already exists, we take UUID from there. > - if [ -r ${hostid_file} ]; then > + # If ${hostid_file} already exists, we take UUID from there. We use > + # a -f rather than a -r check as the histid_file may in fact be > + # a symbolic link. > per the test man-page, `-r' tests for readability, regardless of type, and `-f' tests for the existence of a regular file. `-r' does include an implicit test for existence, so `-r' will in fact work for symlinks, and fail reliably if the symlink source_file does not exist (relevant bits from the test man-page at the bottom of this message): $ # setup target/src dirs for demonstration of test $ mkdir target src $ # setup source_files for sym-linking $ jot - 1 10 | xargs -I {} touch src/{} $ # symlink in files $ find "$PWD/src" -type f -depth 1 -maxdepth 1 -print0 | xargs -0 -n1 sh -c 'ln -s "$*" target/' worker $ # make target read-only, note that mode 0400 on target will result in -r to fail $ chmod 500 target $ # demonstrate that -r works with symlinks $ jot - 1 10 | while read trg; do [ -r "target/$trg" ] && echo "I can read target/$trg"; done I can read target/1 I can read target/2 I can read target/3 I can read target/4 I can read target/5 I can read target/6 I can read target/7 I can read target/8 I can read target/9 I can read target/10 $ # demonstrate that -f also works with symlinks $ jot - 1 10 | while read trg; do [ -f "target/$trg" ] && echo "target/$trg is a regular file"; done target/1 is a regular file target/2 is a regular file target/3 is a regular file target/4 is a regular file target/5 is a regular file target/6 is a regular file target/7 is a regular file target/8 is a regular file target/9 is a regular file target/10 is a regular file > + # > + if [ -f ${hostid_file} ]; then > hostid_set `cat ${hostid_file}` > with this patch, if ${hostid_file} exists, and is non-readable, cat ${hostid_file} will fail, and yield no $1 to hostid_set (effectively identical to a hostid_file that is empty). this is not the desired behavior: $ # using our above setup, make target/1 unreadable $ chmod 000 target/1 $ # demonstrate failure of the new test with an unreadable, but existing file $ [ -f target/1 ] && cat target/1 cat: target/1: Permission denied > else > # No hostid file, generate UUID. > - hostid_generate > + hostid_reset > This line is actually why you are seeing a hostid_file on restart. The hostid_file does not exist on your system, and per the comment, and implementation, if a hostid_file does not exist, one is generated and set via sysctl (via the hostid_set function). Your suggested change changes the functionality of this program to both generate a hostid, and then store it in a hostid file. This seems to me to be a change in functionality, and not a bug. > [...snip] > There is a small race condition in this file (unless rc.d is doing some locking on hostid_file in the caller) if [ -r ${hostid_file} ]; then hostid_set `cat ${hostid_file}` else ... Insofar as it's possible (however unlikely) that the file mode (or file mode of the parents) could change between the test and the read. This could probably be resolved using lockf, but it's definitely not a big deal. ---------------snippits from man 1 test----------------- -r file True if file exists and is readable. [...snip] -f file True if file exists and is a regular file. -- regards, matt