Re: [PATCH v3 1/4] arm64: Add and export some accessor functions for xen boot

2015-10-30 Thread Fu Wei 
Hi Vladimir,

Good idea! I can see your patch for it in master branch.

Great thanks for your help, I will try this ASAP :-)


On 29 October 2015 at 20:03, Vladimir 'φ-coder/phcoder' Serbinenko
 wrote:
> On 23.07.2015 07:16, fu@linaro.org wrote:
>> From: Fu Wei 
>>
>> Add accessor functions of "loaded" flag in
>> grub-core/loader/arm64/linux.c.
>>
>> Export accessor functions of "loaded" flag and
>> grub_linux_get_fdt function in include/grub/arm64/linux.h.
>>
>> Purpose: Reuse the existing code of devicetree in linux module.
>>
>> Signed-off-by: Fu Wei 
>> ---
>>  grub-core/loader/arm64/linux.c | 13 +
>>  include/grub/arm64/linux.h |  6 +-
>>  2 files changed, 18 insertions(+), 1 deletion(-)
>>
>> diff --git a/grub-core/loader/arm64/linux.c b/grub-core/loader/arm64/linux.c
>> index 987f5b9..cf6026e 100644
>> --- a/grub-core/loader/arm64/linux.c
>> +++ b/grub-core/loader/arm64/linux.c
>> @@ -48,6 +48,19 @@ static grub_addr_t initrd_end;
>>  static void *loaded_fdt;
>>  static void *fdt;
>>
>> +/* The accessor functions for "loaded" flag */
>> +int
>> +grub_linux_get_loaded (void)
>> +{
>> +  return loaded;
>> +}
>> +
>> +void
>> +grub_linux_set_loaded (int loaded_flag)
>> +{
>> +  loaded = loaded_flag;
>> +}
>> +
> Accessor functions are usually useless in GRUB. We have no public API to
> respect. So it only adds clutter. Also "loaded" flag is static for а
> good reason: it's specific to linux.c. I'm going to move fdt part to
> fdt.c and have uniform interface for both linux and xen.
>>  static void *
>>  get_firmware_fdt (void)
>>  {
>> diff --git a/include/grub/arm64/linux.h b/include/grub/arm64/linux.h
>> index 65796d9..20058f3 100644
>> --- a/include/grub/arm64/linux.h
>> +++ b/include/grub/arm64/linux.h
>> @@ -43,10 +43,14 @@ struct grub_arm64_linux_kernel_header
>>  };
>>
>>  /* Declare the functions for getting dtb and checking/booting image */
>> -void *grub_linux_get_fdt (void);
>>  grub_err_t grub_arm64_uefi_check_image (struct 
>> grub_arm64_linux_kernel_header
>>  *lh);
>>  grub_err_t grub_arm64_uefi_boot_image (grub_addr_t addr, grub_size_t size,
>> char *args);
>>
>> +/* Export the accessor functions for gettin dtb and "loaded" flag */
>> +void EXPORT_FUNC (*grub_linux_get_fdt) (void);
>> +int EXPORT_FUNC (grub_linux_get_loaded) (void);
>> +void EXPORT_FUNC (grub_linux_set_loaded) (int loaded_flag);
>> +
> EXPORT_* are necessary only for core. Not for modules.
>>  #endif /* ! GRUB_LINUX_CPU_HEADER */
>>
>
>



-- 
Best regards,

Fu Wei
Software Engineer
Red Hat Software (Beijing) Co.,Ltd.Shanghai Branch
Ph: +86 21 61221326(direct)
Ph: +86 186 2020 4684 (mobile)
Room 1512, Regus One Corporate Avenue,Level 15,
One Corporate Avenue,222 Hubin Road,Huangpu District,
Shanghai,China 200021

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH v3 2/4] arm64: Add xen_boot module file

2015-10-30 Thread Fu Wei 
Hi Vladimir,

yes, Thanks for your modification :-)

I just follow the xen boot protocol :
http://wiki.xen.org/wiki/Xen_ARM_with_Virtualization_Extensions/Multiboot

xen_module is just for "--type" option, I will discuss this with Xen
developer for this.
If we need this option, I will resubmit  it :-)

Great thanks!

On 29 October 2015 at 22:27, Vladimir 'φ-coder/phcoder' Serbinenko
 wrote:
