linux-kernel - Re: livepatch/x86: apply alternatives and paravirt patches after relocations

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] [day] [month] [year] [list]

Date:   Fri, 19 Aug 2016 10:32:18 +0200
From:   Petr Mladek <pmladek@...e.com>
To:     Jessica Yu <jeyu@...hat.com>
Cc:     Josh Poimboeuf <jpoimboe@...hat.com>,
        Miroslav Benes <mbenes@...e.cz>,
        Jiri Kosina <jikos@...nel.org>,
        Chris J Arges <chris.j.arges@...onical.com>,
        Eugene Shatokhin <eugene.shatokhin@...alab.ru>,
        live-patching@...r.kernel.org, x86@...nel.org,
        linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: livepatch/x86: apply alternatives and paravirt patches after
 relocations

On Thu 2016-08-18 14:03:13, Jessica Yu wrote:
> +++ Petr Mladek [18/08/16 11:51 +0200]:
> >On Wed 2016-08-17 20:58:29, Jessica Yu wrote:
> >>Implement arch_klp_init_object_loaded() for x86, which applies
> >>alternatives/paravirt patches. This fixes the order in which relocations
> >>and alternatives/paravirt patches are applied.
> >>
> >>--- /dev/null
> >>+++ b/arch/x86/kernel/livepatch.c
> >>+	for (s = info->sechdrs; s < info->sechdrs + info->hdr.e_shnum; s++) {
> >>+		/* Apply per-object .klp.arch sections */
> >>+		cnt = sscanf(info->secstrings + s->sh_name,
> >>+			     ".klp.arch.%55[^.].%127s",
> >>+			     sec_objname, secname);
> >>+		if (cnt != 2)
> >>+			continue;
> >>+		if (strcmp(sec_objname, objname))
> >>+			continue;
> >>+		if (!strcmp(".altinstructions", secname))
> >
> >The previous version of the patch compared against "altinstructions"
> >(without the dot). I admit that I haven't tested it but the dot
> >looks suspicious here.
> 
> Good eye, I should have explained why the dot is needed in the strcmp..
> So, the new documentation states that any arch-specific sections to
> be applied by livepatch are to be prefixed with the string
> ".klp.arch.$objname.", note the required dot at the end of this prefix.
> 
> So for example, if we have a .parainstructions section with a patch
> for the kvm module, the prefixed section name would look like:
> 
>   .klp.arch.kvm..parainstructions
>   ^   prefix   ^^ original name ^
> 
> That extra dot looks weird, but it is needed when we have section names
> like "__ftr_fixup" on powerpc. Without the extra dot at the end of
> ".klp.arch.$objname." We'd get names like ".klp.arch.$objname__ftr_fixup",
> and we wouldn't be able to tell where the objname ends and where the
> section name begins. But with ".klp.arch.$objname.__ftr_fixup", we
> have a hard delimeter and know that after the dot after $objname comes
> the original section name.

That is a bit unfortunate but it makes perfect sense.
Thanks a lot for explanation.

Best Regards,
Petr