linux-kernel - Re: [PATCH net-next v8 5/7] net: phy: nxp-c45-tja11xx: add MACsec support

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:   Wed, 8 Nov 2023 12:45:40 +0200
From:   "Radu Pirea (OSS)" <radu-nicolae.pirea@....nxp.com>
To:     Simon Horman <horms@...nel.org>
Cc:     sd@...asysnail.net, davem@...emloft.net, edumazet@...gle.com,
        kuba@...nel.org, pabeni@...hat.com, andrew@...n.ch,
        hkallweit1@...il.com, linux@...linux.org.uk,
        richardcochran@...il.com, linux-kernel@...r.kernel.org,
        netdev@...r.kernel.org, sebastian.tobuschat@....nxp.com
Subject: Re: [PATCH net-next v8 5/7] net: phy: nxp-c45-tja11xx: add MACsec
 support

On 04.11.2023 13:35, Simon Horman wrote:
> On Mon, Oct 23, 2023 at 12:43:25PM +0300, Radu Pirea (NXP OSS) wrote:
>> +void nxp_c45_handle_macsec_interrupt(struct phy_device *phydev,
>> +				     irqreturn_t *ret)
>> +{
>> +	struct nxp_c45_phy *priv = phydev->priv;
>> +	struct nxp_c45_secy *pos, *tmp;
>> +	struct nxp_c45_sa *sa;
>> +	u8 encoding_sa;
>> +	int secy_id;
>> +	u32 reg = 0;
>> +
>> +	if (!priv->macsec)
>> +		return;
>> +
>> +	do {
>> +		nxp_c45_macsec_read(phydev, MACSEC_EVR, &reg);
>> +		if (!reg)
>> +			return;
>> +
>> +		secy_id = MACSEC_REG_SIZE - ffs(reg);
>> +		list_for_each_entry_safe(pos, tmp, &priv->macsec->secy_list,
>> +					 list)
>> +			if (pos->secy_id == secy_id)
>> +				break;
>> +
>> +		encoding_sa = pos->secy->tx_sc.encoding_sa;
> 
> Hi Radu,
> 
> I'm unsure if this can happen, but my understanding is that if
> priv->macsec->secy_list is empty then pos will be uninitialised here.
> 
> Flagged by Coccinelle.
> 

True, this this should never happen. No MACsec interrupt should be 
triggered if that list is empty. The IRQ for PN wrap is enabled only 
when a secy is added and disabled when the secy is removed. I added a 
safety check anyway.

-- 
Radu P.