> Committed without the xen_module command. Its argument parsing was
> non-trivial and I don't quite get what its intent is. Can you resubmit?
> On 23.07.2015 07:16, fu@linaro.org wrote:
>> From: Fu Wei 
>>
>> grub-core/loader/arm64/xen_boot.c
>>
>>   - This adds support for the Xen boot on ARM specification for arm64.
>>   - Introduce xen_hypervisor, xen_linux, xen_initrd and xen_xsm
>> to load different binaries for xen boot;
>> Introduce xen_module to load common or custom module for xen boot.
>>   - This Xen boot support is a separated  module for aarch64,
>> but reuse the existing code of devicetree in linux module.
>>
>> Signed-off-by: Fu Wei 
>> ---
>>  grub-core/Makefile.core.def   |   7 +
>>  grub-core/loader/arm64/xen_boot.c | 685 
>> ++
>>  2 files changed, 692 insertions(+)
>>
>> diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
>> index a6101de..796f7e9 100644
>> --- a/grub-core/Makefile.core.def
>> +++ b/grub-core/Makefile.core.def
>> @@ -1648,6 +1648,13 @@ module = {
>>  };
>>
>>  module = {
>> +  name = xen_boot;
>> +  common = lib/cmdline.c;
>> +  arm64 = loader/arm64/xen_boot.c;
>> +  enable = arm64;
>> +};
>> +
>> +module = {
>>name = linux;
>>x86 = loader/i386/linux.c;
>>xen = loader/i386/xen.c;
>> diff --git a/grub-core/loader/arm64/xen_boot.c 
>> b/grub-core/loader/arm64/xen_boot.c
>> new file mode 100644
>> index 000..33a65dd
>> --- /dev/null
>> +++ b/grub-core/loader/arm64/xen_boot.c
>> @@ -0,0 +1,685 @@
>> +/*
>> + *  GRUB  --  GRand Unified Bootloader
>> + *  Copyright (C) 2014  Free Software Foundation, Inc.
>> + *
>> + *  GRUB is free software: you can redistribute it and/or modify
>> + *  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.
>> + *
>> + *  GRUB 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 GRUB.  If not, see .
>> + */
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include/* required by struct xen_hypervisor_header */
>> +#include 
>> +#include 
>> +
>> +GRUB_MOD_LICENSE ("GPLv3+");
>> +
>> +#define XEN_HYPERVISOR_NAME  "xen_hypervisor"
>> +
>> +#define MODULE_DEFAULT_ALIGN  (0x0)
>> +#define MODULE_IMAGE_MIN_ALIGN  MODULE_DEFAULT_ALIGN
>> +#define MODULE_INITRD_MIN_ALIGN  MODULE_DEFAULT_ALIGN
>> +#define MODULE_XSM_MIN_ALIGN  MODULE_DEFAULT_ALIGN
>> +#define MODULE_CUSTOM_MIN_ALIGN  MODULE_DEFAULT_ALIGN
>> +
>> +/* #define MODULE_IMAGE_COMPATIBLE  "xen,linux-image\0xen,module"
>> +#define MODULE_INITRD_COMPATIBLE  "xen,linux-image\0xen,module"
>> +#define MODULE_XSM_COMPATIBLE  "xen,xsm-policy\0xen,module"
>> +#define MODULE_CUSTOM_COMPATIBLE  "xen,module" */
>> +#define MODULE_IMAGE_COMPATIBLE  "multiboot,kernel\0multiboot,module"
>> +#define MODULE_INITRD_COMPATIBLE  "multiboot,ramdisk\0multiboot,module"
>> +#define MODULE_XSM_COMPATIBLE  "xen,xsm-policy\0multiboot,module"
>> +#define MODULE_CUSTOM_COMPATIBLE  "multiboot,module"
>> +
>> +/* This maximum size is defined in Power.org ePAPR V1.1
>> + * https://www.power.org/documentation/epapr-version-1-1/
>> + * 2.2.1.1 Node Name Requirements
>> + * node-name@unit-address
>> + * 31 + 1(@) + 16(64bit address in hex format) + 1(\0) = 49
>> + */
>> +#define FDT_NODE_NAME_MAX_SIZE  (49)
>> +
>> +#define ARG_SHIFT(argc, argv) \
>> +  do { \
>> +(argc)--; \
>> +(argv)++; \
>> +  } while (0)
>> +
>> +struct compat_string_struct
>> +{
>> +  grub_size_t size;
>> +  const char *compat_string;
>> +};
>> +typedef struct compat_string_struct compat_string_struct_t;
>> +#define FDT_COMPATIBLE(x) {.size = sizeof(x), .compat_string = (x)}
>> +
>> +enum module_type
>> +{
>> +  MODULE_IMAGE,
>> +  MODULE_INITRD,
>> +  MODULE_XSM,
>> +  MODULE_CUSTOM
>> +};
>> +typedef enum module_type module_type_t;
>> +
>> +struct fdt_node_info
>> +{
>> +  module_type_t type;
>> +
>> +  const char *compat_string;
>> +  grub_size_t compat_string_size;
>> +};
>> +
>> +struct

Re: [PATCH v3 3/4] * util/grub.d/20_linux_xen.in: Add support of the XEN boot on aarch64

2015-10-30 Thread Fu Wei 
Hi Vladimir,

Great thanks for your suggestion! :-)

