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

[Jonathan Liu]

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) ?

>> >>          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