linux-kernel - RE: [PATCHv2 5/5] tools: ffs-test: add compatibility code for old kernels

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: <006601cf83bc$66197b30$324c7190$%opasiak@samsung.com>
Date:	Mon, 09 Jun 2014 10:25:39 +0200
From:	Krzysztof Opasiak <k.opasiak@...sung.com>
To:	'Michal Nazarewicz' <mina86@...a86.com>,
	'Felipe Balbi' <balbi@...com>,
	Krzysztof Opasiak <k.opasiak@...sung.com>
Cc:	linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: RE: [PATCHv2 5/5] tools: ffs-test: add compatibility code for old
 kernels

Hi Michal,

> -----Original Message-----
> From: Michal Nazarewicz [mailto:mina86@...a86.com]
> Sent: Friday, June 06, 2014 11:13 AM
> To: Felipe Balbi; Krzysztof Opasiak
> Cc: linux-usb@...r.kernel.org; linux-kernel@...r.kernel.org; Michal
> Nazarewicz
> Subject: [PATCHv2 5/5] tools: ffs-test: add compatibility code for
> old kernels

(... snip ...)

> +static size_t descs_to_legacy(void **legacy, const void
> *descriptors) {
> +	__u32 length, flags, fs_count = 0, hs_count = 0, count;
> +	const unsigned char *descs = descriptors, *usb_descs;
> +	unsigned char *out;
> +
> +	if (get_unaligned_le32(descs) !=
> FUNCTIONFS_DESCRIPTORS_MAGIC_V2)
> +		return 0;
> +
> +	length = get_unaligned_le32(descs + 4);
> +	if (length < 8)
> +		return 0;
> +	descs += 8;
> +	length -= 8;
> +
> +#define GET_LE32(ret) do {				\
> +		if (length < 4)				\
> +			return 0;			\
> +		ret = get_unaligned_le32(descs);	\
> +		descs += 4;				\
> +		length -= 4;				\
> +	} while (0)
> +
> +	GET_LE32(flags);
> +	if (flags & FUNCTIONFS_HAS_FS_DESC)
> +		GET_LE32(fs_count);
> +	if (flags & FUNCTIONFS_HAS_HS_DESC)
> +		GET_LE32(hs_count);
> +	if (!fs_count && !hs_count)
> +		return 0;
> +	if (flags & FUNCTIONFS_HAS_SS_DESC)
> +		GET_LE32(count);  /* The value is ignored later on
> anyway. */
> +	if (flags)
> +		return 0;

As far as I understand you are taking the flags from descriptor and then test them with possible ffs speed flags, after getting flags your are not assigning anything to this variable so in this place it will be != 0  unless none of the flags has been provided. I don't think this is intended behavior.

> +
> +#undef GET_LE32
> +
> +	usb_descs = descs;
> +	for (count = fs_count + hs_count; count; --count) {
> +		if (length < *descs)
> +			return 0;
> +		length -= *descs;
> +		descs += *descs;
> +	}
> +
> +	length = 16 + (descs - usb_descs);
> +	out = malloc(length);
> +	put_unaligned_le32(FUNCTIONFS_DESCRIPTORS_MAGIC, out);
> +	put_unaligned_le32(length, out + 4);
> +	put_unaligned_le32(fs_count, out + 8);
> +	put_unaligned_le32(hs_count, out + 12);
> +	memcpy(out + 16, usb_descs, length - 16);
> +	*legacy = out;
> +	return length;
> +}
> +

As this is an example which will be copy-paste by many users maybe you should you struct usb_functionfs_descs_head and struct usb_functionfs_descs_head_v2 instead of direct operations using hard-coded offsets to make this function more readable?

--
BR's
Krzysztof Opasiak
Samsung R&D Institute Poland
Samsung Electronics
k.opasiak@...sung.com




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