Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Fri, 29 May 2020 13:14:56 -0700
From:   Kees Cook <keescook@chromium.org>
To:     Sargun Dhillon <sargun@sargun.me>
Cc:     christian.brauner@ubuntu.com,
        containers@lists.linux-foundation.org, cyphar@cyphar.com,
        jannh@google.com, jeffv@google.com, linux-api@vger.kernel.org,
        linux-kernel@vger.kernel.org, palmer@google.com, rsesek@google.com,
        tycho@tycho.ws, Matt Denton <mpdenton@google.com>
Subject: Re: [PATCH v2 1/3] seccomp: Add find_notification helper
Message-ID: <202005291309.6183EC0604@keescook>
References: <20200528110858.3265-1-sargun@sargun.me>
 <20200528110858.3265-2-sargun@sargun.me>
 <202005282319.2BA6AD88@keescook>
 <20200529174037.GA11153@ircssh-2.c.rugged-nimbus-611.internal>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20200529174037.GA11153@ircssh-2.c.rugged-nimbus-611.internal>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Fri, May 29, 2020 at 05:40:38PM +0000, Sargun Dhillon wrote:
> > 
> > While the comment is good, let's actually enforce this with:
> > 
> > if (WARN_ON(!mutex_is_locked(&filter->notif_lock)))
> > 	return NULL;
> > 
> I don't see much use of lockdep in seccomp (well, any), but
> wouldn't a stronger statement be to use lockdep, and just have:
> 
> lockdep_assert_held(&filter->notify_lock);
> 
> As that checks that the lock is held by the current task.

/me slaps his forehead

Yes. I need more coffee or something. Yes, I meant
lockdep_assert_held(), and now I need to go fix my pstore series since I
confused myself into the wrong function and using it so many times in
pstore overwrote the correct function in my head. Thank you!

> Although, that does put this check behind lockdep, which means
> that running in "normal" circumstances is less safe (but faster?).

Now, that's fine. The check needs to be "am *I* holding this mutex?" and
I don't think anything except lockdep can do that.

-- 
Kees Cook