Re: [PATCH v3] wic: ensure generated disk system identifier is non-zero

[Ed Bartosh <ed.bartosh@linux.intel.com>]

On Sun, Jul 30, 2017 at 08:37:26PM +1000, Jonathan Liu wrote:
> Hi Ed,
> 
> On 30 July 2017 at 20:02, Ed Bartosh <ed.bartosh@linux.intel.com> wrote:
> > On Sat, Jul 29, 2017 at 12:45:27AM +1000, Jonathan Liu wrote:
> >> Zero may be interpreted as no MBR signature present and another
> >> partitioning program might install a new MBR signature.
> >>
> >> Signed-off-by: Jonathan Liu <net147@gmail.com>
> >> ---
> >>  scripts/lib/wic/plugins/imager/direct.py | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/scripts/lib/wic/plugins/imager/direct.py b/scripts/lib/wic/plugins/imager/direct.py
> >> index f20d8433f1..fe9b688ab0 100644
> >> --- a/scripts/lib/wic/plugins/imager/direct.py
> >> +++ b/scripts/lib/wic/plugins/imager/direct.py
> >> @@ -297,7 +297,7 @@ class PartitionedImage():
> >>                            # all partitions (in bytes)
> >>          self.ptable_format = ptable_format  # Partition table format
> >>          # Disk system identifier
> >> -        self.identifier = int.from_bytes(os.urandom(4), 'little')
> >> +        self.identifier = int.from_bytes(os.urandom(4), 'little') or 0xffffffff
> >>
> >
> > Can we generate random identifier by calling os.urandom again if
> > self.identifier is zero? Something like this, may be?
> >
> > while True:
> >     self.identifier = int.from_bytes(os.urandom(4), 'little')
> >     if self.identifier:
> >         break
> >
> > Assigning constant 0xffffffff can potentially create nonunique identifiers.
> >
>
> There is no guarantee that urandom will create unique identifiers.
We can't have a guarantee with urandom, true. My point was that if you
need random value it's better to call urandom again than using a constant value.

> Infinite loop needs a timeout and error in the case it is unable to
> generate a suitable identifier.
I'm not sure it's needed, but it's not a big deal to add this. I really
doubt os.urandom can generate long enough sequence of zeros.

> It is only 0xffffffff if it is 0. That
> means 0xffffffff is twice as likely to occur (2 in 4294967296 instead
> of 1 in 4294967296) so there is tiny bias. Note that this is how it is
> implemented for the DISK_SIGNATURE variable in OE and in GNU Parted as
> well.
Does this mean we shouldn't do better?

> >>          self.partitions = partitions
> >>          self.partimages = []
> >> --
> >> 2.13.2
> >>
> >> --
> >> _______________________________________________
> >> Openembedded-core mailing list
> >> Openembedded-core@lists.openembedded.org
> >> http://lists.openembedded.org/mailman/listinfo/openembedded-core
>
> Regards,
> Jonathan

--
Regards,
Ed