On 29 October 2015 at 23:25, Vladimir 'φ-coder/phcoder' Serbinenko
 wrote:
>> +if [ "x$machine" != xaarch64 ]; then
>> + multiboot_cmd="multiboot"
>> + module_linux_cmd="module"
>> + module_initrd_cmd="module --nounzip"
>> +else
>> + multiboot_cmd="xen_hypervisor"
>> + module_linux_cmd="xen_linux"
>> + module_initrd_cmd="xen_initrd"
>> +fi
>> +
> Please do not hardcode an assumption that grub-mkconfig is executed on
> the same machine as GRUB is booted. I know that we have instances of
> such assumption in some cases but we'd like to eliminate them. Alternatives:
> - Check arch on boot time


> - Check that new xen commands are supported (define a new feature)
> Please add xen_* aliases on x86 as well
I would like to go this way, but could you provide some help or a
little example for :
(1) How to check the new xen commands(or xen_boot module)
(2)add xen_* aliases on x86, is that like something below?

diff --git a/grub-core/loader/i386/xen.c b/grub-core/loader/i386/xen.c
index c4d9689..b88d51b 100644
--- a/grub-core/loader/i386/xen.c
+++ b/grub-core/loader/i386/xen.c
@@ -696,10 +696,14 @@ GRUB_MOD_INIT (xen)
   0, N_("Load Linux."));
   cmd_multiboot = grub_register_command ("multiboot", grub_cmd_xen,
 0, N_("Load Linux."));
+  cmd_multiboot = grub_register_command ("xen_hypervisor", grub_cmd_xen,
+0, N_("Load Linux."));
   cmd_initrd = grub_register_command ("initrd", grub_cmd_initrd,
  0, N_("Load initrd."));
   cmd_module = grub_register_command ("module", grub_cmd_module,
  0, N_("Load module."));
+  cmd_module = grub_register_command ("xen_linux", grub_cmd_module,
+ 0, N_("Load module."));
   my_mod = mod;
 }

But how to deal with xen_initrd ?
Could you help me ?

Great thanks !!

>
>



-- 
Best regards,

Fu Wei
Software Engineer
Red Hat Software (Beijing) Co.,Ltd.Shanghai Branch
Ph: +86 21 61221326(direct)
Ph: +86 186 2020 4684 (mobile)
Room 1512, Regus One Corporate Avenue,Level 15,
One Corporate Avenue,222 Hubin Road,Huangpu District,
Shanghai,China 200021

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: Anonymous commit (empty Author and Committer)

2015-10-30 Thread Vladimir 'φ-coder/phcoder' Serbinenko 
On 30.10.2015 21:09, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
> On 30.10.2015 21:06, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
>> On 30.10.2015 15:26, Andrei Borzenkov wrote:
>>> See
>>> http://git.savannah.gnu.org/cgit/grub.git/commit/?id=206676601eb853fc319df14cd3398fbdfde665ac
>>>
>>>
>>> I was not even aware that this is possible. Is there anything on server
>>> side that can prevent it?
>>>
>>> Would be good if commit were amended and force pushed to fix it.
>>>
>> It is a bug in SGit. I'll investigate how it happened

I don't have non-fast-forward rights. Does someone from savannah-users
have them? Could he just delete this commit?




signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: Anonymous commit (empty Author and Committer)

2015-10-30 Thread Vladimir 'phcoder' Serbinenko 
On 30 Oct 2015 9:06 pm, "Vladimir 'φ-coder/phcoder' Serbinenko" <
phco...@gmail.com> wrote:
>
> On 30.10.2015 15:26, Andrei Borzenkov wrote:
> > See
> >
http://git.savannah.gnu.org/cgit/grub.git/commit/?id=206676601eb853fc319df14cd3398fbdfde665ac
> >
> >
> > I was not even aware that this is possible. Is there anything on server
> > side that can prevent it?
> >
> > Would be good if commit were amended and force pushed to fix it.
> >
> It is a bug in SGit. I'll investigate how it happened
Turns out I didn't fill out my name and email after sgit reinstall and it
happily made a commit with no name rather than bailing out like normal git
does. I'll make a patch for sgit
> > ___
> > Grub-devel mailing list
> > Grub-devel@gnu.org
> > https://lists.gnu.org/mailman/listinfo/grub-devel
> > .
> >
>
>
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH v3 3/4] * util/grub.d/20_linux_xen.in: Add support of the XEN boot on aarch64

