linux-kernel - Re: [PATCH] drivers:bluetooth:ath3k.c Fixed sparse warning for cast to restricted _

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] [day] [month] [year] [list]

Message-Id: <0154F030-52BE-4935-8150-1A96737E7413@holtmann.org>
Date:	Wed, 26 Mar 2014 00:21:22 -0700
From:	Marcel Holtmann <marcel@...tmann.org>
To:	Surendra Patil <surendra.tux@...il.com>
Cc:	"Gustavo F. Padovan" <gustavo@...ovan.org>,
	Johan Hedberg <johan.hedberg@...il.com>,
	Joe Perches <joe@...ches.com>,
	bt <linux-bluetooth@...r.kernel.org>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] drivers:bluetooth:ath3k.c Fixed sparse warning for cast to restricted __le32

Hi Surendra,

> To fix the sparse warning "cast to restricted __le32" marked
> "rom_version"  to __le32 instead of unsigned int in "struct ath3k_version"
> and added cpu_to_le32() for the expression assigning int value to "rom_version".
> Successfully built the module without warnings and errors on x86 machine with sparse.
> 
> Signed-off-by: Surendra Patil <surendra.tux@...il.com>
> ---
> drivers/bluetooth/ath3k.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/bluetooth/ath3k.c b/drivers/bluetooth/ath3k.c
> index be571fe..5564207 100644
> --- a/drivers/bluetooth/ath3k.c
> +++ b/drivers/bluetooth/ath3k.c
> @@ -50,7 +50,7 @@
> #define ATH3K_NAME_LEN				0xFF
> 
> struct ath3k_version {
> -	unsigned int	rom_version;
> +	__le32		rom_version;
> 	unsigned int	build_version;
> 	unsigned int	ram_version;
> 	unsigned char	ref_clock;

so I think this struct should be __packed in the first place and use strictly typed variables.

Seems like all fields have an endian issue.

> @@ -375,7 +375,7 @@ static int ath3k_load_patch(struct usb_device *udev)
> 		return ret;
> 	}
> 
> -	pt_version.rom_version = *(int *)(firmware->data + firmware->size - 8);
> +	pt_version.rom_version = cpu_to_le32(*(int *)(firmware->data + firmware->size - 8));
> 	pt_version.build_version = *(int *)
> 		(firmware->data + firmware->size - 4);

I have no idea what this code is doing since that is just crazy. The variables are unsigned int and are now casted into int and not to mention that it is unaligned access. Has this code every been run on anything other than x86 at all. Does it happen to just work by pure luck.

Regards

Marcel

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