From owner-svn-src-head@freebsd.org Wed Jan 31 15:20:52 2018 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 0F63AED0E17; Wed, 31 Jan 2018 15:20:52 +0000 (UTC) (envelope-from royger@gmail.com) Received: from mail-wm0-x22e.google.com (mail-wm0-x22e.google.com [IPv6:2a00:1450:400c:c09::22e]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 7F255684AE; Wed, 31 Jan 2018 15:20:51 +0000 (UTC) (envelope-from royger@gmail.com) Received: by mail-wm0-x22e.google.com with SMTP id g1so8882994wmg.2; Wed, 31 Jan 2018 07:20:51 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=sender:date:from:to:cc:subject:message-id:mime-version :content-disposition:user-agent; bh=RhXOZTbqLssNXnmFO5Y9UcKPdSTji5OBiwtafs/DOTI=; b=fJRPdUFtq2D2ldDB3xoXgbHYVWeB7ftdFuV3GDT9peXZpR/TwkXpMTomSVVTPz9gD6 iZ7oGS7wUbbgX4hQx1E2JTfh37RSFhpkm1+rD8jZ++lT0pplfYwJMLwsoHmX0TWqSuZc Khvmainx9+rQeNiHLP5invM8hdAcK684z9uQyMUyAOancr86S6qjwAsQhma6RTXIxDwZ H+mRnpv5znwdCaZ1RMTvNB0IIy5Y7NqRVWe5iwiKU33+pJ2NTFlYMQex8Grrqa9x3/TZ 8kS6ItLee0894HRMQvH9vnIHAUE6nkG01FPdhgplKwXLG0A/BxqR4Opy81Z4ozMvhDE9 yKCA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:sender:date:from:to:cc:subject:message-id :mime-version:content-disposition:user-agent; bh=RhXOZTbqLssNXnmFO5Y9UcKPdSTji5OBiwtafs/DOTI=; b=lYEcsf/8hRBJdkVLXbO3Q2CdwjE/p6g07huGFZMYR95+QSkajufqkW2Dc124+Idv1m u2//P4bQgMRMRwbO1uqtHH5HzH0WI2Aoj3VLZ6dwJeCILvO/1sAICiykpTHHIZcsXOj1 yzuI/anJReKt2tHji/k1bAWBPz+FzGYEkybOChtWqULHDzBPxK4CaMr0UYp+dP+r2Hxf UkR9MTrF4D9CIvvxmgxLjmhlbmGZYpSn6KfaBoBJxhXkeiKSJOCj9TZ/YrTL4aX6l7Ax 9t0KTO7Cct8q92vpgt9pKCAsts6EiIqyOvmIGR5gTkIEQzH4RZnO9Yys06pHVWskcyxq LrCQ== X-Gm-Message-State: AKwxytcVkn5dtGFs1A6LSSy7aiaxlw/Sx5Ulbia9CyFsxR1Ow14lF13u ygp1xUZxMki5gYhMMerDAWpJXA== X-Google-Smtp-Source: AH8x226bTzr9oFPmF6tADbJhwzThxvdYP+5u1Yo/sIBlI5DNCANLCpWX63vZx8UTbL/ZIRE6Y/hTVw== X-Received: by 10.80.154.6 with SMTP id o6mr13247327edb.146.1517412050136; Wed, 31 Jan 2018 07:20:50 -0800 (PST) Received: from localhost (default-46-102-197-194.interdsl.co.uk. [46.102.197.194]) by smtp.gmail.com with ESMTPSA id b12sm9241358eda.81.2018.01.31.07.20.49 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 31 Jan 2018 07:20:49 -0800 (PST) Sender: =?UTF-8?Q?Roger_Pau_Monn=C3=A9?= Date: Wed, 31 Jan 2018 15:20:46 +0000 From: Roger Pau =?iso-8859-1?Q?Monn=E9?= To: Wojciech Macek Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r328536 - in head/stand: common powerpc/kboot Message-ID: <20180131151823.fwigjbd5uubhshpj@MacBook-Pro-de-Roger.local> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: NeoMutt/20171208 X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.25 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 31 Jan 2018 15:20:52 -0000 On Mon, Jan 29, 2018 at 09:24:28AM +0000, Wojciech Macek wrote: > Modified: head/stand/common/load_elf.c > ============================================================================== > --- head/stand/common/load_elf.c Mon Jan 29 09:21:08 2018 (r328535) > +++ head/stand/common/load_elf.c Mon Jan 29 09:24:28 2018 (r328536) > @@ -29,6 +29,7 @@ > __FBSDID("$FreeBSD$"); > > #include > +#include > #include > #include > #include > @@ -118,15 +119,72 @@ __elfN(load_elf_header)(char *filename, elf_file_t ef) > err = EFTYPE; > goto error; > } > + > if (ehdr->e_ident[EI_CLASS] != ELF_TARG_CLASS || /* Layout ? */ > ehdr->e_ident[EI_DATA] != ELF_TARG_DATA || So here you force EI_DATA == ELF_TARG_DATA in order to continue... > - ehdr->e_ident[EI_VERSION] != EV_CURRENT || /* Version ? */ > - ehdr->e_version != EV_CURRENT || > - ehdr->e_machine != ELF_TARG_MACH) { /* Machine ? */ > + ehdr->e_ident[EI_VERSION] != EV_CURRENT) /* Version ? */ { > err = EFTYPE; > goto error; > } > > + /* > + * Fixup ELF endianness. > + * > + * The Xhdr structure was loaded using block read call to > + * optimize file accesses. It might happen, that the endianness > + * of the system memory is different that endianness of > + * the ELF header. > + * Swap fields here to guarantee that Xhdr always contain > + * valid data regardless of architecture. > + */ > + if (ehdr->e_ident[EI_DATA] == ELFDATA2MSB) { > + ehdr->e_type = be16toh(ehdr->e_type); ... yet here you check for EI_DATA == ELFDATA2MSB which AFAICT it's not possible given the check above, so the following if branch is dead code. > + ehdr->e_machine = be16toh(ehdr->e_machine); > + ehdr->e_version = be32toh(ehdr->e_version); > + if (ehdr->e_ident[EI_CLASS] == ELFCLASS64) { > + ehdr->e_entry = be64toh(ehdr->e_entry); > + ehdr->e_phoff = be64toh(ehdr->e_phoff); > + ehdr->e_shoff = be64toh(ehdr->e_shoff); > + } else { > + ehdr->e_entry = be32toh(ehdr->e_entry); > + ehdr->e_phoff = be32toh(ehdr->e_phoff); > + ehdr->e_shoff = be32toh(ehdr->e_shoff); > + } > + ehdr->e_flags = be32toh(ehdr->e_flags); > + ehdr->e_ehsize = be16toh(ehdr->e_ehsize); > + ehdr->e_phentsize = be16toh(ehdr->e_phentsize); > + ehdr->e_phnum = be16toh(ehdr->e_phnum); > + ehdr->e_shentsize = be16toh(ehdr->e_shentsize); > + ehdr->e_shnum = be16toh(ehdr->e_shnum); > + ehdr->e_shstrndx = be16toh(ehdr->e_shstrndx); > + > + } else { > + ehdr->e_type = le16toh(ehdr->e_type); > + ehdr->e_machine = le16toh(ehdr->e_machine); > + ehdr->e_version = le32toh(ehdr->e_version); > + if (ehdr->e_ident[EI_CLASS] == ELFCLASS64) { > + ehdr->e_entry = le64toh(ehdr->e_entry); > + ehdr->e_phoff = le64toh(ehdr->e_phoff); > + ehdr->e_shoff = le64toh(ehdr->e_shoff); > + } else { > + ehdr->e_entry = le32toh(ehdr->e_entry); > + ehdr->e_phoff = le32toh(ehdr->e_phoff); > + ehdr->e_shoff = le32toh(ehdr->e_shoff); > + } > + ehdr->e_flags = le32toh(ehdr->e_flags); > + ehdr->e_ehsize = le16toh(ehdr->e_ehsize); > + ehdr->e_phentsize = le16toh(ehdr->e_phentsize); > + ehdr->e_phnum = le16toh(ehdr->e_phnum); > + ehdr->e_shentsize = le16toh(ehdr->e_shentsize); > + ehdr->e_shnum = le16toh(ehdr->e_shnum); > + ehdr->e_shstrndx = le16toh(ehdr->e_shstrndx); > + } I think this chunk (and all the similar ones below) should be put on a macro in order to avoid such big chunks of code repetition. It's also fairly easy to forget to change one of the branches in the future. Roger.