linux-kernel - RE: [PATCH] Make efi-pstore return a unique id

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 1 Nov 2013 19:22:12 +0000
From:	Seiji Aguchi <seiji.aguchi@....com>
To:	Tony Luck <tony.luck@...il.com>,
	Richard Weinberger <richard.weinberger@...il.com>
CC:	Madper Xie <cxie@...hat.com>,
	"matt.fleming@...el.com" <matt.fleming@...el.com>,
	linux-kernel <linux-kernel@...r.kernel.org>,
	"Linux EFI" <linux-efi@...r.kernel.org>,
	谢成骏 <bbboson@...il.com>
Subject: RE: [PATCH] Make efi-pstore return a unique id

> >> +{
> >> +       char id_str[64];
> >> +       u64 id = 0;
> >> +
> >> +       sprintf(id_str, "%lu%u%d", timestamp, part, count);
> >> +       if (kstrtoull(id_str, 10, &id))
> >> +               pr_warn("efi-pstore: failed to generate id\n");
> >> +       return id;
> >> +}
> >
> > This is just odd. You make a string from three ints and then a parse
> > it to a int again.
> 
> Agreed.  I liked your ((timestamp * 100 + part) * 100 + count function much
> more than this.

I was worried that the part and count could be more than 100.
If it happens, the id may not be unique...

But, currently, size of nvram storage is limited, so it is a corner case.
I respect your opinion.

> @@ -125,9 +137,11 @@ static int efi_pstore_write(enum pstore_type_id type,
>  	efi_char16_t efi_name[DUMP_NAME_LEN];
>  	efi_guid_t vendor = LINUX_EFI_CRASH_GUID;
>  	int i, ret = 0;
> +	unsigned long timestamp;
> 
> +	timestamp = get_seconds();
>  	sprintf(name, "dump-type%u-%u-%d-%lu-%c", type, part, count,
> -		get_seconds(), compressed ? 'C' : 'D');
> +		timestamp, compressed ? 'C' : 'D');
> 

I don't think you need to change efi_pstore_write().
It is just add a local timestamp variable.

Seiji

>  	for (i = 0; i < DUMP_NAME_LEN; i++)
>  		efi_name[i] = name[i];
> --
> 1.8.4.2