NetBSD Problem Report #56112

From www@netbsd.org  Fri Apr 16 19:28:25 2021
Return-Path: <www@netbsd.org>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits))
	(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id 4F7AA1A921F
	for <gnats-bugs@gnats.NetBSD.org>; Fri, 16 Apr 2021 19:28:25 +0000 (UTC)
Message-Id: <20210416192824.54D301A923C@mollari.NetBSD.org>
Date: Fri, 16 Apr 2021 19:28:24 +0000 (UTC)
From: parrottjustin16@gmail.com
Reply-To: parrottjustin16@gmail.com
To: gnats-bugs@NetBSD.org
Subject: races in usr.bin/shlock
X-Send-Pr-Version: www-1.0

>Number:         56112
>Category:       bin
>Synopsis:       races in usr.bin/shlock
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    bin-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Fri Apr 16 19:30:00 +0000 2021
>Closed-Date:    
>Last-Modified:  Sat Aug 17 14:28:06 +0000 2024
>Originator:     Justin Parrott
>Release:        9.1 and current
>Organization:
independent
>Environment:
localhost$ uname -a
NetBSD localhost 9.1 NetBSD 9.1 (GENERIC) #0: Sun Oct 18 19:24:30 UTC 2020  mkrepro@mkrepro.NetBSD.org:/usr/src/sys/arch/amd64/compile/GENERIC amd64

>Description:
There's a race in which shlock will notify the caller that it has the lock, but in fact another process has it.
>How-To-Repeat:
I basically have 50 processes dealing with the same couple of locks

Anyway, here's an example:
        { while true; do echo; done } |
        xargs -L 1 -P 50 sh -c '
                until shlock -f /tmp/lock -p $$; do done
                l=$(cat /tmp/lock | sed "s/^[ ]*//")
                if [ "$$" != "$l" ]
                then
                        echo pid\($$\) does not match lock\($l\)
                else
                        #echo the lock is ours\!
                fi
                #sleep 3
        '


>Fix:
So inn-2.6.4/backends/shlock.c isn't susceptible to the race as far as I can see (it passes the above example anyway).

Or use flock(1)

>Release-Note:

>Audit-Trail:
From: "Christos Zoulas" <christos@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/56112 CVS commit: src/usr.bin/shlock
Date: Fri, 16 Apr 2021 18:41:12 -0400

 Module Name:	src
 Committed By:	christos
 Date:		Fri Apr 16 22:41:12 UTC 2021

 Modified Files:
 	src/usr.bin/shlock: shlock.c

 Log Message:
 PR/56112: Justin Parrott: Don't unlink a lock file without locking it, because
 you can have races when multiple processes try to unlink it. Check the link
 count to see if we have won the race.

 While here:
 - use snprintf
 - use warn
 - use size_t/ssize_t
 - use loops instead of goto
 - KNF


 To generate a diff of this commit:
 cvs rdiff -u -r1.13 -r1.14 src/usr.bin/shlock/shlock.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

From: Erik Fair <fair@netbsd.org>
To: gnats-bugs@netbsd.org
Cc: gnats-admin@netbsd.org,
 netbsd-bugs@netbsd.org,
 parrottjustin16@gmail.com
Subject: Re: PR/56112 CVS commit: src/usr.bin/shlock
Date: Sat, 17 Apr 2021 16:54:57 -0700

 Wow,

 Congratulations - first new bug in decades.

 I didn=E2=80=99t use flock(2) in shlock(1) because in 1985 when shlock =
 was written, flock(2) was a BSD-ism, and I wanted (needed) the tool to =
 be as portable as possible between the varying =E2=80=A6 flavors of Unix =
 at the time, e.g. System III, System V and their unfortunate descendants =
 - hence =E2=80=9Cdot locking=E2=80=9D as had been done with Unix email =
 mailboxes since Version 6 mail(1).

 	Erik Fair=

State-Changed-From-To: open->feedback
State-Changed-By: maya@NetBSD.org
State-Changed-When: Mon, 19 Apr 2021 07:15:40 +0000
State-Changed-Why:
Does the fix work?


From: Justin Parrott <parrottjustin16@gmail.com>
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: bin/56112 (races in usr.bin/shlock)
Date: Wed, 21 Apr 2021 12:32:29 -0400

 --0000000000008a902b05c07e1c67
 Content-Type: text/plain; charset="UTF-8"

 It appears to be fine here

 On Mon, Apr 19, 2021, 3:15 AM <maya@netbsd.org> wrote:

 > Synopsis: races in usr.bin/shlock
 >
 > State-Changed-From-To: open->feedback
 > State-Changed-By: maya@NetBSD.org
 > State-Changed-When: Mon, 19 Apr 2021 07:15:40 +0000
 > State-Changed-Why:
 > Does the fix work?
 >
 >
 >
 >

 --0000000000008a902b05c07e1c67
 Content-Type: text/html; charset="UTF-8"
 Content-Transfer-Encoding: quoted-printable

 <div dir=3D"auto">It appears to be fine here</div><br><div class=3D"gmail_q=
 uote"><div dir=3D"ltr" class=3D"gmail_attr">On Mon, Apr 19, 2021, 3:15 AM  =
 &lt;<a href=3D"mailto:maya@netbsd.org">maya@netbsd.org</a>&gt; wrote:<br></=
 div><blockquote class=3D"gmail_quote" style=3D"margin:0 0 0 .8ex;border-lef=
 t:1px #ccc solid;padding-left:1ex">Synopsis: races in usr.bin/shlock<br>
 <br>
 State-Changed-From-To: open-&gt;feedback<br>
 State-Changed-By: maya@NetBSD.org<br>
 State-Changed-When: Mon, 19 Apr 2021 07:15:40 +0000<br>
 State-Changed-Why:<br>
 Does the fix work?<br>
 <br>
 <br>
 <br>
 </blockquote></div>

 --0000000000008a902b05c07e1c67--

State-Changed-From-To: feedback->needs-pullups
State-Changed-By: dholland@NetBSD.org
State-Changed-When: Sat, 01 May 2021 20:38:47 +0000
State-Changed-Why:
should get into -9, unfortunately christos mixed the fix with a large cleanup


State-Changed-From-To: needs-pullups->open
State-Changed-By: riastradh@NetBSD.org
State-Changed-When: Sat, 17 Aug 2024 14:28:06 +0000
State-Changed-Why:
This has been in needs-pullups since 2021, but I'm reluctant to pull it
up without any automatic tests that I can find.  Can I interest someone
in writing some automatic tests for shlock(1)?


>Unformatted:

Home	PR Database Search