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

[Jonathan Liu]

On 31 July 2017 at 17:58, Jonathan Liu <net147 at gmail.com> wrote:
> Hi Ed,
>
> On 31 July 2017 at 17:28, Ed Bartosh <ed.bartosh at linux.intel.com> wrote:
>> 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 at 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 at 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?
>>
>
> How about random.SystemRandom().randrange(1, 0xffffffff) ?
>

random.SystemRandom().randint(1, 0xffffffff) actually

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