linux-kernel - Re: [RFC PATCH 08/12] e1000e: allow bad checksum

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]

Date:	Tue, 30 Sep 2008 10:38:19 +0200 (CEST)
From:	Jiri Kosina <jkosina@...e.cz>
To:	Jesse Brandeburg <jesse.brandeburg@...el.com>
cc:	linux-kernel@...r.kernel.org, linux-netdev@...r.kernel.org,
	kkeil@...e.de, agospoda@...hat.com, arjan@...ux.intel.com,
	david.graham@...el.com, bruce.w.allan@...el.com, jkosina@...e.cz,
	john.ronciak@...el.com, Thomas Gleixner <tglx@...utronix.de>,
	chris.jones@...onical.com, tim.gardner@...el.com, airlied@...il.com
Subject: Re: [RFC PATCH 08/12] e1000e: allow bad checksum

On Mon, 29 Sep 2008, Jesse Brandeburg wrote:

>  	for (i = 0;; i++) {
> -		if (e1000_validate_nvm_checksum(&adapter->hw) >= 0)
> +		if (e1000_validate_nvm_checksum(hw) >= 0) {
> +			/* copy the MAC address out of the NVM */
> +			if (e1000e_read_mac_addr(&adapter->hw))
> +				e_err("NVM Read Error reading MAC address\n");
>  			break;
> +		}
>  		if (i == 2) {
>  			e_err("The NVM Checksum Is Not Valid\n");
> -			err = -EIO;
> -			goto err_eeprom;
> +			e1000e_dump_eeprom(adapter);
> +			/*
> +			 * set MAC address to all zeroes to invalidate and
> +			 * temporary disable this device for the user. This
> +			 * blocks regular traffic while still permitting
> +			 * ethtool ioctls from reaching the hardware as well as
> +			 * allowing the user to run the interface after
> +			 * manually setting a hw addr using
> +			 * `ip link set address`
> +			 */
> +			memset(hw->mac.addr, 0, netdev->addr_len);
> +			break;
>  		}
>  	}

BTW wouldn't something like

	if (e1000_validate_nvm_checksum(hw) >= 0 ||
	    e1000_validate_nvm_checksum(hw) >= 0) {
		/* copy the MAC address out of the NVM */
		if (e1000e_read_mac_addr(&adapter->hw))
		e_err("NVM Read Error reading MAC address\n");
	} else {
		e_err("The NVM Checksum Is Not Valid\n");
		e1000e_dump_eeprom(adapter);
		/*
		 * set MAC address to all zeroes to invalidate and
		 * temporary disable this device for the user. This
		 * blocks regular traffic while still permitting
		 * ethtool ioctls from reaching the hardware as well as
		 * allowing the user to run the interface after
		 * manually setting a hw addr using
		 * `ip link set address`
		 */
		memset(hw->mac.addr, 0, netdev->addr_len);
	}

just be much more readable? Having for(;;) loop which always performs 
three iterations, and having "if" inside that distinguishes two iterations 
from each other just looks peculiar to my eyes :)

Thanks,

-- 
Jiri Kosina
SUSE Labs

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