netdev - for_each_netdev_feature() broken on big endian

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: <9231D502B07C5E4A8B32D5115C9F19991EE57330@IRSMSX101.ger.corp.intel.com>
Date:   Fri, 7 Sep 2018 15:10:53 +0000
From:   "Mehrtens, Hauke" <hauke.mehrtens@...el.com>
To:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>
CC:     "Doychev, Zahari" <zahari.doychev@...el.com>,
        "Langer, Thomas" <thomas.langer@...el.com>
Subject: for_each_netdev_feature() broken on big endian

Hi,

On a MIPS 32 Big endian system the netdev_sync_upper_features() function does not work correctly.
It does not disbale bit 15 (NETIF_F_LRO, 0x0000000000008000), but 47 (NETIF_F_HW_TC, 0x0000800000000000).

The for_each_netdev_feature() macro is used to go over all netdev feature flags and calls for_each_set_bit() with a u64.
This is the code:
#define for_each_netdev_feature(mask_addr, bit)	\
	for_each_set_bit(bit, (unsigned long *)mask_addr, NETDEV_FEATURE_COUNT)
https://elixir.bootlin.com/linux/v4.19-rc2/source/include/linux/netdev_features.h#L157

The for_each_set_bit() macro calls the find_first_bit() macro and works directly on the bits in the memory
organized in 32 bit values in the generic implementation, see here:
https://elixir.bootlin.com/linux/v4.19-rc2/source/lib/find_bit.c#L102

On big endian systems the u64 value is stored in a different order in memory compared to little endian systems.

When I execute this code: 
	netdev_features_t upper_disables = NETIF_F_UPPER_DISABLES;
	printk("%s:%i: addr: 0x%llx, size: %i\n", __func__, __LINE__, upper_disables, NETDEV_FEATURE_COUNT);
	ret = find_first_bit((unsigned long *)&upper_disables, NETDEV_FEATURE_COUNT);
	printk("%s:%i: addr: 0x%llx, size: %i, ret: %li\n", __func__, __LINE__, upper_disables, NETDEV_FEATURE_COUNT, ret);
I get this result:
	[   35.227140] netdev_sync_upper_features:6912: addr: 0x8000, size: 48
	[   35.233337] netdev_sync_upper_features:6914: addr: 0x8000, size: 48, ret: 47
Expected would be a ret: 15.

As I understood the documentation for for_each_set_bit() this should find the first bit in memory and not in a integer,
so for_each_netdev_feature() should not use for_each_set_bit().
I do not really know what a good fix would be, I could convert the feature flags to little endian before calling for_each_netdev_feature(), would this be ok?

Hauke