linux-kernel - Re: [PATCH] Staging: TIDSPBRIDGE: Remove UUID helper

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <529BC5B0.8010202@gmail.com>
Date:	Mon, 02 Dec 2013 01:26:40 +0200
From:	Ivajlo Dimitrov <ivo.g.dimitrov.75@...il.com>
To:	Joe Perches <joe@...ches.com>,
	Ivaylo DImitrov <ivo.g.dimitrov.75@...il.com>
CC:	omar.ramirez@...itl.com, gregkh@...uxfoundation.org,
	pali.rohar@...il.com, pavel@....cz, linux-kernel@...r.kernel.org,
	devel@...verdev.osuosl.org, Ivaylo Dimitrov <freemangordon@....bg>
Subject: Re: [PATCH] Staging: TIDSPBRIDGE: Remove UUID helper


On 01.12.2013 21:06, Joe Perches wrote:
> On Sun, 2013-12-01 at 19:07 +0200, Ivaylo DImitrov wrote:
>> From: Ivaylo Dimitrov <freemangordon@....bg>
>>
>> Custom uuid helper function is needed only in rmgr/dbdcd.c and doesn't
>> need to be exported. It can also be made way simpler by using sscanf.
> []
>> diff --git a/drivers/staging/tidspbridge/rmgr/dbdcd.c b/drivers/staging/tidspbridge/rmgr/dbdcd.c
> []
>> @@ -74,6 +74,40 @@ static int get_dep_lib_info(struct dcd_manager *hdcd_mgr,
>>   				   enum nldr_phase phase);
>>   
>>   /*
>> + *  ======== dcd_uuid_from_string ========
>> + *  Purpose:
>> + *      Converts an ANSI string to a dsp_uuid.
>> + *  Parameters:
>> + *      sz_uuid:    Pointer to a string that represents a dsp_uuid object.
>> + *      uuid_obj:      Pointer to a dsp_uuid object.
>> + *  Returns:
>> + *  Requires:
>> + *      uuid_obj & sz_uuid are non-NULL values.
>> + *  Ensures:
>> + *  Details:
>> + *      We assume the string representation of a UUID has the following format:
>> + *      "12345678_1234_1234_1234_123456789abc".
>> + */
>> +static void dcd_uuid_from_string(char *sz_uuid, struct dsp_uuid *uuid_obj)
>> +{
>> +	char c;
>> +	u64 t;
>> +
>> +	/*
>> +	 * sscanf implementation cannot deal with hh format modifier
>> +	 * if the converted value doesn't fit in u32. So, convert the
>> +	 * last six bytes to u64 and memcpy what is needed
>> +	 */
>> +	sscanf(sz_uuid, "%8x%c%4hx%c%4hx%c%2hhx%2hhx%c%llx",
>> +	       &uuid_obj->data1, &c, &uuid_obj->data2, &c,
>> +	       &uuid_obj->data3, &c, &uuid_obj->data4,
>> +	       &uuid_obj->data5, &c, &t);
>> +
>> +	t = cpu_to_be64(t);
>> +	memcpy(&uuid_obj->data6[0], ((char*)&t) + 2, 6);
>> +}
> It'd probably be better to return true or false on
> successful conversion, use a temporary struct dsp_uuid,
> check the sscanf return is 10 and only copy to uuid_obj
> on success.
>
> Something like:
>
> static bool dcd_uuid_from_string(char *sz_uuid, struct dsp_uuid *uuid_obj)
> {
> 	char c;
> 	u64 t;
> 	struct dsp_uuid tmp;
>
> 	/*
> 	 * sscanf implementation cannot deal with hh format modifier
> 	 * if the converted value doesn't fit in u32. So, convert the
> 	 * last six bytes to u64 and memcpy what is needed
> 	 */
> 	if (sscanf(sz_uuid, "%8x%c%4hx%c%4hx%c%2hhx%2hhx%c%llx",
> 		   &tmp.data1, &c, &tmp.data2, &c,
> 		   &tmp.data3, &c, &tmp.data4,
> 		   &tmp.data5, &c, &t) != 10)
> 		return false;
>
> 	t = cpu_to_be64(t);
> 	memcpy(&tmp.data6[0], ((char*)&t) + 2, 6);
> 	*uuid_obj =  tmp;
>
> 	return true;
> }
>
>
If there is to be a return value from that function, it is better to be 
int (-EINVAL/0), IMO. And I am not sure that makes much of a sense, as 
(afaik) uuids are read from baseimage.dof and codec nodes, if those are 
broken I suspect wrong uuids will be our least problem.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/