linux-kernel - Re: [PATCH v9] livepatch: Clear relocation targets on a module removal

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20230124223241.ywbhxdrj26qfgtd6@treble>
Date:   Tue, 24 Jan 2023 14:32:41 -0800
From:   Josh Poimboeuf <jpoimboe@...nel.org>
To:     Joe Lawrence <joe.lawrence@...hat.com>
Cc:     linux-kernel@...r.kernel.org, linux-modules@...r.kernel.org,
        live-patching@...r.kernel.org, x86@...nel.org, jikos@...nel.org,
        pmladek@...e.com, Miroslav Benes <mbenes@...e.cz>,
        Josh Poimboeuf <jpoimboe@...hat.com>,
        Song Liu <song@...nel.org>
Subject: Re: [PATCH v9] livepatch: Clear relocation targets on a module
 removal

On Tue, Jan 24, 2023 at 03:08:27PM -0500, Joe Lawrence wrote:
> > +	/*
> > +	 * For a livepatch relocation, the restore r2 instruction might have
> > +	 * been previously written if the relocation references a symbol in a
> > +	 * module which was unloaded and is now being reloaded.  In that case,
> > +	 * skip the warning and instruction write.
> > +	 */
> > +	if (klp_sym && insn_val == PPC_INST_LD_TOC)
> > +		return 0;
> 
> Hi Josh,
> 
> Nit: shouldn't this return 1?
> 
> And if you're willing to entertain a small refactor, wouldn't
> restore_r2() be clearer if it returned -ESOMETHING on error?
> 
> Maybe converting to a boolean could work, but then I'd suggest a name
> that clearly implies success/fail given true/false return.  Maybe
> replace_nop_with_ld_toc() or replace_nop_to_restore_r2() ... still
> -ESOMETHING is more intuitive to me as there are cases like this where
> the function safely returns w/o replacing anything.

Indeed, and I actually already discovered that and made such changes,
just need to get around to posting the patches.

-- 
Josh