netdev - Re: [patch 1/3] net: sxgbe: sxgbe_mdio

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, 01 Apr 2014 16:23:22 -0400 (EDT)
From:	David Miller <davem@...emloft.net>
To:	dan.carpenter@...cle.com
Cc:	bh74.an@...sung.com, ks.giri@...sung.com, siva.kallam@...sung.com,
	vipul.pandya@...sung.com, netdev@...r.kernel.org,
	kernel-janitors@...r.kernel.org
Subject: Re: [patch 1/3] net: sxgbe: sxgbe_mdio_register() frees the bus

From: Dan Carpenter <dan.carpenter@...cle.com>
Date: Tue, 1 Apr 2014 16:38:44 +0300

> @@ -219,13 +219,6 @@ int sxgbe_mdio_register(struct net_device *ndev)
>  		}
>  	}
>  
> -	if (!err) {
> -		netdev_err(ndev, "PHY not found\n");
> -		mdiobus_unregister(mdio_bus);
> -		mdiobus_free(mdio_bus);
> -		goto mdiobus_err;
> -	}
> -
...
> @@ -93,9 +93,9 @@ static void sxgbe_core_set_umac_addr(void __iomem *ioaddr, unsigned char *addr,
>  {
>  	u32 high_word, low_word;
>  
> -	high_word = (addr[5] << 8) || (addr[4]);
> -	low_word = ((addr[3] << 24) || (addr[2] << 16) ||
> -		    (addr[1] << 8) || (addr[0]));
> +	high_word = (addr[5] << 8) | (addr[4]);
> +	low_word = (addr[3] << 24) | (addr[2] << 16) |
> +		   (addr[1] << 8) | (addr[0]);
>  	writel(high_word, ioaddr + SXGBE_CORE_ADD_HIGHOFFSET(reg_n));
>  	writel(low_word, ioaddr + SXGBE_CORE_ADD_LOWOFFSET(reg_n));
>  }

Nothing says "DRIVER NOT TESTED" like these two bugs.

The MDIO bus is always freed, and the MAC address is corrupted
into a boolean value before being programmed into the hardware.

Frankly, this kind of stuff is unacceptable.

I cannot see how this driver can function successfully with
these two errors, it looks simply impossible.

I'm not going to hide my feelings, this was a truly terrible driver
submission.  The amount of reviewing resources consumed during all
of these iterations was huge, and it still went in with bugs like
this.

It probably should have gone into staging.
--
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