[U-Boot] [PATCH v6 3/3] GPT: provide commands to selectively rename partitions

[Lothar Waßmann]

Hi,

On Sun, 25 Jun 2017 14:54:56 -0700 Alison Chaiken wrote:
> On Sun, Jun 18, 2017 at 4:03 AM, Wolfgang Denk <wd at denx.de> wrote:
> 
> > Dear Alison,
> >
> > In message <CAOuSAjdHerD7iWSwv5HQmx07nALRHschnH5=XToNEZDqA9JsvQ at mail.
> > gmail.com> you wrote:
> > >
> > > The idea behind the 'swap' mode is that a storage device can have two
> > sets
> > > of partitions, one set all named 'primary' and one set all named
> > 'backup'.
> > >   The software updater in userspace can then simply rename the partitions
> > > with sgdisk in order to pick the new image.   The swap mode changes the
> > > whole set of labels at once, so there's little chance of being
> > interrupted.
> >
> > It's still a sequential, non-atomic operation, and "little chance"
> > is exactly the places where Murphy likes to hit you.
> >
> > > One additional note: the last version I posted worked fine for the
> > sandbox,
> > > but wouldn't link for an ARM target with the Linaro toolchain, as the
> > > linker couldn't find atoi().   I guess the libc for the x86 compiler
> > > includes it.   To test on ARM, I copied in simple_atoi() from
> > > lib/vsprintf.c, but assuredly that is an ugly solution.    Does anyone
> > have
> > > a better idea to solve this problem?
> >
> > U-Boot should always be self-contained and not link regular library
> > code from the tool chain.
> >
> > Best regards,
> >
> > Wolfgang Denk
> >
> 
> I'm about to submit a new version of the patches that adopts Wolfgang's and
> Tom's suggestions about replacing atoi().
> 
> Regarding the atomicity of 'gpt swap, the point is that 'gpt swap' first
> modifies the names in an in-memory
> data structure, and then uses the existing 'gpt write' functionality to
> change the actual partition table stored on the device.  Thus,
> interruption of the new command is low-risk, as interruption of the
> modification of the new data structure has no persistent effect, and
> the risk associated with 'gpt write' is the same as always.
> 
> By the way, in the course of testing an earlier version of this patch
> series, I noticed that 'gpt write' and 'gpt verify' segv if presented with
> a non-null-terminated partitions string.  It's the strlen function in lib
> that actually generates an error. I haven't yet quite figured out what the
> best solution to the problem is: should strlen() itself be modified, or is
> it enough to test in gpt.c?   The right solution is not to present the
> commands with poorly formed strings, but it's easy to do so.
> 
You can use strnlen() if you know the maximum allowed length of the
string.

NB: A quick glance at set_gpt_info() revealed this potential crash
cause:
|	str = strdup(str_part);
|
|	/* extract disk guid */
|	s = str;
|	val = extract_val(str, "uuid_disk");
strdup() may fail (especially if the input string is not zero
terminated) and return a NULL pointer which then will happily be used
by extract_val().



Lothar Waßmann