2015-10-30 Thread Vladimir 'φ-coder/phcoder' Serbinenko 
On 30.10.2015 09:44, Fu Wei wrote:
> Hi Vladimir,
> 
> Great thanks for your suggestion! :-)
> 
> On 29 October 2015 at 23:25, Vladimir 'φ-coder/phcoder' Serbinenko
>  wrote:
>>> +if [ "x$machine" != xaarch64 ]; then
>>> + multiboot_cmd="multiboot"
>>> + module_linux_cmd="module"
>>> + module_initrd_cmd="module --nounzip"
>>> +else
>>> + multiboot_cmd="xen_hypervisor"
>>> + module_linux_cmd="xen_linux"
>>> + module_initrd_cmd="xen_initrd"
>>> +fi
>>> +
>> Please do not hardcode an assumption that grub-mkconfig is executed on
>> the same machine as GRUB is booted. I know that we have instances of
>> such assumption in some cases but we'd like to eliminate them. Alternatives:
>> - Check arch on boot time
> 
> 
>> - Check that new xen commands are supported (define a new feature)
>> Please add xen_* aliases on x86 as well
> I would like to go this way, but could you provide some help or a
> little example for :
> (1) How to check the new xen commands(or xen_boot module)
> (2)add xen_* aliases on x86, is that like something below?
> 
see grub-core/normal/main.c the features array.
> diff --git a/grub-core/loader/i386/xen.c b/grub-core/loader/i386/xen.c
> index c4d9689..b88d51b 100644
> --- a/grub-core/loader/i386/xen.c
> +++ b/grub-core/loader/i386/xen.c
> @@ -696,10 +696,14 @@ GRUB_MOD_INIT (xen)
>0, N_("Load Linux."));
>cmd_multiboot = grub_register_command ("multiboot", grub_cmd_xen,
>  0, N_("Load Linux."));
> +  cmd_multiboot = grub_register_command ("xen_hypervisor", grub_cmd_xen,
> +0, N_("Load Linux."));
>cmd_initrd = grub_register_command ("initrd", grub_cmd_initrd,
>   0, N_("Load initrd."));
>cmd_module = grub_register_command ("module", grub_cmd_module,
>   0, N_("Load module."));
> +  cmd_module = grub_register_command ("xen_linux", grub_cmd_module,
> + 0, N_("Load module."));
>my_mod = mod;
>  }
> 
> But how to deal with xen_initrd ?
> Could you help me ?
> 
Just another alias to module. Possibly you might want to add a code to
xen_initrd tto check that xen_linux was already run
> Great thanks !!
> 
>>
>>
> 
> 
> 




signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: Plain dm-crypt

2015-10-30 Thread Daniel Kahn Gillmor 
On Thu 2015-10-29 13:46:42 -0400, christopher.to...@riseup.net wrote:
> No, since I type the line in manually every time, it is not located 
> anywhere for it to be discovered and need denying. I know my system very 
> well. I know if I put one USB drive into a slot, it will be named 
> (USB0). If I plug more than one USB drive into the system, I know what 
> they will be named based on their physical locations.

"Deniable" crypto seems like it would have very limited utility [0], but
there could still be some marginal use cases for it (for people who
aren't Christopher Toews, anyway: Chris has already admitted publicly on
this list to using encryption on his high-entropy devices, so he can't
deny it any longer).

If the patchset is small/simple (i haven't seen a pointer to the patches
-- can someone supply a link?), and it doesn't introduce any troubling
UI/UX issues, providing the feature in grub doesn't seem like a terrible
idea.

The other use case that Chris hasn't mentioned is that there may be
people who don't trust LUKS to adequately protect the master dm-crypt
key for their volume.  I'm unaware of any legitimate reason to distrust
the cryptography in the LUKS header itself, but i also haven't thought
deeply about attacking it either.

> Look, you can't presume to know my setup better than I do and then try 
> to convince me that I don't require this feature. I understand if it 
> won't be included, but don't hold it out because you think people are 
> too stupid to know how to use it, or worse, because you think people are 
> too stupid to know that they don't need it. There are people who want 
> this feature for good reasons and have the know-how to be able to 
> utilize it. Why can't you just accept that?

