Re: [PATCH] uboot: Add the missing disk write operation support
On Tue, Jan 15, 2019 at 04:59:56PM +0200, Cristian Ciocaltea wrote: > uboot_disk_write() is currently lacking the write support > to storage devices because, historically, those devices did not > implement block_write() in U-Boot. > > The solution has been tested using a patched U-Boot loading > and booting GRUB in a QEMU vexpress-a9 environment. > The disk write operations were triggered with GRUB's save_env > command. > > Signed-off-by: Cristian Ciocaltea Reviewed-by: Daniel Kiper If there are no objections I will apply this patch in a week or so. Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH v2 2/3] grub: Make grub_envblk_iterate() return an int
On Wed, Jan 16, 2019 at 01:34:42PM -0500, Prarit Bhargava wrote: > grub_envblk_iterate() is a void. Future functions will require the > ability to interpret return codes from the iteration, so > grub_envblk_iterate() should be an int. > > The value of 0 returned from the hook functions is overloaded and cannot > be parsed by callers to grub_envblk_iterate() for error handling. To fix > this add GRUB_ERR_ITERATE_STOP and GRUB_ERR_ITERATE_CONTINUE to the error > returns so the iteration can be stopped and continued respectively. > > Make grub_envblk_iterate() return an int. > > Signed-off-by: Prarit Bhargava I do not like this patch. First of all GRUB errors are defined as an enum not as an int. The second: do not add error codes if they are not really needed. And yours are not needed at all. Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH v2 3/3] grub: add extend variable functionality
On Wed, Jan 16, 2019 at 01:34:43PM -0500, Prarit Bhargava wrote: > Customers and users of the kernel are commenting that there is no way to > update > a grub variable without copy and pasting the existing data. > > For example, > > [10:57 AM root@intel-wildcatpass-07 grub-2.02]# ./grub-editenv list > saved_entry=0 > next_entry= > kernelopts=root=/dev/mapper/rhel_intel--wildcatpass--07-root ro > crashkernel=auto resume=/dev/mapper/rhel_intel--wildcatpass--07-swap > rd.lvm.lv=rhel_intel-wildcatpass-07/root > rd.lvm.lv=rhel_intel-wildcatpass-07/swap console=ttyS0,115200n81 > ignore_loglevel > [10:57 AM root@intel-wildcatpass-07 grub-2.02]# ./grub-editenv - set > kernelopts="root=/dev/mapper/rhel_intel--wildcatpass--07-root ro > crashkernel=auto resume=/dev/mapper/rhel_intel--wildcatpass--07-swap > rd.lvm.lv=rhel_intel-wildcatpass-07/root > rd.lvm.lv=rhel_intel-wildcatpass-07/swap console=ttyS0,115200n81 > ignore_loglevel newarg" > [10:57 AM root@intel-wildcatpass-07 grub-2.02]# ./grub-editenv list > saved_entry=0 > next_entry= > kernelopts=root=/dev/mapper/rhel_intel--wildcatpass--07-root ro > crashkernel=auto resume=/dev/mapper/rhel_intel--wildcatpass--07-swap > rd.lvm.lv=rhel_intel-wildcatpass-07/root > rd.lvm.lv=rhel_intel-wildcatpass-07/swap console=ttyS0,115200n81 > ignore_loglevel newarg > > which is cumbersome. > > Add functionality to add to an existing variable. For example, > > [10:58 AM root@intel-wildcatpass-07 grub-2.02]# ./grub-editenv list > saved_entry=0 > next_entry= > kernelopts=root=/dev/mapper/rhel_intel--wildcatpass--07-root ro > crashkernel=auto resume=/dev/mapper/rhel_intel--wildcatpass--07-swap > rd.lvm.lv=rhel_intel-wildcatpass-07/root > rd.lvm.lv=rhel_intel-wildcatpass-07/swap console=ttyS0,115200n81 > ignore_loglevel > [10:58 AM root@intel-wildcatpass-07 grub-2.02]# ./grub-editenv - set > kernelopts+="newarg" > [10:59 AM root@intel-wildcatpass-07 grub-2.02]# ./grub-editenv list > saved_entry=0 > next_entry= > kernelopts=root=/dev/mapper/rhel_intel--wildcatpass--07-root ro > crashkernel=auto resume=/dev/mapper/rhel_intel--wildcatpass--07-swap > rd.lvm.lv=rhel_intel-wildcatpass-07/root > rd.lvm.lv=rhel_intel-wildcatpass-07/swap console=ttyS0,115200n81 > ignore_loglevel newarg > > Signed-off-by: Prarit Bhargava I prefer v1 patch +/- my suggestions. IMO current solution is an overkill. Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH v2 1/3] grub-editenv: Verify the variable size
On Wed, Jan 16, 2019 at 01:34:41PM -0500, Prarit Bhargava wrote:
> It is possible to execute 'grub-editenv - set ="some data"',
> which results in an unremoveable entry
>
> ="some data"
Please explain why it happens.
> Verify the variable has a size before setting a value.
s/variable/argument/
s/size/size greater than zero/
> Signed-off-by: Prarit Bhargava
> Cc: mleit...@redhat.com
> Cc: pjo...@redhat.com
> Cc: javi...@redhat.com
> Cc: ar...@redhat.com
> Cc: Daniel Kiper
> ---
> util/grub-editenv.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/util/grub-editenv.c b/util/grub-editenv.c
> index 118e89fe57fe..0b6c69b9688c 100644
> --- a/util/grub-editenv.c
> +++ b/util/grub-editenv.c
> @@ -217,6 +217,9 @@ set_variables (const char *name, int argc, char *argv[])
>
>*(p++) = 0;
>
> + if (! grub_strlen(argv[0]))
> +grub_util_error (_("No parameter specified"));
s/parameter/argument/
Daniel
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: MKPasswd idempotency patch
On Tue, Dec 25, 2018 at 12:01:12PM +0100, grub-de...@agowa338.de wrote:
> Hello,
>
> I want to use grub-mkpasswd inside of a ansible playbook, therefore a
> idempotency or check feature is required. Without it, there is no
> posibility to verify, that the configuration is already inplace.
> Therefore I want to suggest implementing the attached changes. The
> simplest one I was thinking of, is grub-mkpasswd allowing to specify a
> salt and comparing the results.
I am not sure I understand. Could you elaborate on this?
> My patch also introduces a quiet mode, that basically suppresses all
> prompts and only expects the password to be entered/piped in once. It
> also changes the output slightly to not inlcude the `PBKDF2 hash of
> your password is `-part.
It seems to me that it should be split into 3 parts...
> The patch is attached to this mail.
>
> Best,
> Klaus Frank
> >From d289bbb5f5dffbbed2c871989f55e48b756e9ae2 Mon Sep 17 00:00:00 2001
> From: Klaus Frank
> Date: Tue, 25 Dec 2018 06:17:38 +0100
> Subject: [PATCH] Implement salt-value and quiet parameter for
> grub-mkpasswd-pbkdf2
>
> ---
> util/grub-mkpasswd-pbkdf2.c | 103 ++--
> 1 file changed, 88 insertions(+), 15 deletions(-)
>
> diff --git a/util/grub-mkpasswd-pbkdf2.c b/util/grub-mkpasswd-pbkdf2.c
> index 5805f3c10..018715bf3 100644
> --- a/util/grub-mkpasswd-pbkdf2.c
> +++ b/util/grub-mkpasswd-pbkdf2.c
> @@ -42,10 +42,15 @@
>
> #include "progname.h"
>
> +static int dec_from_hex(const int); // Converts the charcode into an integer
> representation of the representing hex value.
> +int unhexify(grub_uint8_t*, const char*);
> +
> static struct argp_option options[] = {
>{"iteration-count", 'c', N_("NUM"), 0, N_("Number of PBKDF2 iterations"),
> 0},
>{"buflen", 'l', N_("NUM"), 0, N_("Length of generated hash"), 0},
>{"salt", 's', N_("NUM"), 0, N_("Length of salt"), 0},
> + {"salt-value", 'v', N_("STR"), 0, N_("Salt"), 0},
For this patch number one...
> + {"quiet", 'q', 0, 0, N_("Only output hash, suppress other output, indended
> for pipes"), 0},
...and for this patch number two.
The rest goes into patch three. The basic idea is that one logical
change is one patch.
>{ 0, 0, 0, 0, 0, 0 }
> };
>
> @@ -54,6 +59,8 @@ struct arguments
>unsigned int count;
>unsigned int buflen;
>unsigned int saltlen;
> + char *value;
> + unsigned char quiet;
> };
>
> static error_t
> @@ -76,6 +83,15 @@ argp_parser (int key, char *arg, struct argp_state *state)
> case 's':
>arguments->saltlen = strtoul (arg, NULL, 0);
>break;
> +
> +case 'v':
> + arguments->value = arg;
> + break;
> +
> +case 'q':
> + arguments->quiet = 1;
> + break;
> +
> default:
>return ARGP_ERR_UNKNOWN;
> }
> @@ -94,29 +110,70 @@ hexify (char *hex, grub_uint8_t *bin, grub_size_t n)
> {
>while (n--)
> {
> - if (((*bin & 0xf0) >> 4) < 10)
> - *hex = ((*bin & 0xf0) >> 4) + '0';
> - else
> - *hex = ((*bin & 0xf0) >> 4) + 'A' - 10;
> + if (((*bin & 0xf0) >> 4) < 10) {
> +*hex = ((*bin & 0xf0) >> 4) + '0';
> + } else {
Curly braces are not needed here and below.
> +*hex = ((*bin & 0xf0) >> 4) + 'A' - 10;
> + }
Please do not introduce clutter like this. If you wish to do some
cleanups do it in separate patch.
>hex++;
>
>if ((*bin & 0xf) < 10)
> - *hex = (*bin & 0xf) + '0';
> +*hex = (*bin & 0xf) + '0';
Ditto.
>else
> - *hex = (*bin & 0xf) + 'A' - 10;
> +*hex = (*bin & 0xf) + 'A' - 10;
Ditto.
>hex++;
>bin++;
> }
>*hex = 0;
> }
>
> +static int
> +dec_from_hex(const int hex) {
> + if (48 <= hex && hex <= 57) {
> +return hex - 48;
> + } else if (65 <= hex && hex <= 90) {
> +return hex - 65 + 10;
> + } else if (97 <= hex && hex <= 122) {
> +return hex - 97 + 10;
> + } else {
> +exit(1);
> + }
> +}
grub_strtoul() please.
> +int
> +unhexify(grub_uint8_t* output, const char* hexstring) {
> + // sizeof(output) == (sizeof(hexstring) - 1) / 2 + 1
> + if (sizeof(output) < (sizeof(hexstring) - 1) / 2 + 1) {
> +return -1;
> + }
> + char *output_ptr = NULL;
> + output_ptr = (char*)output;
> + int CharCodeInDecimal = 0;
> + void *HexDigits = xmalloc(sizeof("FF"));
> + char *HexDigitOne = HexDigits;
> + char *HexDigitTwo = (char*)HexDigits + 1;
> + char *HexString_ptr = NULL;
> + HexString_ptr = (char*)hexstring;
> + while(*HexString_ptr) {
> +memcpy (HexDigitOne, HexString_ptr++, sizeof (char));
> +memcpy (HexDigitTwo, HexString_ptr++, sizeof (char));
> +
> +CharCodeInDecimal = dec_from_hex((int)*HexDigitOne) * 16 +
> dec_from_hex((int)*HexDigitTwo);
> +*output_ptr++ = (char)CharCodeInDecimal;
> + }
> + output_ptr = '\0';
> + return 0;
> +}
This is a mess. There is a pretty good chance that similar function
exists in the wild. So, please do not reinvent the wheel.
Daniel
_
Re: GRUB Xen PVH chainloading
On Mon, Jan 07, 2019 at 01:43:11PM +0100, Juergen Gross wrote: > On 07/01/2019 12:41, Colin Watson wrote: > > Hi, > > > > I'm working on integrating the recently-merged PVH support for GRUB into > > the Debian packages. As a result I find myself thinking about how to > > handle the two-stage boot loader scheme that our packages currently > > implement for PV. I think that it would not be very hard to do this for > > PVH in the manner outlined below, but my x86 asm skills aren't quite up > > to some of the work needed in GRUB. Assuming that nobody sees any > > obvious holes in this, does anyone fancy giving it a go? > > Seems to be a very good idea. > > > Background > > -- > > > > Around the time PV support was implemented in GRUB 2, we put together a > > scheme to minimise the coupling between the guest configuration file on > > the host and the boot loader configuration in the guest. The scheme and > > its rationale are described here: > > > > > > https://wiki.xen.org/wiki/PvGrub2#Chainloading_guest_pvgrub2_from_domain_0_pvgrub2 > > > > Essentially the same rationale applies to the PVH case: it should be > > possible for the guest to declare its own booting arrangements (though > > of course some hosts may wish to just provide a grub.cfg and handle all > > that on the host side), and this should be decoupled from the GRUB image > > provided by the host as far as possible in order to minimise > > compatibility issues. > > > > There seems to be no obstacle to this in principle: just as a PV boot > > loader can chainload another one from the guest's disk, so too could a > > PVH boot loader chainload another one from the guest's disk. > > > > What needs to be done > > - > > > > GRUB needs to support chainloading another PVH boot loader. I think > > this involves observing the existence of the XEN_ELFNOTE_PHYS32_ENTRY > > note and following the machine state rules for the domain builder here: > > > > https://xenbits.xen.org/docs/unstable-staging/misc/pvh.html > > > > (I had a brief go at implementing this, but foundered on my fairly > > minimal understanding of GRUB's relocator/boot code.) > > The needed effort should indeed be rather small. > > In the moment I have no idea when I'll be able to do it, as I have > plenty of other things to do. In case you want to try it and need some > hints, please feel free to ask (maybe I'm able to give an answer > without having to try to implement it myself ;-) ). > > > We need to define a modification to > > https://xenbits.xen.org/docs/unstable-staging/misc/x86-xenpv-bootloader.html > > for PVH boot loaders. I suggest the obvious: a second-stage PVH boot > > loader should be installed into the guest filesystem as > > /boot/xen/pvhboot-.elf, and otherwise things generally behave the > > same way. I'd be happy to draft a patch to the protocol specification > > once a proof-of-concept exists. > > > > The as-yet-unmerged GRUB patch to support the existing PV boot protocol > > (https://salsa.debian.org/grub-team/grub/blob/master/debian/patches/grub-install-pvxen-paths.patch) > > needs to be extended to support the amended protocol. This is trivial > > given the above. > > Would you mind trying to upstream this patch? I have CC-ed Daniel Kiper > one of the grub2 maintainers), as I guess with his Xen skills he will be > the one looking at the patch. Yes, I am happy to help in development, e.g. give some hints if needed, and review the patches. Sadly I am not able to do development myself because I am busy with other stuff right now. Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH 1/1] Upgrade Gnulib; switch to bootstrap tool
On Wed, Jan 09, 2019 at 12:29:30PM +, Colin Watson wrote: > Upgrade Gnulib files to 20190105. > > It's much easier to maintain GRUB's use of portability support files > from Gnulib when the process is automatic and driven by a single > configuration file, rather than by maintainers occasionally running > gnulib-tool and committing the result. Removing these > automatically-copied files from revision control also removes the > temptation to hack the output in ways that are difficult for future > maintainers to follow. Gnulib includes a "bootstrap" program which is > designed for this. > > The canonical way to bootstrap GRUB from revision control is now > "./bootstrap", but "./autogen.sh" is still useful if you just want to > generate the GRUB-specific parts of the build system. > > GRUB now requires Autoconf >= 2.63 and Automake >= 1.11, in line with > Gnulib. Could you update INSTALL file too? Additionally, if you do such huge change could you move Gnulib from grub-core/gnulib to grub-core/lib/gnulib? Including all diffs which are currently outside of grub-core/gnulib directory. Well, maybe *.diff files should be renamed to *.patch... Could you add to the commit message how to exactly update Gnulib? Just list of steps like in the commit 461f1d8af (Import upstream zstd-1.3.6). Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH v2 2/3] grub: Make grub_envblk_iterate() return an int
grub_envblk_iterate() is a void. Future functions will require the
ability to interpret return codes from the iteration, so
grub_envblk_iterate() should be an int.
The value of 0 returned from the hook functions is overloaded and cannot
be parsed by callers to grub_envblk_iterate() for error handling. To fix
this add GRUB_ERR_ITERATE_STOP and GRUB_ERR_ITERATE_CONTINUE to the error
returns so the iteration can be stopped and continued respectively.
Make grub_envblk_iterate() return an int.
Signed-off-by: Prarit Bhargava
Cc: mleit...@redhat.com
Cc: pjo...@redhat.com
Cc: javi...@redhat.com
Cc: ar...@redhat.com
Cc: Daniel Kiper
---
grub-core/commands/loadenv.c | 6 +++---
grub-core/lib/envblk.c | 17 -
include/grub/err.h | 4 +++-
include/grub/lib/envblk.h| 6 +++---
util/grub-editenv.c | 2 +-
5 files changed, 18 insertions(+), 17 deletions(-)
diff --git a/grub-core/commands/loadenv.c b/grub-core/commands/loadenv.c
index 3fd664aac331..1bbae5f7f332 100644
--- a/grub-core/commands/loadenv.c
+++ b/grub-core/commands/loadenv.c
@@ -144,14 +144,14 @@ set_var (const char *name, const char *value, void
*whitelist)
if (! whitelist)
{
grub_env_set (name, value);
- return 0;
+ return GRUB_ERR_ITERATE_CONTINUE;
}
if (test_whitelist_membership (name,
(const grub_env_whitelist_t *) whitelist))
grub_env_set (name, value);
- return 0;
+ return GRUB_ERR_ITERATE_CONTINUE;
}
static grub_err_t
@@ -192,7 +192,7 @@ print_var (const char *name, const char *value,
void *hook_data __attribute__ ((unused)))
{
grub_printf ("%s=%s\n", name, value);
- return 0;
+ return GRUB_ERR_ITERATE_CONTINUE;
}
static grub_err_t
diff --git a/grub-core/lib/envblk.c b/grub-core/lib/envblk.c
index 230e0e9d9abe..3e43843c4535 100644
--- a/grub-core/lib/envblk.c
+++ b/grub-core/lib/envblk.c
@@ -223,7 +223,7 @@ grub_envblk_delete (grub_envblk_t envblk, const char *name)
}
}
-void
+int
grub_envblk_iterate (grub_envblk_t envblk,
void *hook_data,
int hook (const char *name, const char *value, void
*hook_data))
@@ -247,8 +247,7 @@ grub_envblk_iterate (grub_envblk_t envblk,
while (p < pend && *p != '=')
p++;
if (p == pend)
-/* Broken. */
-return;
+return GRUB_ERR_OUT_OF_RANGE;
name_end = p;
p++;
@@ -264,13 +263,11 @@ grub_envblk_iterate (grub_envblk_t envblk,
}
if (p >= pend)
-/* Broken. */
-return;
+return GRUB_ERR_OUT_OF_RANGE;
name = grub_malloc (p - name_start + 1);
if (! name)
-/* out of memory. */
-return;
+return GRUB_ERR_OUT_OF_MEMORY;
value = name + (value_start - name_start);
@@ -288,10 +285,12 @@ grub_envblk_iterate (grub_envblk_t envblk,
ret = hook (name, value, hook_data);
grub_free (name);
- if (ret)
-return;
+ if (ret != GRUB_ERR_ITERATE_CONTINUE)
+return ret;
}
p = find_next_line (p, pend);
}
+
+return GRUB_ERR_NONE;
}
diff --git a/include/grub/err.h b/include/grub/err.h
index 1590c688e1d3..75ebb54201e8 100644
--- a/include/grub/err.h
+++ b/include/grub/err.h
@@ -71,7 +71,9 @@ typedef enum
GRUB_ERR_NET_PACKET_TOO_BIG,
GRUB_ERR_NET_NO_DOMAIN,
GRUB_ERR_EOF,
-GRUB_ERR_BAD_SIGNATURE
+GRUB_ERR_BAD_SIGNATURE,
+GRUB_ERR_ITERATE_STOP,
+GRUB_ERR_ITERATE_CONTINUE
}
grub_err_t;
diff --git a/include/grub/lib/envblk.h b/include/grub/lib/envblk.h
index c3e655921709..7637791f4228 100644
--- a/include/grub/lib/envblk.h
+++ b/include/grub/lib/envblk.h
@@ -34,9 +34,9 @@ typedef struct grub_envblk *grub_envblk_t;
grub_envblk_t grub_envblk_open (char *buf, grub_size_t size);
int grub_envblk_set (grub_envblk_t envblk, const char *name, const char
*value);
void grub_envblk_delete (grub_envblk_t envblk, const char *name);
-void grub_envblk_iterate (grub_envblk_t envblk,
- void *hook_data,
- int hook (const char *name, const char *value, void
*hook_data));
+int grub_envblk_iterate (grub_envblk_t envblk,
+ void *hook_data,
+ int hook (const char *name, const char *value, void
*hook_data));
void grub_envblk_close (grub_envblk_t envblk);
static inline char *
diff --git a/util/grub-editenv.c b/util/grub-editenv.c
index 0b6c69b9688c..5f87b09d924a 100644
--- a/util/grub-editenv.c
+++ b/util/grub-editenv.c
@@ -169,7 +169,7 @@ print_var (const char *varname, const char *value,
void *hook_data __attribute__ ((unused)))
{
printf ("%s=%s\n", varname, value);
- return 0;
+ return GRUB_ERR_ITERATE_CONTINUE;
}
static void
--
2.17.2
___
Gru
[PATCH v2 0/3] grub: add grub variable extend functionality
This patchset allows a user to update an existing grub variable on the commandline. v2: - Split into 3 patches. - fix "=value" bug. An additional consequence of this is that the value pointer p in set_variable() cannot underflow. - make grub_envblk_iterate return an int and fix return values. - add "+=" functionality - Changes suggested by David Kiper: Change name to grub_envblk_extend(), capitalize comments, use standard GRUB error enum, change goto from "out" to "err", drop cast in grub_strlen() call, use grub_snprintf() instead of strcpy & memcpy. - Instead of parsing envblk separately, use grub_envblk_iterate() - use a case statement in set_variable() error handling, and add a comment about deleting the "=" sign. Signed-off-by: Prarit Bhargava Cc: mleit...@redhat.com Cc: pjo...@redhat.com Cc: javi...@redhat.com Cc: ar...@redhat.com Cc: Daniel Kiper Prarit Bhargava (3): grub-editenv: Verify the variable size grub: Make grub_envblk_iterate() return an int grub: add extend variable functionality grub-core/commands/loadenv.c | 6 +-- grub-core/lib/envblk.c | 17 --- include/grub/err.h | 4 +- include/grub/lib/envblk.h| 6 +-- util/grub-editenv.c | 86 ++-- 5 files changed, 100 insertions(+), 19 deletions(-) -- 2.17.2 ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH v2 3/3] grub: add extend variable functionality
Customers and users of the kernel are commenting that there is no way to update
a grub variable without copy and pasting the existing data.
For example,
[10:57 AM root@intel-wildcatpass-07 grub-2.02]# ./grub-editenv list
saved_entry=0
next_entry=
kernelopts=root=/dev/mapper/rhel_intel--wildcatpass--07-root ro
crashkernel=auto resume=/dev/mapper/rhel_intel--wildcatpass--07-swap
rd.lvm.lv=rhel_intel-wildcatpass-07/root
rd.lvm.lv=rhel_intel-wildcatpass-07/swap console=ttyS0,115200n81
ignore_loglevel
[10:57 AM root@intel-wildcatpass-07 grub-2.02]# ./grub-editenv - set
kernelopts="root=/dev/mapper/rhel_intel--wildcatpass--07-root ro
crashkernel=auto resume=/dev/mapper/rhel_intel--wildcatpass--07-swap
rd.lvm.lv=rhel_intel-wildcatpass-07/root
rd.lvm.lv=rhel_intel-wildcatpass-07/swap console=ttyS0,115200n81
ignore_loglevel newarg"
[10:57 AM root@intel-wildcatpass-07 grub-2.02]# ./grub-editenv list
saved_entry=0
next_entry=
kernelopts=root=/dev/mapper/rhel_intel--wildcatpass--07-root ro
crashkernel=auto resume=/dev/mapper/rhel_intel--wildcatpass--07-swap
rd.lvm.lv=rhel_intel-wildcatpass-07/root
rd.lvm.lv=rhel_intel-wildcatpass-07/swap console=ttyS0,115200n81
ignore_loglevel newarg
which is cumbersome.
Add functionality to add to an existing variable. For example,
[10:58 AM root@intel-wildcatpass-07 grub-2.02]# ./grub-editenv list
saved_entry=0
next_entry=
kernelopts=root=/dev/mapper/rhel_intel--wildcatpass--07-root ro
crashkernel=auto resume=/dev/mapper/rhel_intel--wildcatpass--07-swap
rd.lvm.lv=rhel_intel-wildcatpass-07/root
rd.lvm.lv=rhel_intel-wildcatpass-07/swap console=ttyS0,115200n81
ignore_loglevel
[10:58 AM root@intel-wildcatpass-07 grub-2.02]# ./grub-editenv - set
kernelopts+="newarg"
[10:59 AM root@intel-wildcatpass-07 grub-2.02]# ./grub-editenv list
saved_entry=0
next_entry=
kernelopts=root=/dev/mapper/rhel_intel--wildcatpass--07-root ro
crashkernel=auto resume=/dev/mapper/rhel_intel--wildcatpass--07-swap
rd.lvm.lv=rhel_intel-wildcatpass-07/root
rd.lvm.lv=rhel_intel-wildcatpass-07/swap console=ttyS0,115200n81
ignore_loglevel newarg
Signed-off-by: Prarit Bhargava
Cc: mleit...@redhat.com
Cc: pjo...@redhat.com
Cc: javi...@redhat.com
Cc: ar...@redhat.com
---
util/grub-editenv.c | 81 +++--
1 file changed, 79 insertions(+), 2 deletions(-)
diff --git a/util/grub-editenv.c b/util/grub-editenv.c
index 5f87b09d924a..bc997c5312d0 100644
--- a/util/grub-editenv.c
+++ b/util/grub-editenv.c
@@ -201,6 +201,49 @@ write_envblk (const char *name, grub_envblk_t envblk)
fclose (fp);
}
+struct extend_variable_data {
+ grub_envblk_t envblk;
+ const char *name;
+ const char *add;
+};
+
+static int
+extend_variable_hook (const char *varname, const char *value, void *hook_data)
+{
+ char *new = NULL;
+ int ret = GRUB_ERR_NONE;
+ struct extend_variable_data *data = hook_data;
+
+ if (grub_strcmp (data->name, varname))
+return GRUB_ERR_ITERATE_CONTINUE;
+
+ /* Create the new entry.*/
+ new = grub_zalloc (grub_strlen (value) + grub_strlen (data->add) + 2);
+ if (!new)
+{
+ return GRUB_ERR_OUT_OF_MEMORY;
+ goto err;
+}
+ grub_snprintf(new, grub_strlen (value) + grub_strlen (data->add) + 2,
+ "%s %s", value, data->add);
+
+ /* Erase the current entry. */
+ grub_envblk_delete (data->envblk, varname);
+ /* Add the updated entry. */
+ if (! grub_envblk_set (data->envblk, varname, new))
+{
+ /* Restore the original entry. This should always work. */
+ grub_envblk_set (data->envblk, varname, value);
+ ret = GRUB_ERR_EOF;
+ goto err;
+}
+
+ err:
+grub_free(new);
+return GRUB_ERR_ITERATE_STOP;
+
+}
+
static void
set_variables (const char *name, int argc, char *argv[])
{
@@ -210,18 +253,52 @@ set_variables (const char *name, int argc, char *argv[])
while (argc)
{
char *p;
+ struct extend_variable_data data;
+ int err;
p = strchr (argv[0], '=');
if (! p)
grub_util_error (_("invalid parameter %s"), argv[0]);
+ /* Delete "=" to separate variable name and value */
*(p++) = 0;
if (! grub_strlen(argv[0]))
grub_util_error (_("No parameter specified"));
- if (! grub_envblk_set (envblk, argv[0], p))
-grub_util_error ("%s", _("environment block too small"));
+ switch (*(p - 2))
+{
+ case '+': /* Extend a parameter */
+*(p - 2) = 0; /* Delete "+" from variable name */
+
+data.envblk = envblk;
+data.name = argv[0];
+data.add = p;
+err = grub_envblk_iterate (envblk, &data, extend_variable_hook);
+switch (err)
+ {
+ case GRUB_ERR_ITERATE_STOP: /* Success */
+ break;
+
+case GRUB_ERR_OUT_OF_RANGE:
+case GRUB_ERR_EOF:
+ grub_util_error ("%s", _("environment block too small"));
+ break;
+
+
[PATCH v2 1/3] grub-editenv: Verify the variable size
It is possible to execute 'grub-editenv - set ="some data"',
which results in an unremoveable entry
="some data"
Verify the variable has a size before setting a value.
Signed-off-by: Prarit Bhargava
Cc: mleit...@redhat.com
Cc: pjo...@redhat.com
Cc: javi...@redhat.com
Cc: ar...@redhat.com
Cc: Daniel Kiper
---
util/grub-editenv.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/util/grub-editenv.c b/util/grub-editenv.c
index 118e89fe57fe..0b6c69b9688c 100644
--- a/util/grub-editenv.c
+++ b/util/grub-editenv.c
@@ -217,6 +217,9 @@ set_variables (const char *name, int argc, char *argv[])
*(p++) = 0;
+ if (! grub_strlen(argv[0]))
+grub_util_error (_("No parameter specified"));
+
if (! grub_envblk_set (envblk, argv[0], p))
grub_util_error ("%s", _("environment block too small"));
--
2.17.2
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
