linux-kernel - Re: [PATCH v2 59/62] livepatch/klp-build: Introduce klp-build script for generating livepatch modules

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <305c6d06-ae85-4595-ba05-9aa7b93739bd@redhat.com>
Date: Wed, 11 Jun 2025 17:44:23 -0400
From: Joe Lawrence <joe.lawrence@...hat.com>
To: Josh Poimboeuf <jpoimboe@...nel.org>, x86@...nel.org
Cc: linux-kernel@...r.kernel.org, Petr Mladek <pmladek@...e.com>,
 Miroslav Benes <mbenes@...e.cz>, live-patching@...r.kernel.org,
 Song Liu <song@...nel.org>, laokz <laokz@...mail.com>,
 Jiri Kosina <jikos@...nel.org>, Marcos Paulo de Souza <mpdesouza@...e.com>,
 Weinan Liu <wnliu@...gle.com>, Fazla Mehrab <a.mehrab@...edance.com>,
 Chen Zhongjin <chenzhongjin@...wei.com>, Puranjay Mohan <puranjay@...nel.org>
Subject: Re: [PATCH v2 59/62] livepatch/klp-build: Introduce klp-build script
 for generating livepatch modules

On 5/9/25 4:17 PM, Josh Poimboeuf wrote:
> +revert_patches() {
> +	local extra_args=("$@")
> +	local patches=("${APPLIED_PATCHES[@]}")
> +
> +	for (( i=${#patches[@]}-1 ; i>=0 ; i-- )) ; do
> +		revert_patch "${patches[$i]}" "${extra_args[@]}"
> +	done
> +
> +	APPLIED_PATCHES=()
> +
> +	# Make sure git actually sees the patches have been reverted.
> +	[[ -d "$SRC/.git" ]] && (cd "$SRC" && git update-index -q --refresh)
> +}

< warning: testing entropy field fully engaged :D >

Minor usability nit: I had run `klp-build --help` while inside a VM that
didn't seem to have r/w access to .git/.  Since the cleanup code is
called unconditionally, it gave me a strange error when running this
`git update-index` when I never supplied any patches to operate on. I
just wanted to see the usage.

Could this git update-index be made conditional?

  if (( ${#APPLIED_PATCHES[@]} > 0 )); then
      ([[ -d "$SRC/.git" ]] && cd "$SRC" && git update-index -q --refresh)
      APPLIED_PATCHES=()
  fi

Another way to find yourself in this function is to move .git/ out of
the way.  In that case, since it's the last line of revert_patches(), I
think the failure of [[ -d "$SRC/.git" ]] causes the script to
immediately exit:

  - for foo.patch, at the validate_patches() -> revert_patches() call
  - for --help, at the cleanup() -> revert_patches() call

So if you don't like the conditional change above, should
revert_patches() end with `true` to eat the [[ -d "$SRC/.git" ]] status?
 Or does that interfere with other calls to that function throughout the
script?

FWIW, with either adjustment, the script seems happy to operate on a
plain ol' kernel source tree without git.

-- 
Joe