netdev - Re: ath10k: mac80211 driver for Qualcomm Atheros 802.11ac CQA98xx devices

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

Message-ID: <alpine.DEB.2.02.1307101859360.29717@ayla.of.borg>
Date:	Wed, 10 Jul 2013 19:12:28 +0200 (CEST)
From:	Geert Uytterhoeven <geert@...ux-m68k.org>
To:	Kalle Valo <kvalo@....qualcomm.com>,
	"John W. Linville" <linville@...driver.com>
cc:	linux-wireless@...r.kernel.org, netdev@...r.kernel.org,
	Linux Kernel Development <linux-kernel@...r.kernel.org>
Subject: Re: ath10k: mac80211 driver for Qualcomm Atheros 802.11ac CQA98xx
 devices

On Wed, 10 Jul 2013, Linux Kernel Mailing List wrote:
> --- /dev/null
> +++ b/drivers/net/wireless/ath/ath10k/hw.h

> +#define SUPPORTED_FW_MAJOR	1
> +#define SUPPORTED_FW_MINOR	0
> +#define SUPPORTED_FW_RELEASE	0
> +#define SUPPORTED_FW_BUILD	629

> +static int ath10k_check_fw_version(struct ath10k *ar)
> +{
> +	char version[32];
> +
> +	if (ar->fw_version_major >= SUPPORTED_FW_MAJOR &&
> +	    ar->fw_version_minor >= SUPPORTED_FW_MINOR &&
> +	    ar->fw_version_release >= SUPPORTED_FW_RELEASE &&
> +	    ar->fw_version_build >= SUPPORTED_FW_BUILD)
> +		return 0;

My attention got triggered by:

drivers/net/wireless/ath/ath10k/core.c: In function ‘ath10k_check_fw_version’:
drivers/net/wireless/ath/ath10k/core.c:79: warning: comparison is always true due to limited range of data type

as an u16 is always larger or equal than zero. Not much you can do to
silence that warning, though.

However, I don't think the version check is correct.
Shouldn't it stop checking later fields if an exact match is found in an
earlier field?

I.e.

	if (ar->fw_version_major > SUPPORTED_FW_MAJOR ||
	    (ar->fw_version_major == SUPPORTED_FW_MAJOR &&
	     ar->fw_version_minor > SUPPORTED_FW_MINOR) ||
	     ...) { ... }

Currently e.g. (major, minor) = (3, 0) is considered older than (2, 1).

Or perhaps minor is never reset to zero when major is increased? In that
case, the check is correct, but IMHO it's a bit silly to split the version
number in seperate fields.

> +	snprintf(version, sizeof(version), "%u.%u.%u.%u",
> +		 SUPPORTED_FW_MAJOR, SUPPORTED_FW_MINOR,
> +		 SUPPORTED_FW_RELEASE, SUPPORTED_FW_BUILD);
> +
> +	ath10k_warn("WARNING: Firmware version %s is not officially supported.\n",
> +		    ar->hw->wiphy->fw_version);
> +	ath10k_warn("Please upgrade to version %s (or newer)\n", version);
> +
> +	return 0;
> +}

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html