Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
Date:   Fri, 25 Mar 2022 15:36:24 -0700
From:   Kees Cook <keescook@chromium.org>
To:     Shuah Khan <skhan@linuxfoundation.org>
Cc:     shuah@kernel.org, Andy Lutomirski <luto@amacapital.net>,
        Will Drewry <wad@chromium.org>,
        linux-kselftest@vger.kernel.org,
        Willem de Bruijn <willemb@google.com>,
        Jakub Kicinski <kuba@kernel.org>, linux-kernel@vger.kernel.org,
        linux-hardening@vger.kernel.org
Subject: Re: [PATCH 1/2] selftests/harness: Run TEARDOWN for ASSERT failures
Message-ID: <202203251535.4B900BC0@keescook>
References: <20220324231907.1363887-1-keescook@chromium.org>
 <20220324231907.1363887-2-keescook@chromium.org>
 <f705c381-cbe6-0862-e10f-44f2afdef24d@linuxfoundation.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <f705c381-cbe6-0862-e10f-44f2afdef24d@linuxfoundation.org>
Precedence: bulk

On Fri, Mar 25, 2022 at 01:37:20PM -0600, Shuah Khan wrote:
> On 3/24/22 5:19 PM, Kees Cook wrote:
> > The kselftest test harness has traditionally not run the registered
> > TEARDOWN handler when a test encountered an ASSERT. This creates
> > unexpected situations and tests need to be very careful about using
> > ASSERT, which seems a needless hurdle for test writers.
> > 
> > Because of the harness's design for optional failure handlers, the
> > original implementation of ASSERT used an abort() to immediately
> > stop execution, but that meant the context for running teardown was
> > lost. Instead, use setjmp/longjmp so that teardown can be done.
> > 
> 
> Thanks for the patch. The change look good to me.
> 
> > Failed SETUP routines continue to not be followed by TEARDOWN, though.
> 
> Does this mean failed setup() routines have to handle TEARDOWN? What
> are guidelines to follow for setup() failures?
> 
> Can you add a bit more detail on what you meant by " Failed SETUP
> routines continue to not be followed by TEARDOWN, though".

Sure! It means that any failures in a SETUP need to be cleaned up by the
SETUP, as TEARDOWN won't be run. (As in, this is unchanged from how
things behaved prior to this patch.)

> 
> With that:
> 
> Reviewed-by: Shuah Khan <skhan@linuxfoundation.org>

Thanks!

-- 
Kees Cook