From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755891AbdKCKWU (ORCPT ); Fri, 3 Nov 2017 06:22:20 -0400 Received: from mail.kernel.org ([198.145.29.99]:35118 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750826AbdKCKWS (ORCPT ); Fri, 3 Nov 2017 06:22:18 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 2FB35214F0 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=jeyu@kernel.org Date: Fri, 3 Nov 2017 11:22:13 +0100 From: Jessica Yu To: Josh Poimboeuf Cc: linux-kernel@vger.kernel.org, live-patching@vger.kernel.org Subject: Re: x86/module: Detect corrupt relocations against nonzero data Message-ID: <20171103102212.64yxc7fikhyj76o6@redbean> References: <3999d59e87ad73d4030e98bd90a6ffc00bc054d2.1509659611.git.jpoimboe@redhat.com> <20171103021942.nfxsdp43cziaoiow@treble> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii; format=flowed Content-Disposition: inline In-Reply-To: <20171103021942.nfxsdp43cziaoiow@treble> X-OS: Linux redbean 4.14.0-rc7-next-20171102+ x86_64 User-Agent: NeoMutt/20171027 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org +++ Josh Poimboeuf [02/11/17 21:19 -0500]: >On Thu, Nov 02, 2017 at 04:57:11PM -0500, Josh Poimboeuf wrote: >> There have been some cases where external tooling (e.g., kpatch-build) >> creates a corrupt relocation which targets the wrong address. This is a >> silent failure which can corrupt memory in unexpected places. >> >> On x86, the bytes of data being overwritten by relocations are always >> initialized to zero beforehand. Use that knowledge to add sanity checks >> to detect such cases before they corrupt memory. >> >> Signed-off-by: Josh Poimboeuf >> --- >> arch/x86/kernel/module.c | 13 +++++++++++++ >> 1 file changed, 13 insertions(+) >> >> diff --git a/arch/x86/kernel/module.c b/arch/x86/kernel/module.c >> index 62e7d70aadd5..a69b12617820 100644 >> --- a/arch/x86/kernel/module.c >> +++ b/arch/x86/kernel/module.c >> @@ -172,19 +172,27 @@ int apply_relocate_add(Elf64_Shdr *sechdrs, >> case R_X86_64_NONE: >> break; >> case R_X86_64_64: >> + if (*(u64 *)loc != 0) >> + goto nonzero; >> *(u64 *)loc = val; >> break; >> case R_X86_64_32: >> + if (*(u32 *)loc != 0) >> + goto nonzero; >> *(u32 *)loc = val; >> if (val != *(u32 *)loc) >> goto overflow; >> break; >> case R_X86_64_32S: >> + if (*(s32 *)loc != 0) >> + goto nonzero; >> *(s32 *)loc = val; >> if ((s64)val != *(s32 *)loc) >> goto overflow; >> break; >> case R_X86_64_PC32: >> + if (*(u64 *)loc != 0) >> + goto nonzero; >> val -= (u64)loc; >> *(u32 *)loc = val; > >NACK - this last bit is obviously a bug, not sure how it passed my >testing without module load failures... Thanks for the patch Josh, btw - could you also CC the x86 folks when you send out v2? Thanks! Jessica