[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20181210190333.GB28127@edgewater-inn.cambridge.arm.com>
Date: Mon, 10 Dec 2018 19:03:33 +0000
From: Will Deacon <will.deacon@....com>
To: Mark Rutland <mark.rutland@....com>
Cc: mingo@...nel.org, linux-kernel@...r.kernel.org,
akpm@...ux-foundation.org, anders.roxell@...aro.org,
boqun.feng@...il.com, naresh.kamboju@...aro.org,
peterz@...radead.org
Subject: Re: [PATCH 2/2] locking/atomics: Check atomic headers with sha1sum
On Mon, Dec 10, 2018 at 05:50:35PM +0000, Mark Rutland wrote:
> We currently check the atomic headers at build-time to ensure they
> haven't been modified directly, and these checks require regenerating
> the headers in full. As this takes a few seconds, even when
> parallelized, this is too slow to run for every kernel build.
>
> Instead, we can generate a hash of each header as we generate them,
> which we can cheaply check at build time (~0.16s for all headers).
>
> This patch does so, updating headers with their hashes using the new
> gen-atomics.sh script. As some users apparently build the kernel wihout
> coreutils, lacking sha1sum, the checks are skipped in this case.
> Presumably, most developers have a working coreutils installation.
>
> Signed-off-by: Mark Rutland <mark.rutland@....com>
> Cc: Andrew Morton <akpm@...ux-foundation.org>
> Cc: Boqun Feng <boqun.feng@...il.com>
> Cc: Ingo Molnar <mingo@...nel.org>
> Cc: Peter Zijlstra <peterz@...radead.org>
> Cc: Will Deacon <will.deacon@....com>
> ---
> include/asm-generic/atomic-instrumented.h | 1 +
> include/asm-generic/atomic-long.h | 1 +
> include/linux/atomic-fallback.h | 1 +
> scripts/atomic/check-atomics.sh | 26 ++++++++++++++++++++------
> scripts/atomic/gen-atomics.sh | 20 ++++++++++++++++++++
> 5 files changed, 43 insertions(+), 6 deletions(-)
> create mode 100755 scripts/atomic/gen-atomics.sh
>
> diff --git a/include/asm-generic/atomic-instrumented.h b/include/asm-generic/atomic-instrumented.h
> index b8f5b35216e1..e8730c6b9fe2 100644
> --- a/include/asm-generic/atomic-instrumented.h
> +++ b/include/asm-generic/atomic-instrumented.h
> @@ -1785,3 +1785,4 @@ atomic64_dec_if_positive(atomic64_t *v)
> })
>
> #endif /* _ASM_GENERIC_ATOMIC_INSTRUMENTED_H */
> +// b29b625d5de9280f680e42c7be859b55b15e5f6a
> diff --git a/include/asm-generic/atomic-long.h b/include/asm-generic/atomic-long.h
> index a833d385a70b..881c7e27af28 100644
> --- a/include/asm-generic/atomic-long.h
> +++ b/include/asm-generic/atomic-long.h
> @@ -1010,3 +1010,4 @@ atomic_long_dec_if_positive(atomic_long_t *v)
>
> #endif /* CONFIG_64BIT */
> #endif /* _ASM_GENERIC_ATOMIC_LONG_H */
> +// 77558968132ce4f911ad53f6f52ce423006f6268
> diff --git a/include/linux/atomic-fallback.h b/include/linux/atomic-fallback.h
> index 1c02c0112fbb..a7d240e465c0 100644
> --- a/include/linux/atomic-fallback.h
> +++ b/include/linux/atomic-fallback.h
> @@ -2292,3 +2292,4 @@ atomic64_dec_if_positive(atomic64_t *v)
> #define atomic64_cond_read_relaxed(v, c) smp_cond_load_relaxed(&(v)->counter, (c))
>
> #endif /* _LINUX_ATOMIC_FALLBACK_H */
> +// 25de4a2804d70f57e994fe3b419148658bb5378a
> diff --git a/scripts/atomic/check-atomics.sh b/scripts/atomic/check-atomics.sh
> index c30101cddf2d..cfa0c2f71c84 100755
> --- a/scripts/atomic/check-atomics.sh
> +++ b/scripts/atomic/check-atomics.sh
> @@ -7,13 +7,27 @@ ATOMICDIR=$(dirname $0)
> ATOMICTBL=${ATOMICDIR}/atomics.tbl
> LINUXDIR=${ATOMICDIR}/../..
>
> +echo '' | sha1sum - > /dev/null 2>&1
> +if [ $? -ne 0 ]; then
> + printf "sha1sum not available, skipping atomic header checks.\n"
> + exit 0
> +fi
> +
> cat <<EOF |
> -gen-atomic-instrumented.sh asm-generic/atomic-instrumented.h
> -gen-atomic-long.sh asm-generic/atomic-long.h
> -gen-atomic-fallback.sh linux/atomic-fallback.h
> +asm-generic/atomic-instrumented.h
> +asm-generic/atomic-long.h
> +linux/atomic-fallback.h
> EOF
> -while read script header; do
> - if ! (${ATOMICDIR}/${script} ${ATOMICTBL} | diff - ${LINUXDIR}/include/${header} > /dev/null); then
> - printf "warning: include/${header} is out-of-date.\n"
> +while read header; do
> + OLDSUM="$(tail -n 1 ${LINUXDIR}/include/${header})"
> + OLDSUM="${OLDSUM#// }"
> +
> + NEWSUM="$(head -n -1 ${LINUXDIR}/include/${header} | sha1sum)"
> + NEWSUM="${NEWSUM%% *}"
Here we're relying on head and tail being available, but it looks like
they're also part of coreutils, so that should be fine.
> +
> + if [ "${OLDSUM}" != "${NEWSUM}" ]; then
> + printf "warning: generated include/${header} has been modified.\n"
> fi
> done
> +
> +exit 0
> diff --git a/scripts/atomic/gen-atomics.sh b/scripts/atomic/gen-atomics.sh
> new file mode 100755
> index 000000000000..27400b0cd732
> --- /dev/null
> +++ b/scripts/atomic/gen-atomics.sh
> @@ -0,0 +1,20 @@
> +#!/bin/sh
> +# SPDX-License-Identifier: GPL-2.0
> +#
> +# Generate atomic headers
> +
> +ATOMICDIR=$(dirname $0)
> +ATOMICTBL=${ATOMICDIR}/atomics.tbl
> +LINUXDIR=${ATOMICDIR}/../..
> +
> +cat <<EOF |
> +gen-atomic-instrumented.sh asm-generic/atomic-instrumented.h
> +gen-atomic-long.sh asm-generic/atomic-long.h
> +gen-atomic-fallback.sh linux/atomic-fallback.h
> +EOF
> +while read script header; do
> + ${ATOMICDIR}/${script} ${ATOMICTBL} > ${LINUXDIR}/include/${header}
> + HASH="$(sha1sum ${LINUXDIR}/include/${header})"
> + HASH="${HASH%% *}"
> + printf "// %s\n" "${HASH}" >> ${LINUXDIR}/include/${header}
> +done
Thanks, this looks sensible to me:
Acked-by: Will Deacon <will.deacon@....com>
Will
Powered by blists - more mailing lists