I suspect that Vladimir isn't resistant because he thinks people are
stupid.  he's probably resistant like any other responsible long-term
developer/maintainer of crucial infrastructure.  more features == more
support == more bugs, so conservatism on adding features is healthy, and
it's not surprising that he is looking for a more concrete example of
how it might get used.

That said, it's tough to to provide a more concrete use case than the
one descirbed by Chris here.  In particular, this is a feature whose
users will generally not admit to being users (since admitting to being
a user is to lose its benefit).  So i appreciate Chris being willing to
out himself here in service of others who might like to have something
that looks deniable.

(fwiw, i'd really like it if all hard drives shipped from the
manufacturer full of high-entropy noise by default.  That would provide
people like the users Chris is advocating for with much more convincing
cover)

  --dkg

[0] https://www.debian-administration.org/users/dkg/weblog/104 discusses
deniability for chat, but similar analysis probably applies to
encrypted volumes.

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: Anonymous commit (empty Author and Committer)

2015-10-30 Thread Andrei Borzenkov 

30.10.2015 23:59, Lennart Sorensen пишет:

On Fri, Oct 30, 2015 at 09:19:19PM +0100, Vladimir 'φ-coder/phcoder' Serbinenko 
wrote:

On 30.10.2015 21:09, Vladimir 'φ-coder/phcoder' Serbinenko wrote:

On 30.10.2015 21:06, Vladimir 'φ-coder/phcoder' Serbinenko wrote:

On 30.10.2015 15:26, Andrei Borzenkov wrote:

See
http://git.savannah.gnu.org/cgit/grub.git/commit/?id=206676601eb853fc319df14cd3398fbdfde665ac


I was not even aware that this is possible. Is there anything on server
side that can prevent it?

Would be good if commit were amended and force pushed to fix it.


It is a bug in SGit. I'll investigate how it happened


I don't have non-fast-forward rights. Does someone from savannah-users
have them? Could he just delete this commit?


If you do that, then anyone that already did a pull after it went in
will have a broken tree.  Rather annoying.



If we decide to fix this commit it is better done now, while it is the 
last one. It is annoying but do you have suggestion how it can be done 
differently?


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Anonymous commit (empty Author and Committer)

2015-10-30 Thread Andrei Borzenkov 
See 
http://git.savannah.gnu.org/cgit/grub.git/commit/?id=206676601eb853fc319df14cd3398fbdfde665ac


I was not even aware that this is possible. Is there anything on server 
side that can prevent it?


Would be good if commit were amended and force pushed to fix it.

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: Anonymous commit (empty Author and Committer)

2015-10-30 Thread Konstantin Khomoutov 
On Fri, 30 Oct 2015 17:26:00 +0300
Andrei Borzenkov  wrote:

> See 
> http://git.savannah.gnu.org/cgit/grub.git/commit/?id=206676601eb853fc319df14cd3398fbdfde665ac
> 
> I was not even aware that this is possible. Is there anything on
> server side that can prevent it?

A hook running on "update" event could check the commits being pushed
and reject the update if some commit among those does not pass the
necessary checks.

Please see the githooks(5) manual page.

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: Anonymous commit (empty Author and Committer)

2015-10-30 Thread Lennart Sorensen 
On Fri, Oct 30, 2015 at 09:19:19PM +0100, Vladimir 'φ-coder/phcoder' Serbinenko 
wrote:
> On 30.10.2015 21:09, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
> > On 30.10.2015 21:06, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
> >> On 30.10.2015 15:26, Andrei Borzenkov wrote:
> >>> See
> >>> http://git.savannah.gnu.org/cgit/grub.git/commit/?id=206676601eb853fc319df14cd3398fbdfde665ac
> >>>
> >>>
> >>> I was not even aware that this is possible. Is there anything on server
> >>> side that can prevent it?
> >>>
> >>> Would be good if commit were amended and force pushed to fix it.
> >>>
> >> It is a bug in SGit. I'll investigate how it happened
> 
> I don't have non-fast-forward rights. Does someone from savannah-users
> have them? Could he just delete this commit?

If you do that, then anyone that already did a pull after it went in
will have a broken tree.  Rather annoying.

-- 
Len Sorensen

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

[PATCH] mkimage: zero fill alignment space

2015-10-30 Thread Andrei Borzenkov 
This did not cause real problem but is good for reproducible builds. I hit
it with recent bootinfoscript that displays embedded config; I was puzzled
by random garbage at the end.

Also remove redundant zeroing code where we fill in the whole memory block
anyway.

