From owner-freebsd-current@FreeBSD.ORG Tue Nov 20 07:25:04 2012 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id B9427E07 for ; Tue, 20 Nov 2012 07:25:04 +0000 (UTC) (envelope-from baptiste.daroussin@gmail.com) Received: from mail-we0-f182.google.com (mail-we0-f182.google.com [74.125.82.182]) by mx1.freebsd.org (Postfix) with ESMTP id 3D0578FC0C for ; Tue, 20 Nov 2012 07:25:03 +0000 (UTC) Received: by mail-we0-f182.google.com with SMTP id u54so1385912wey.13 for ; Mon, 19 Nov 2012 23:25:02 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=sender:date:from:to:subject:message-id:references:mime-version :content-type:content-disposition:in-reply-to:user-agent; bh=rJGHirFRtkuuM0T6Bpz3rxFjavD8DVRxDjEd415ouwY=; b=owQKDfWKPyp7AHg28IjJZfbyxlY0xg2ASyYTb9TnstSXLXVb5hrcXrn+kIAT+Tbh3P CuKOsGIcaKd+jr4huMd1bidGLPZNhOoZtcIC3BkB/zGB2s3CYNBs5av3xbI0SXs0Uhil 9pUANhIAP9fcMbb3IymKJ9k385orfmd0KsQ5xwXOOyM0+h0fZwnALKVYC9wkDcraOYhX pEE14mvW1czE4RVl1VrvrXidpW31zBy0yoIuBBu4fz/FOKt/0ZrJIfXl2xD+FsH2EhsF PGvRkxsHqV+iCWtkoz4H6IQOt0d4tkobUQ7RNObdHf/E9ExZV7NJGDIAQdRaQAVs+LaS Ua4w== Received: by 10.180.109.132 with SMTP id hs4mr3654448wib.1.1353396302138; Mon, 19 Nov 2012 23:25:02 -0800 (PST) Received: from ithaqua.etoilebsd.net (ithaqua.etoilebsd.net. [37.59.37.188]) by mx.google.com with ESMTPS id s12sm16549833wik.11.2012.11.19.23.25.01 (version=TLSv1/SSLv3 cipher=OTHER); Mon, 19 Nov 2012 23:25:01 -0800 (PST) Sender: Baptiste Daroussin Date: Tue, 20 Nov 2012 08:24:59 +0100 From: Baptiste Daroussin To: Mateusz Guzik , Ryan Stone , FreeBSD Current Subject: Re: pw keeps setting /etc/group to 0600 Message-ID: <20121120072459.GH71195@ithaqua.etoilebsd.net> References: <20121119222843.GB22292@dft-labs.eu> <20121119223745.GG71195@ithaqua.etoilebsd.net> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="poJSiGMzRSvrLGLs" Content-Disposition: inline In-Reply-To: <20121119223745.GG71195@ithaqua.etoilebsd.net> User-Agent: Mutt/1.5.21 (2010-09-15) X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.14 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: Tue, 20 Nov 2012 07:25:04 -0000 --poJSiGMzRSvrLGLs Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Nov 19, 2012 at 11:37:45PM +0100, Baptiste Daroussin wrote: > On Mon, Nov 19, 2012 at 11:28:43PM +0100, Mateusz Guzik wrote: > > On Sat, Nov 17, 2012 at 11:20:21AM -0500, Ryan Stone wrote: > > > My original complaint that /etc/group gets permissions of 0600 is a r= esult > > > of a bug in libutil, which bapt@ ported pw to use in r242349. The new > > > group manipulation API using mktemp to create a temporary file, write= s the > > > new group database to the temp file and then renames the temp file to > > > /etc/group. The problem here is that mktemp creates a file with a mo= de of > > > 600, and libutil never chmods it. That should be pretty trivial to f= ix. > >=20 > > My additional 0,03$: > >=20 > > I took closer look to this and I think that problems are much broader > > than this. I don't know if similar problems were present before. > >=20 > > First, pw should not fail if other instance is running, it should wait > > instead (think of parallel batch scripts adding some users/groups). > >=20 > > Second, current code has a race: > > lockfd =3D open(group_file, O_RDONLY, 0); > > if (lockfd < 0 || fcntl(lockfd, F_SETFD, 1) =3D=3D -1) > > err(1, "%s", group_file); > > if (flock(lockfd, LOCK_EX|LOCK_NB) =3D=3D -1) { > > [..] > > gr_copy(pfd, tfd, gr, old_gr); /* copy from groupfile to tempfile */ > > [..] > > rename(tempfile,groupfile); > >=20 > > Now let's consider threads A and B: > >=20 > > A: open() > > A: lock(); > > A: gr_copy > > B: open() > >=20 > > Now B has file descriptor to /etc/group that is about to be removed. > >=20 > > A: rename() > > A: unlock() > > B: lock() > >=20 > > Now B has a lock on unlinked file. > >=20 > > B: gr_copy() > > B: rename() > >=20 > > ... and stores new content losing modifications done by A > >=20 > > Third, I don't like current api. > > gr_lock and gr_tmp have no arguments (that matter anyway) > > gr_copy operates on two descriptors given as arguments > > gr_mkdb takes nothing and is expected to do The Right Thing >=20 > gr_mkdb should chmod 0644 after renaming if rename worked. >=20 > I should work on this soon. >=20 > The API has been design to match the exact same api of pw_utils, I don't = like it > either but at least this is consistent. >=20 > regards, > Bapt Should be fixed now, regards, Bapt --poJSiGMzRSvrLGLs Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.19 (FreeBSD) iEYEARECAAYFAlCrMEsACgkQ8kTtMUmk6EwFPACePY6HimVwKaaDHqgRGavziX4V KbIAnjTHaU+XBpFLvC3yNk7/YexpRr1Z =vGGz -----END PGP SIGNATURE----- --poJSiGMzRSvrLGLs--