From patchwork Thu Aug 18 21:09:51 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Thiago Jung Bauermann X-Patchwork-Id: 9289319 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 4A778600CB for ; Fri, 19 Aug 2016 04:30:41 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 34F4829185 for ; Fri, 19 Aug 2016 04:30:41 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 263F9291A0; Fri, 19 Aug 2016 04:30:41 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=unavailable version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 7AFB829185 for ; Fri, 19 Aug 2016 04:30:40 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1750757AbcHSEak (ORCPT ); Fri, 19 Aug 2016 00:30:40 -0400 Received: from mx0a-001b2d01.pphosted.com ([148.163.156.1]:53956 "EHLO mx0a-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750721AbcHSEaj (ORCPT ); Fri, 19 Aug 2016 00:30:39 -0400 Received: from pps.filterd (m0098404.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.11/8.16.0.11) with SMTP id u7IL8q07088854 for ; Thu, 18 Aug 2016 17:10:03 -0400 Received: from e24smtp01.br.ibm.com (e24smtp01.br.ibm.com [32.104.18.85]) by mx0a-001b2d01.pphosted.com with ESMTP id 24vb9n71ma-1 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT) for ; Thu, 18 Aug 2016 17:10:03 -0400 Received: from localhost by e24smtp01.br.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Thu, 18 Aug 2016 18:10:00 -0300 Received: from d24dlp01.br.ibm.com (9.18.248.204) by e24smtp01.br.ibm.com (10.172.0.143) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Thu, 18 Aug 2016 18:09:59 -0300 X-IBM-Helo: d24dlp01.br.ibm.com X-IBM-MailFrom: bauerman@linux.vnet.ibm.com X-IBM-RcptTo: linux-kernel@vger.kernel.org; linux-security-module@vger.kernel.org Received: from d24relay02.br.ibm.com (d24relay02.br.ibm.com [9.13.184.26]) by d24dlp01.br.ibm.com (Postfix) with ESMTP id ED528352006E; Thu, 18 Aug 2016 17:09:37 -0400 (EDT) Received: from d24av02.br.ibm.com (d24av02.br.ibm.com [9.8.31.93]) by d24relay02.br.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id u7IL9xav26345908; Thu, 18 Aug 2016 18:09:59 -0300 Received: from d24av02.br.ibm.com (localhost [127.0.0.1]) by d24av02.br.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id u7IL9v01010141; Thu, 18 Aug 2016 18:09:58 -0300 Received: from hactar.localnet ([9.78.140.90]) by d24av02.br.ibm.com (8.14.4/8.14.4/NCO v10.0 AVin) with ESMTP id u7IL9ucf010101; Thu, 18 Aug 2016 18:09:57 -0300 From: Thiago Jung Bauermann To: Dave Young Cc: kexec@lists.infradead.org, Balbir Singh , "H. Peter Anvin" , linux-ima-devel@lists.sourceforge.net, Stewart Smith , Baoquan He , Michael Ellerman , x86@kernel.org, Ingo Molnar , Mimi Zohar , Vivek Goyal , Thomas Gleixner , Eric Richter , linux-kernel@vger.kernel.org, linux-security-module@vger.kernel.org, Eric Biederman , Andrew Morton , Samuel Mendoza-Jonas , linuxppc-dev@lists.ozlabs.org Subject: Re: [PATCH v2 3/6] kexec_file: Allow skipping checksum calculation for some segments. Date: Thu, 18 Aug 2016 18:09:51 -0300 User-Agent: KMail/4.14.3 (Linux/3.13.0-93-generic; KDE/4.14.13; x86_64; ; ) In-Reply-To: <20160818090330.GC845@dhcp-128-65.nay.redhat.com> References: <1471058305-30198-1-git-send-email-bauerman@linux.vnet.ibm.com> <1471058305-30198-4-git-send-email-bauerman@linux.vnet.ibm.com> <20160818090330.GC845@dhcp-128-65.nay.redhat.com> MIME-Version: 1.0 X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 16081821-1523-0000-0000-00000215F68C X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 16081821-1524-0000-0000-000027EFD1FA Message-Id: <105132489.pljZvDuEBr@hactar> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:, , definitions=2016-08-18_09:, , signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 suspectscore=0 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1604210000 definitions=main-1608180276 Sender: owner-linux-security-module@vger.kernel.org Precedence: bulk List-ID: X-Virus-Scanned: ClamAV using ClamSMTP Hello Dave, Thanks for your review! [ Trimming down Cc: list a little to try to clear the "too many recipients" mailing list restriction. ] Am Donnerstag, 18 August 2016, 17:03:30 schrieb Dave Young: > On 08/13/16 at 12:18am, Thiago Jung Bauermann wrote: > > Adds checksum argument to kexec_add_buffer specifying whether the given > > segment should be part of the checksum calculation. > > Since it is used with add buffer, could it be added to kbuf as a new > field? I was on the fence about adding it as a new argument to kexec_add_buffer or as a new field to struct kexec_buf. Both alternatives make sense to me. I implemented your suggestion in the patch below, what do you think? > Like kbuf.no_checksum, default value is 0 that means checksum is needed > if it is 1 then no need a checksum. It's an interesting idea and I implemented it that way, though in practice all current users of struct kexec_buf put it on the stack so the field needs to be initialized explicitly. diff --git a/arch/powerpc/kernel/kexec_elf_64.c b/arch/powerpc/kernel/kexec_elf_64.c index 22afc7b5ee73..d009f5363968 100644 --- a/arch/powerpc/kernel/kexec_elf_64.c +++ b/arch/powerpc/kernel/kexec_elf_64.c @@ -107,7 +107,7 @@ static int elf_exec_load(struct kimage *image, struct elfhdr *ehdr, int ret; size_t i; struct kexec_buf kbuf = { .image = image, .buf_max = ppc64_rma_size, - .top_down = false }; + .top_down = false, .skip_checksum = false }; /* Read in the PT_LOAD segments. */ for (i = 0; i < ehdr->e_phnum; i++) { @@ -162,7 +162,8 @@ void *elf64_load(struct kimage *image, char *kernel_buf, struct elf_info elf_info; struct fdt_reserve_entry *rsvmap; struct kexec_buf kbuf = { .image = image, .buf_min = 0, - .buf_max = ppc64_rma_size }; + .buf_max = ppc64_rma_size, + .skip_checksum = false }; ret = build_elf_exec_info(kernel_buf, kernel_len, &ehdr, &elf_info); if (ret) diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c index 38a1cdf6aa05..7b8f62c86651 100644 --- a/arch/x86/kernel/crash.c +++ b/arch/x86/kernel/crash.c @@ -617,7 +617,8 @@ int crash_load_segments(struct kimage *image) { int ret; struct kexec_buf kbuf = { .image = image, .buf_min = 0, - .buf_max = ULONG_MAX, .top_down = false }; + .buf_max = ULONG_MAX, .top_down = false, + .skip_checksum = false }; /* * Determine and load a segment for backup area. First 640K RAM diff --git a/arch/x86/kernel/kexec-bzimage64.c b/arch/x86/kernel/kexec-bzimage64.c index 4b3a75329fb6..449f433cd225 100644 --- a/arch/x86/kernel/kexec-bzimage64.c +++ b/arch/x86/kernel/kexec-bzimage64.c @@ -341,7 +341,7 @@ static void *bzImage64_load(struct kimage *image, char *kernel, unsigned int setup_hdr_offset = offsetof(struct boot_params, hdr); unsigned int efi_map_offset, efi_map_sz, efi_setup_data_offset; struct kexec_buf kbuf = { .image = image, .buf_max = ULONG_MAX, - .top_down = true }; + .top_down = true, .skip_checksum = false }; header = (struct setup_header *)(kernel + setup_hdr_offset); setup_sects = header->setup_sects; diff --git a/include/linux/kexec.h b/include/linux/kexec.h index 4559a1a01b0a..e5b3d99cbe50 100644 --- a/include/linux/kexec.h +++ b/include/linux/kexec.h @@ -100,6 +100,9 @@ struct kexec_segment { size_t bufsz; unsigned long mem; size_t memsz; + + /* Whether this segment is ignored in the checksum calculation. */ + bool skip_checksum; }; #ifdef CONFIG_COMPAT @@ -151,15 +154,16 @@ struct kexec_file_ops { /** * struct kexec_buf - parameters for finding a place for a buffer in memory - * @image: kexec image in which memory to search. - * @buffer: Contents which will be copied to the allocated memory. - * @bufsz: Size of @buffer. - * @mem: On return will have address of the buffer in memory. - * @memsz: Size for the buffer in memory. - * @buf_align: Minimum alignment needed. - * @buf_min: The buffer can't be placed below this address. - * @buf_max: The buffer can't be placed above this address. - * @top_down: Allocate from top of memory. + * @image: kexec image in which memory to search. + * @buffer: Contents which will be copied to the allocated memory. + * @bufsz: Size of @buffer. + * @mem: On return will have address of the buffer in memory. + * @memsz: Size for the buffer in memory. + * @buf_align: Minimum alignment needed. + * @buf_min: The buffer can't be placed below this address. + * @buf_max: The buffer can't be placed above this address. + * @top_down: Allocate from top of memory. + * @skip_checksum: Don't verify checksum for this buffer in purgatory. */ struct kexec_buf { struct kimage *image; @@ -171,6 +175,7 @@ struct kexec_buf { unsigned long buf_min; unsigned long buf_max; bool top_down; + bool skip_checksum; }; int __weak arch_kexec_walk_mem(struct kexec_buf *kbuf, diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c index c8418d62e2fc..f6e9958bf578 100644 --- a/kernel/kexec_file.c +++ b/kernel/kexec_file.c @@ -658,6 +658,7 @@ int kexec_add_buffer(struct kexec_buf *kbuf) ksegment->bufsz = kbuf->bufsz; ksegment->mem = kbuf->mem; ksegment->memsz = kbuf->memsz; + ksegment->skip_checksum = kbuf->skip_checksum; kbuf->image->nr_segments++; return 0; } @@ -672,7 +673,6 @@ static int kexec_calculate_store_digests(struct kimage *image) char *digest; void *zero_buf; struct kexec_sha_region *sha_regions; - struct purgatory_info *pi = &image->purgatory_info; zero_buf = __va(page_to_pfn(ZERO_PAGE(0)) << PAGE_SHIFT); zero_buf_sz = PAGE_SIZE; @@ -712,11 +712,7 @@ static int kexec_calculate_store_digests(struct kimage *image) struct kexec_segment *ksegment; ksegment = &image->segment[i]; - /* - * Skip purgatory as it will be modified once we put digest - * info in purgatory. - */ - if (ksegment->kbuf == pi->purgatory_buf) + if (ksegment->skip_checksum) continue; ret = crypto_shash_update(desc, ksegment->kbuf, @@ -788,7 +784,7 @@ static int __kexec_load_purgatory(struct kimage *image, unsigned long min, Elf_Shdr *sechdrs = NULL; struct kexec_buf kbuf = { .image = image, .bufsz = 0, .buf_align = 1, .buf_min = min, .buf_max = max, - .top_down = top_down }; + .top_down = top_down, .skip_checksum = true }; /* * sechdrs_c points to section headers in purgatory and are read @@ -893,7 +889,10 @@ static int __kexec_load_purgatory(struct kimage *image, unsigned long min, if (kbuf.buf_align < bss_align) kbuf.buf_align = bss_align; - /* Add buffer to segment list */ + /* + * Add buffer to segment list. Don't checksum the segment as + * it will be modified once we put digest info in purgatory. + */ ret = kexec_add_buffer(&kbuf); if (ret) goto out;