---
 util/mkimage.c | 29 +
 1 file changed, 13 insertions(+), 16 deletions(-)

diff --git a/util/mkimage.c b/util/mkimage.c
index 35df998..1c522fe 100644
--- a/util/mkimage.c
+++ b/util/mkimage.c
@@ -992,7 +992,7 @@ grub_install_generate_image (const char *dir, const char 
*prefix,
 {
   char *kernel_img, *core_img;
   size_t kernel_size, total_module_size, core_size, exec_size;
-  size_t memdisk_size = 0, config_size = 0, config_size_pure = 0;
+  size_t memdisk_size = 0, memdisk_size_pure = 0, config_size = 0, 
config_size_pure = 0;
   size_t prefix_size = 0;
   char *kernel_path;
   size_t offset;
@@ -1035,7 +1035,8 @@ grub_install_generate_image (const char *dir, const char 
*prefix,
 
   if (memdisk_path)
 {
-  memdisk_size = ALIGN_UP(grub_util_get_image_size (memdisk_path), 512);
+  memdisk_size_pure = grub_util_get_image_size (memdisk_path);
+  memdisk_size = ALIGN_UP(memdisk_size_pure, 512);
   grub_util_info ("the size of memory disk is 0x%" GRUB_HOST_PRIxLONG_LONG,
  (unsigned long long) memdisk_size);
   total_module_size += memdisk_size + sizeof (struct grub_module_header);
@@ -1043,8 +1044,8 @@ grub_install_generate_image (const char *dir, const char 
*prefix,
 
   if (config_path)
 {
-  config_size_pure = grub_util_get_image_size (config_path) + 1;
-  config_size = ALIGN_ADDR (config_size_pure);
+  config_size_pure = grub_util_get_image_size (config_path);
+  config_size = ALIGN_ADDR (config_size_pure + 1);
   grub_util_info ("the size of config file is 0x%" GRUB_HOST_PRIxLONG_LONG,
  (unsigned long long) config_size);
   total_module_size += config_size + sizeof (struct grub_module_header);
@@ -1140,19 +1141,21 @@ grub_install_generate_image (const char *dir, const 
char *prefix,
 size_t i;
 for (i = 0; i < npubkeys; i++)
   {
-   size_t curs;
struct grub_module_header *header;
+   size_t key_size, orig_size;
 
-   curs = grub_util_get_image_size (pubkey_paths[i]);
+   orig_size = grub_util_get_image_size (pubkey_paths[i]);
+   key_size = ALIGN_ADDR (orig_size);
 
header = (struct grub_module_header *) (kernel_img + offset);
memset (header, 0, sizeof (struct grub_module_header));
header->type = grub_host_to_target32 (OBJ_TYPE_PUBKEY);
-   header->size = grub_host_to_target32 (curs + sizeof (*header));
+   header->size = grub_host_to_target32 (key_size + sizeof (*header));
offset += sizeof (*header);
 
grub_util_load_image (pubkey_paths[i], kernel_img + offset);
-   offset += ALIGN_ADDR (curs);
+   memset (kernel_img + offset + orig_size, 0, key_size - orig_size);
+   offset += key_size;
   }
   }
 
@@ -1167,6 +1170,7 @@ grub_install_generate_image (const char *dir, const char 
*prefix,
   offset += sizeof (*header);
 
   grub_util_load_image (memdisk_path, kernel_img + offset);
+  memset (kernel_img + offset + memdisk_size_pure, 0, memdisk_size - 
memdisk_size_pure);
   offset += memdisk_size;
 }
 
@@ -1181,7 +1185,7 @@ grub_install_generate_image (const char *dir, const char 
*prefix,
   offset += sizeof (*header);
 
   grub_util_load_image (config_path, kernel_img + offset);
-  *(kernel_img + offset + config_size_pure - 1) = 0;
+  memset (kernel_img + offset + config_size_pure, 0, config_size - 
config_size_pure);
   offset += config_size;
 }
 
@@ -1267,17 +1271,10 @@ grub_install_generate_image (const char *dir, const 
char *prefix,
  = grub_host_to_target_addr (image_target->link_addr);
}
   full_size = core_size + decompress_size;
-
   full_img = xmalloc (full_size);
-  memset (full_img, 0, full_size); 
-
   memcpy (full_img, decompress_img, decompress_size);
-
   memcpy (full_img + decompress_size, core_img, core_size);
 
-  memset (full_img + decompress_size + core_size, 0,
- full_size - (decompress_size + core_size));
-
   free (core_img);
   core_img = full_img;
   core_size = full_size;
-- 
tg: (2066766..) u/mkimage-zero-pad (depends on: master)

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [Savannah-users] Anonymous commit (empty Author and Committer)

2015-10-30 Thread Balaco Baco 

> > See 
> > http://git.savannah.gnu.org/cgit/grub.git/commit/?id=206676601eb853fc3
> 19df14cd3398fbdfde665ac
> >
> >
> > 
> > 
> > I was not even aware that this is possible. Is there anything on 
> > server side that can prevent it?
> > 
> > Would be good if commit were amended and force pushed to fix it.
> > 
> > 
> 
> Is this even a problem? I'm pretty sure Git warns you if you try to
> commit something before user.name and user.email are defined, and if
> someone wants to do so, I don't see why you should try to stop them.
> It wouldn't work, anyway; they would just write some simple name like
> "anonymous" and some nonsense email if they really want to be anonymous.
> 

This commit just removed a line from the README:

"Please look at the GRUB Wiki  for
testing-procedures."

Is this change something that may justify that it's-not-me-there action?
I agree that it should not be prevented to avoid some pressure
situations that might be (eventually) present. Bogus random data there
would be harder to track. The empty fields are easy, and if it
guarantees some safety to whoever does it, should be the choice.


-- 
http://www.fastmail.com - mmm... Fastmail...


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: Anonymous commit (empty Author and Committer)

2015-10-30 Thread Jeff King 
On Fri, Oct 30, 2015 at 05:26:00PM +0300, Andrei Borzenkov wrote:

> See 
> http://git.savannah.gnu.org/cgit/grub.git/commit/?id=206676601eb853fc319df14cd3398fbdfde665ac
> 
> I was not even aware that this is possible. Is there anything on server side
> that can prevent it?

I would have thought that receive.fsckObjects would reject it, but seems
that git-fsck does not complain about it at all, as it is otherwise
syntactically valid (a space separating the zero-length name from the
email, and <> surrounding the empty email).

We do complain during "git commit" about an empty name. We don't seem to
do so for blank emails, though. The only discussion I could find
mentions that should probably disallow both[1].

I wonder how this commit was created in the first place (through
git-commit, and we have an empty-name case that is not covered, or using
a lower-level tool that bypasses the checks).

-Peff

[1] http://article.gmane.org/gmane.comp.version-control.git/261237

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: Anonymous commit (empty Author and Committer)

2015-10-30 Thread Vladimir 'φ-coder/phcoder' Serbinenko 
On 30.10.2015 15:26, Andrei Borzenkov wrote:
> See
> http://git.savannah.gnu.org/cgit/grub.git/commit/?id=206676601eb853fc319df14cd3398fbdfde665ac
> 
> 
> I was not even aware that this is possible. Is there anything on server
> side that can prevent it?
> 
> Would be good if commit were amended and force pushed to fix it.
> 
It is a bug in SGit. I'll investigate how it happened
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
> .
> 




signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: Anonymous commit (empty Author and Committer)

2015-10-30 Thread Vladimir 'φ-coder/phcoder' Serbinenko 
On 30.10.2015 21:06, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
> On 30.10.2015 15:26, Andrei Borzenkov wrote:
>> See
>> http://git.savannah.gnu.org/cgit/grub.git/commit/?id=206676601eb853fc319df14cd3398fbdfde665ac
>>
>>
>> I was not even aware that this is possible. Is there anything on server
>> side that can prevent it?
>>
>> Would be good if commit were amended and force pushed to fix it.
>>
> It is a bug in SGit. I'll investigate how it happened
>> ___
>> Grub-devel mailing list
>> Grub-devel@gnu.org
>> https://lists.gnu.org/mailman/listinfo/grub-devel
>> .
>>
> 
> 




signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] mkimage: zero fill alignment space

2015-10-30 Thread Vladimir 'φ-coder/phcoder' Serbinenko 
On 30.10.2015 16:49, Andrei Borzenkov wrote:
> This did not cause real problem but is good for reproducible builds. I hit
> it with recent bootinfoscript that displays embedded config; I was puzzled
> by random garbage at the end.
> 
> Also remove redundant zeroing code where we fill in the whole memory block
> anyway.
> 
Could we just zero-out the whole block when we allocate it to have an
easier code and avoid it happening again?
> ---
>  util/mkimage.c | 29 +
>  1 file changed, 13 insertions(+), 16 deletions(-)
> 
> diff --git a/util/mkimage.c b/util/mkimage.c
> index 35df998..1c522fe 100644
> --- a/util/mkimage.c
> +++ b/util/mkimage.c
> @@ -992,7 +992,7 @@ grub_install_generate_image (const char *dir, const char 
> *prefix,
>  {
>char *kernel_img, *core_img;
>size_t kernel_size, total_module_size, core_size, exec_size;
> -  size_t memdisk_size = 0, config_size = 0, config_size_pure = 0;
> +  size_t memdisk_size = 0, memdisk_size_pure = 0, config_size = 0, 
> config_size_pure = 0;
>size_t prefix_size = 0;
>char *kernel_path;
>size_t offset;
> @@ -1035,7 +1035,8 @@ grub_install_generate_image (const char *dir, const 
> char *prefix,
>  
>if (memdisk_path)
>  {
> -  memdisk_size = ALIGN_UP(grub_util_get_image_size (memdisk_path), 512);
> +  memdisk_size_pure = grub_util_get_image_size (memdisk_path);
> +  memdisk_size = ALIGN_UP(memdisk_size_pure, 512);
>grub_util_info ("the size of memory disk is 0x%" 
> GRUB_HOST_PRIxLONG_LONG,
> (unsigned long long) memdisk_size);
>total_module_size += memdisk_size + sizeof (struct grub_module_header);
> @@ -1043,8 +1044,8 @@ grub_install_generate_image (const char *dir, const 
> char *prefix,
>  
>if (config_path)
>  {
> -  config_size_pure = grub_util_get_image_size (config_path) + 1;
> -  config_size = ALIGN_ADDR (config_size_pure);
> +  config_size_pure = grub_util_get_image_size (config_path);
> +  config_size = ALIGN_ADDR (config_size_pure + 1);
>grub_util_info ("the size of config file is 0x%" 
> GRUB_HOST_PRIxLONG_LONG,
> (unsigned long long) config_size);
>total_module_size += config_size + sizeof (struct grub_module_header);
> @@ -1140,19 +1141,21 @@ grub_install_generate_image (const char *dir, const 
> char *prefix,
>  size_t i;
>  for (i = 0; i < npubkeys; i++)
>{
> - size_t curs;
>   struct grub_module_header *header;
> + size_t key_size, orig_size;
>  
> - curs = grub_util_get_image_size (pubkey_paths[i]);
> + orig_size = grub_util_get_image_size (pubkey_paths[i]);
> + key_size = ALIGN_ADDR (orig_size);
>  
>   header = (struct grub_module_header *) (kernel_img + offset);
>   memset (header, 0, sizeof (struct grub_module_header));
>   header->type = grub_host_to_target32 (OBJ_TYPE_PUBKEY);
> - header->size = grub_host_to_target32 (curs + sizeof (*header));
> + header->size = grub_host_to_target32 (key_size + sizeof (*header));
>   offset += sizeof (*header);
>  
>   grub_util_load_image (pubkey_paths[i], kernel_img + offset);
> - offset += ALIGN_ADDR (curs);
> + memset (kernel_img + offset + orig_size, 0, key_size - orig_size);
> + offset += key_size;
>}
>}
>  
> @@ -1167,6 +1170,7 @@ grub_install_generate_image (const char *dir, const 
> char *prefix,
>offset += sizeof (*header);
>  
>grub_util_load_image (memdisk_path, kernel_img + offset);
> +  memset (kernel_img + offset + memdisk_size_pure, 0, memdisk_size - 
> memdisk_size_pure);
>offset += memdisk_size;
>  }
>  
> @@ -1181,7 +1185,7 @@ grub_install_generate_image (const char *dir, const 
> char *prefix,
>offset += sizeof (*header);
>  
>grub_util_load_image (config_path, kernel_img + offset);
> -  *(kernel_img + offset + config_size_pure - 1) = 0;
> +  memset (kernel_img + offset + config_size_pure, 0, config_size - 
> config_size_pure);
>offset += config_size;
>  }
>  
> @@ -1267,17 +1271,10 @@ grub_install_generate_image (const char *dir, const 
> char *prefix,
> = grub_host_to_target_addr (image_target->link_addr);
>   }
>full_size = core_size + decompress_size;
> -
>full_img = xmalloc (full_size);
> -  memset (full_img, 0, full_size); 
> -
>memcpy (full_img, decompress_img, decompress_size);
> -
>memcpy (full_img + decompress_size, core_img, core_size);
>  
> -  memset (full_img + decompress_size + core_size, 0,
> -   full_size - (decompress_size + core_size));
> -
>free (core_img);
>core_img = full_img;
>core_size = full_size;
> 




signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel