Date: Wed, 11 Nov 2015 10:15:56 -0600
From: Josh Poimboeuf <jpoimboe@redhat.com>
To: Minfei Huang <mnfhuang@gmail.com>
Cc: Zhou Chengming <zhouchengming1@huawei.com>, sjenning@redhat.com,
        jkosina@suse.cz, vojtech@suse.cz, live-patching@vger.kernel.org,
        linux-kernel@vger.kernel.org, guohanjun@huawei.com,
        huawei.libin@huawei.com, xiexiuqi@huawei.com, cbay@alwaysdata.com
Subject: Re: [PATCH v2] livepatch: x86: bugfix about kASLR
Message-ID: <20151111161556.GA5331@treble.redhat.com>
References: <1446791100-41585-1-git-send-email-zhouchengming1@huawei.com>
 <20151110140721.GC3163@treble.redhat.com>
 <20151111084648.GA26302@dhcp-129-10.nay.redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <20151111084648.GA26302@dhcp-129-10.nay.redhat.com>
User-Agent: Mutt/1.5.23.1-rc1 (2014-03-12)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2292
Lines: 57

On Wed, Nov 11, 2015 at 04:46:48PM +0800, Minfei Huang wrote:
> On 11/10/15 at 08:07am, Josh Poimboeuf wrote:
> > On Fri, Nov 06, 2015 at 02:25:00PM +0800, Zhou Chengming wrote:
> > > When enable KASLR, livepatch will adjust old_addr of changed
> > > function accordingly. So do the same thing for reloc.
> > > 
> > > +
> > > +#if defined(CONFIG_RANDOMIZE_BASE)
> > > +			/* If KASLR has been enabled, adjust old value accordingly */
> > > +			if (kaslr_enabled())
> > > +				reloc->val += kaslr_offset();
> > > +#endif
> > >  			ret = klp_verify_vmlinux_symbol(reloc->name,
> > >  							reloc->val);
> > >  			if (ret)
> > 
> > Zhou, thanks a lot for this fix.
> > 
> > Generally I think this patch is fine.  However, Chris J Arges is working
> > on another patch[*] which may get rid of reloc->val as an input and make
> > this patch obsolete.
> > 
> > So, assuming Chris's patch eventually gets accepted, I don't see a need
> > for this one unless anybody wants it as a bug fix for 4.4.
> > 
> > [*] https://lkml.kernel.org/r/1447085770-11729-1-git-send-email-chris.j.arges@canonical.com
> > 
> 
> Hi, Josh.
> 
> I think Jessica Yu is working on the relative patchset which will
> offload relocation logical to the module loader.

Yeah, Jessica's patch set does move the relocation logic to the module
loader, and it does result in a rewrite of this code.  However, it still
uses an address-based symbol addressing scheme, so it doesn't resolve
this particular issue.

Chris's patch changes the symbol addressing scheme from address-based to
sympos-based.  To be consistent, I think it should do that for both
function addresses and relocation symbol addresses.  Then that would fix
this issue and make Zhou's patch obsolete.

If Chris's smaller patch is merged before Jessica's bigger patch set,
Jessica's patches can be rebased on top of Chris's to keep the new
sympos-based addressing scheme.

Or vice versa: if Jessica's patches are merged first, then Chris's can
be rebased.  Either way, Chris's patches will obsolete this one.

-- 
Josh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/