Date: Fri, 19 Jun 2015 17:09:45 -0600 From: Ian Lepore <ian@freebsd.org> To: Maxim Sobolev <sobomax@FreeBSD.org> Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r284614 - head/sys/boot/uboot/lib Message-ID: <1434755385.1415.114.camel@freebsd.org> In-Reply-To: <201506192224.t5JMOxpC097306@svn.freebsd.org> References: <201506192224.t5JMOxpC097306@svn.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, 2015-06-19 at 22:24 +0000, Maxim Sobolev wrote: > Author: sobomax > Date: Fri Jun 19 22:24:58 2015 > New Revision: 284614 > URL: https://svnweb.freebsd.org/changeset/base/284614 > > Log: > Provide bug4bug workaround for certain dumbiness of the u-boot's API_env_enum > function, which is expected to set returned env to NULL upon reaching the end > of the environment list but fails to do so in certain cases. The respective > u-boot code looks like the following (HEAD at the time of this commit): > > --- api.c --- > 496 static int API_env_enum(va_list ap) > ... > 510 *next = last; > 511 > 512 for (i = 0; env_get_char(i) != '\0'; i = n + 1) { > 513 for (n = i; env_get_char(n) != '\0'; ++n) { > 514 if (n >= CONFIG_ENV_SIZE) { > 515 /* XXX shouldn't we set *next = NULL?? */ > 516 return 0; > 517 } > 518 } > ------------- > > The net result is that any unfortunate user of the loader's ub_env_enum() > function hitting this condition would be trapped in the infinite loop, as > the main use pattern of ub_env_enum() is basically the following: > > while ((env = ub_env_enum(env)) != NULL) { DO STUFF } > > Which would stuck forever with the last element. > > Modified: > head/sys/boot/uboot/lib/glue.c > > Modified: head/sys/boot/uboot/lib/glue.c > ============================================================================== > --- head/sys/boot/uboot/lib/glue.c Fri Jun 19 21:55:12 2015 (r284613) > +++ head/sys/boot/uboot/lib/glue.c Fri Jun 19 22:24:58 2015 (r284614) > @@ -513,7 +513,7 @@ ub_env_enum(const char *last) > if (!syscall(API_ENV_ENUM, NULL, (uint32_t)last, (uint32_t)&env)) > return (NULL); > > - if (env == NULL) > + if (env == NULL || last == env) > /* no more env. variables to enumerate */ > return (NULL); > > This is only a problem with an unpatched u-boot, which has a completely bogus and un-useful implementation of API_env_enum(). That's why every one of our u-boot ports has the same patch to put in an implementation that actually works. Your change works around the worst part of the bug (the infinite loop) but leaves the major problem of the implementation only returning values initially loaded from the saved environment, not anything set by the scripts that loaded ubldr(). -- Ian
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?1434755385.1415.114.camel>