netdev - Re: [RFC net-next v1 4/5] net: macsec: introduce mdo_insert_tx

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]

Message-ID: <274e3acf-50cb-1665-447e-a049251bc67d@oss.nxp.com>
Date: Wed, 16 Aug 2023 13:19:29 +0300
From: "Radu Pirea (OSS)" <radu-nicolae.pirea@....nxp.com>
To: Simon Horman <horms@...nel.org>
Cc: andrew@...n.ch, hkallweit1@...il.com, linux@...linux.org.uk,
 davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
 pabeni@...hat.com, richardcochran@...il.com, sd@...asysnail.net,
 netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [RFC net-next v1 4/5] net: macsec: introduce mdo_insert_tx_tag

On 12.08.2023 21:07, Simon Horman wrote:
> On Fri, Aug 11, 2023 at 06:32:48PM +0300, Radu Pirea (NXP OSS) wrote:
>> @@ -4137,6 +4211,11 @@ static int macsec_newlink(struct net *net, struct net_device *dev,
>>   			if (err)
>>   				goto del_dev;
>>   		}
>> +
>> +		dev->needed_headroom -= MACSEC_NEEDED_HEADROOM;
>> +		dev->needed_headroom += ops->needed_headroom;
>> +		dev->needed_tailroom -= MACSEC_NEEDED_TAILROOM;
>> +		dev->needed_tailroom += ops->needed_tailroom;
> 
> Hi Radu,
> 
> Just above the beginning of this hunk it is assumed that ops may be NULL.
> However, here it is dereferenced unconditionally. Is this safe?

Of course it's not safe.
Thank you.

> Flagged by Smatch.
> 
>>   	}
>>   
>>   	err = register_macsec_dev(real_dev, dev);
>> diff --git a/include/net/macsec.h b/include/net/macsec.h
>> index 33dc7f2aa42e..a988249d9608 100644
>> --- a/include/net/macsec.h
>> +++ b/include/net/macsec.h
>> @@ -272,6 +272,7 @@ struct macsec_context {
>>   		struct macsec_rx_sa_stats *rx_sa_stats;
>>   		struct macsec_dev_stats  *dev_stats;
>>   	} stats;
>> +	struct sk_buff *skb;
> 
> Not strictly related to this patch,
> but it would be nice to update the kernel doc for this
> structure so that it's fields are documented.

Agreed. I will update the documentation.

-- 
Radu P.