From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.71) id 1aUHuj-0001kV-Uq for mharc-grub-devel@gnu.org; Fri, 12 Feb 2016 12:53:54 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51713) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aUHuf-0001iF-Hr for grub-devel@gnu.org; Fri, 12 Feb 2016 12:53:51 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aUHud-0002zh-Qr for grub-devel@gnu.org; Fri, 12 Feb 2016 12:53:49 -0500 Received: from mail-wm0-x22c.google.com ([2a00:1450:400c:c09::22c]:32923) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aUHud-0002yF-FW for grub-devel@gnu.org; Fri, 12 Feb 2016 12:53:47 -0500 Received: by mail-wm0-x22c.google.com with SMTP id g62so72449319wme.0 for ; Fri, 12 Feb 2016 09:53:45 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=subject:to:references:from:message-id:date:user-agent:mime-version :in-reply-to:content-type; bh=h0vVPKmbLTRAwF0AMFPb15YO6L2QC9YV2jpSPmiHPD8=; b=I117YmT0tfEZlXsVBr5qHmuX2ULzfrxNoAtm9Epeu3kQ6ThSehXKufPjen5KhNXWR/ /CGiLrhWjbxSy/PMvtzmkwgC1JDCWSg6NM+/vCavqlDOIzVmDkgmE9edg/6ITDILuOgk hG2/DPSKL564oZp6+JxACaykpZvgpy2NtXZ5ippBkoYGHtP5Ena4A0/d3MR9zzTAzV5o ZruSm2JkYdI25vCtxLm+Ae1UNVLZl1F5w3pYrYcTo9s7u9F5DpldRwH/BkBsNcbKJDRe yCd9ffKWgFg053yMKUni3CJI00pAP8zwt1A24zXKS6mGXBmL/Qa3ac5kgGIi3xaGjptD BxgA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:subject:to:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-type; bh=h0vVPKmbLTRAwF0AMFPb15YO6L2QC9YV2jpSPmiHPD8=; b=ApCZs+Lb4bJN0sddNKouWmcZ84ep++RtKIn0VReaYPhpFYY/6Z1Y3MXsjIjrnowV/8 u0W47XpL+StzmD0+rqZX2f75dTlsWwpj0l6SeVGPHR2yVJXA1nko4Wr6Yy0UMcc9K4Q4 ERPOrHcB7X4V0sGY7vOz83QgWUFlXlymM6YTb8190k9jY0XBsaRackzm6WIic4SgHds/ iAU7YvAfPwoPzISPCLITgv6QOesDR7IWC1fTP/qqZ8upGzXzeYeOyLVldw3fFz+ZTnbE USLtAPwIQcr23UrcqEa3XdMabNnMvOJe56o+rfXIUekus5WUP+PAWUWU4TBsMzbqlnir wjQA== X-Gm-Message-State: AG10YORsoUptM+/G16yhL0Su106M0yIqcrFkOc3BiZ5CSItRcuG9p3mQJisSkV0yhVpC1A== X-Received: by 10.194.216.99 with SMTP id op3mr3156725wjc.26.1455299624567; Fri, 12 Feb 2016 09:53:44 -0800 (PST) Received: from ?IPv6:2620:0:105f:fd00:a2a8:cdff:fe64:b3b5? ([2620:0:105f:fd00:a2a8:cdff:fe64:b3b5]) by smtp.gmail.com with ESMTPSA id et11sm13007262wjc.30.2016.02.12.09.53.43 for (version=TLSv1/SSLv3 cipher=OTHER); Fri, 12 Feb 2016 09:53:43 -0800 (PST) Subject: Re: [PATCH v3 1/3] Add helper functions to interact with android bootimg disks. To: grub-devel@gnu.org References: <1454964459-28213-1-git-send-email-shea@shealevy.com> <1454964459-28213-2-git-send-email-shea@shealevy.com> <56BE1978.1040306@gmail.com> From: =?UTF-8?Q?Vladimir_'=cf=86-coder/phcoder'_Serbinenko?= Message-ID: <56BE1C26.9050301@gmail.com> Date: Fri, 12 Feb 2016 18:53:42 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Icedove/38.5.0 MIME-Version: 1.0 In-Reply-To: <56BE1978.1040306@gmail.com> Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="vvRRCmTopeTER84WgnLtKKkkgbHo5KpnT" X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 2a00:1450:400c:c09::22c X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.14 Precedence: list Reply-To: The development of GNU GRUB List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 12 Feb 2016 17:53:51 -0000 This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --vvRRCmTopeTER84WgnLtKKkkgbHo5KpnT Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 12.02.2016 18:42, Vladimir '=CF=86-coder/phcoder' Serbinenko wrote: > On 08.02.2016 21:47, Shea Levy wrote: >> Currently, the kernel, command line, and ramdisk can be extracted. >> --- >> grub-core/Makefile.core.def | 1 + >> grub-core/loader/android_bootimg.c | 253 ++++++++++++++++++++++++++++= +++++++++ >> include/grub/android_bootimg.h | 12 ++ >> 3 files changed, 266 insertions(+) >> create mode 100644 grub-core/loader/android_bootimg.c >> create mode 100644 include/grub/android_bootimg.h >> >> diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def= >> index 0cc40bb..991adad 100644 >> --- a/grub-core/Makefile.core.def >> +++ b/grub-core/Makefile.core.def >> @@ -1669,6 +1669,7 @@ module =3D { >> arm64 =3D loader/arm64/linux.c; >> common =3D loader/linux.c; >> common =3D lib/cmdline.c; >> + common =3D loader/android_bootimg.c; >> enable =3D noemu; >> }; >> =20 >> diff --git a/grub-core/loader/android_bootimg.c b/grub-core/loader/and= roid_bootimg.c >> new file mode 100644 >> index 0000000..bbee915 >> --- /dev/null >> +++ b/grub-core/loader/android_bootimg.c >> @@ -0,0 +1,253 @@ >> +/* android_bootimg.c - Helpers for interacting with the android booti= mg format. */ >> +/* >> + * GRUB -- GRand Unified Bootloader >> + * Copyright (C) 2016 Free Software Foundation, Inc. >> + * >> + * This program is free software: you can redistribute it and/or mod= ify >> + * it under the terms of the GNU General Public License as published= by >> + * the Free Software Foundation, either version 3 of the License, or= >> + * (at your option) any later version. >> + * >> + * This program is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + * GNU General Public License for more details. >> + * >> + * You should have received a copy of the GNU General Public License= >> + * along with this program. If not, see . >> + */ >> + >> +#include >> +#include >> +#include >> + >> +/* from https://android.googlesource.com/platform/system/core/+/506d2= 33e7ac8ca4efa80768153d842c296477f99/mkbootimg/bootimg.h */ > Parts of the file cannot be effectively under different licenses. Pleas= e > put this into separate header file. >> +/* From here until the end of struct boot_img_hdr available under the= following terms: >> + * >> + * Copyright 2007, The Android Open Source Project >> + * >> + * Licensed under the Apache License, Version 2.0 (the "License"); >> + * you may not use this file except in compliance with the License. >> + * You may obtain a copy of the License at >> + * >> + * http://www.apache.org/licenses/LICENSE-2.0 >> + * >> + * Unless required by applicable law or agreed to in writing, softwar= e >> + * distributed under the License is distributed on an "AS IS" BASIS, >> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or im= plied. >> + * See the License for the specific language governing permissions an= d >> + * limitations under the License. >> + */ >> +#define BOOT_MAGIC "ANDROID!" >> +#define BOOT_MAGIC_SIZE 8 >> +#define BOOT_NAME_SIZE 16 >> +#define BOOT_EXTRA_ARGS_SIZE 1024 >> + >> +struct boot_img_hdr >> +{ >> + grub_uint8_t magic[BOOT_MAGIC_SIZE]; >> + >> + grub_uint32_t kernel_size; >> + grub_uint32_t kernel_addr; >> + >> + grub_uint32_t ramdisk_size; >> + grub_uint32_t ramdisk_addr; >> + >> + grub_uint32_t second_size; >> + grub_uint32_t second_addr; >> + >> + grub_uint32_t tags_addr; >> + grub_uint32_t page_size; >> + grub_uint32_t unused[2]; >> + >> + grub_uint8_t name[BOOT_NAME_SIZE]; >> + >> + grub_uint8_t cmdline[BOOT_ARGS_SIZE]; >> + >> + grub_uint32_t id[8]; >> + >> + grub_uint8_t extra_cmdline[BOOT_EXTRA_ARGS_SIZE]; >> +} GRUB_PACKED; >> + >> +static grub_err_t >> +read_hdr (grub_disk_t disk, struct boot_img_hdr *hd) >> +{ >> + if (grub_disk_read (disk, 0, 0, sizeof *hd, hd)) >> + goto fail; >> + >> + if (grub_memcmp (hd->magic, BOOT_MAGIC, BOOT_MAGIC_SIZE)) >> + goto fail; >> + >> + if (hd->unused[0] || hd->unused[1]) >> + goto fail; >> + >> + hd->kernel_size =3D grub_le_to_cpu32 (hd->kernel_size); >> + hd->kernel_addr =3D grub_le_to_cpu32 (hd->kernel_addr); >> + hd->ramdisk_size =3D grub_le_to_cpu32 (hd->ramdisk_size); >> + hd->ramdisk_addr =3D grub_le_to_cpu32 (hd->ramdisk_addr); >> + hd->second_size =3D grub_le_to_cpu32 (hd->second_size); >> + hd->second_addr =3D grub_le_to_cpu32 (hd->second_addr); >> + hd->tags_addr =3D grub_le_to_cpu32 (hd->tags_addr); >> + hd->page_size =3D grub_le_to_cpu32 (hd->page_size); >> + >> + grub_size_t i; >> + for (i =3D 0; i < sizeof hd->id / sizeof hd->id[0]; ++i) >> + { >> + hd->id[i] =3D grub_le_to_cpu32 (hd->id[i]); >> + } > Why do you need this byteswap? Why not byteswap when it's used? > Also in loaders you can avoid byteswap usually as you know that you loa= d > the file for the same arch as you currently run. Exceptions are arm-be,= > ppc-le and fat binary headers. >> + >> + return GRUB_ERR_NONE; >> + >> +fail: >> + return grub_error (GRUB_ERR_BAD_FS, N_("%s not an android bootimg")= , >> + disk->name); > BAD_FS is not correct anymore. BAD_KERNEL would be more correct. >> +} >> + >> +static grub_ssize_t >> +grub_android_bootimg_read (grub_file_t file, char *buf, grub_size_t l= en) >> +{ >> + grub_size_t len_left =3D file->size - file->offset; >> + len =3D len > len_left ? len_left : len; >> + grub_off_t *begin_offset =3D file->data; >> + grub_off_t actual_offset =3D *begin_offset + file->offset; >> + file->device->disk->read_hook =3D file->read_hook; >> + file->device->disk->read_hook_data =3D file->read_hook_data; >> + grub_errno =3D grub_disk_read (file->device->disk, 0, actual_offset= , len, buf); >> + file->device->disk->read_hook =3D 0; >> + >> + if (grub_errno) >> + return -1; >> + else >> + return len; >> +} >> + >> +static grub_err_t >> +grub_android_bootimg_close (grub_file_t file) >> +{ >> + grub_free (file->data); >> + return GRUB_ERR_NONE; >> +} >> + >> +static struct grub_fs grub_android_bootimg_fs =3D >> + { >> + .name =3D "android_bootimg", >> + .read =3D grub_android_bootimg_read, >> + .close =3D grub_android_bootimg_close >> + }; >> + > please use grub_file_offset_open rather than reimplementing it if you > need it at all. I've looked in linux.c and you'll need only to adjust > prot_file_size calculation and grub_file_seek (add a call and adjust on= e > call). You can easily extract grub_cmd_linux essence into load_linux > (grub_file_t file, grub_off_t off, grub_off_t size) >=20 To clarify: I'm not against using grub_file_offset_open if it makes code simpler. Just choose between those 2 alternatives whichever results in simpler code. >> +grub_err_t >> +grub_android_bootimg_load_kernel (const char *disk_path, grub_file_t = *file, >> + char *cmdline) >> +{ >> + grub_err_t ret =3D GRUB_ERR_NONE; >> + struct grub_file *f =3D 0; >> + grub_disk_t disk =3D grub_disk_open (disk_path); >> + if (!disk) >> + goto err; >> + >> + struct boot_img_hdr hd; >> + if (read_hdr (disk, &hd)) >> + goto err; >> + >> + f =3D grub_zalloc (sizeof *f); >> + if (!f) >> + goto err; >> + >> + f->fs =3D &grub_android_bootimg_fs; >> + f->device =3D grub_zalloc (sizeof *(f->device)); >> + if (!f->device) >> + goto err; >> + f->device->disk =3D disk; >> + >> + f->name =3D grub_malloc (sizeof "kernel"); >> + if (!f->name) >> + goto err; >> + grub_memcpy (f->name, "kernel", sizeof "kernel"); >> + grub_off_t *begin_offset =3D grub_malloc (sizeof *begin_offset); >> + if (!begin_offset) >> + goto err; >> + *begin_offset =3D hd.page_size; >> + f->data =3D begin_offset; >> + f->size =3D hd.kernel_size; >> + >> + *file =3D f; > All of this will go away, so I won't comment on it. >> +grub_err_t >> +grub_android_bootimg_load_initrd (const char *disk_path, grub_file_t = *file) >> +{ >=20 > Instead you can extract grub_cmd_initrd essence into sth like > void *prepare_initrd (grub_size_t size) > which returns a pointer to buffer where the initrd would do. >> diff --git a/include/grub/android_bootimg.h b/include/grub/android_boo= timg.h >> new file mode 100644 >> index 0000000..ff14df7 >> --- /dev/null >> +++ b/include/grub/android_bootimg.h >> @@ -0,0 +1,12 @@ >> +#include >> + >> +#define BOOT_ARGS_SIZE 512 >> + >> +/* Load a kernel from a bootimg. cmdline must be at least BOOT_ARGS_S= IZE */ >> +grub_err_t >> +grub_android_bootimg_load_kernel (const char *disk_path, grub_file_t = *file, >> + char *cmdline); >> + >> +/* Load an initrd from a bootimg. */ >> +grub_err_t >> +grub_android_bootimg_load_initrd (const char *disk_path, grub_file_t = *file); >> >=20 >=20 --vvRRCmTopeTER84WgnLtKKkkgbHo5KpnT Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iF4EAREKAAYFAla+HCcACgkQmBXlbbo5nOuLxAEAl6SIWj2Abfto9lSoXvldXsb8 zuFrw2UKpX4TP5CLwkABAIFGA0OA4PLevodhiIF9EEZQ2dJHJ6DKC6fCInaFwgHM =qZ2Q -----END PGP SIGNATURE----- --vvRRCmTopeTER84WgnLtKKkkgbHo5KpnT--