On Sat, 2 Sep 2023 22:03:08 +0200
"Vladimir 'phcoder' Serbinenko" <phco...@gmail.com> wrote:
> 2 problems:
> 1) Did you audit that all call places either clear or propagate error?
Am I correct in understanding that you're asking if all calls to
grub_xvasprintf() with clear grub_errno or return it? I have not
checked that. There's not many calls of grub_xvasprintf(), but there's
138 calls to grub_xasprintf(). I have checked a few and it looks like
many return grub_errno when grub_xasprintf() returns NULL right now.
So those would currently be returning either SUCCESS when they
shouldn't or a failure code that is not what it should be. Those
already assume that grub_xvasprintf() works as if this patch were
applied.
Is your concern that there is code that relies on this
bug in grub_xvasprintf()? For instance, that some function returns
grub_errno after a failed grub_xvasprintf(), but the callee expects to
receive some prior error code instead of the out of memory error?
> 2) We use standard error message "out of memory". This allows it to be
> translated and not be too technical.
I made it more specific so its easier to tell where the out of memory
is coming from. I suppose this could be done better in other ways, for
instance adding file and line number to all grub_error calls like is
done in redhat's grub repo. So I'll change this.
Glenn
> Le sam. 2 sept. 2023, 04:17, Glenn Washburn <developm...@efficientek.com> a
> écrit :
>
> > When failing to allocate the preallocate buffer, grub_xvasprintf()
> > returns NULL, but does not set grub_errno. Returning NULL is sufficient
> > for a caller to determine there was an error. However, some usages of
> > grub_xvasprintf() check for a NULL return value and then return
> > grub_errno, which could return some previously set error code or
> > potentially even a success code.
> >
> > Signed-off-by: Glenn Washburn <developm...@efficientek.com>
> > ---
> > grub-core/kern/misc.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/grub-core/kern/misc.c b/grub-core/kern/misc.c
> > index b57249acb81b..afb41bd63a6a 100644
> > --- a/grub-core/kern/misc.c
> > +++ b/grub-core/kern/misc.c
> > @@ -1232,6 +1232,7 @@ grub_xvasprintf (const char *fmt, va_list ap)
> > if (!ret)
> > {
> > free_printf_args (&args);
> > + grub_error (GRUB_ERR_OUT_OF_MEMORY, "grub_xvasprintf failed to
> > allocate memory of size %" PRIuGRUB_SIZE, as);
> > return NULL;
> > }
> >
> > --
> > 2.34.1
> >
> >
> > _______________________________________________
> > 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
