linux-kernel - RE: [PATCH net-next v3 2/5] net: phy: microchip

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] [day] [month] [year] [list]

Message-ID:
 <CO1PR11MB47714631E6DBB1AAF54A00C1E25A2@CO1PR11MB4771.namprd11.prod.outlook.com>
Date: Wed, 13 Nov 2024 11:05:51 +0000
From: <Divya.Koppera@...rochip.com>
To: <kuba@...nel.org>
CC: <andrew@...n.ch>, <Arun.Ramadoss@...rochip.com>,
	<UNGLinuxDriver@...rochip.com>, <hkallweit1@...il.com>,
	<linux@...linux.org.uk>, <davem@...emloft.net>, <edumazet@...gle.com>,
	<pabeni@...hat.com>, <netdev@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>, <richardcochran@...il.com>,
	<vadim.fedorenko@...ux.dev>
Subject: RE: [PATCH net-next v3 2/5] net: phy: microchip_ptp : Add ptp library
 for Microchip phys

Hi Jakub,

> -----Original Message-----
> From: Jakub Kicinski <kuba@...nel.org>
> Sent: Wednesday, November 13, 2024 3:50 AM
> To: Divya Koppera - I30481 <Divya.Koppera@...rochip.com>
> Cc: andrew@...n.ch; Arun Ramadoss - I17769
> <Arun.Ramadoss@...rochip.com>; UNGLinuxDriver
> <UNGLinuxDriver@...rochip.com>; hkallweit1@...il.com;
> linux@...linux.org.uk; davem@...emloft.net; edumazet@...gle.com;
> pabeni@...hat.com; netdev@...r.kernel.org; linux-kernel@...r.kernel.org;
> richardcochran@...il.com; vadim.fedorenko@...ux.dev
> Subject: Re: [PATCH net-next v3 2/5] net: phy: microchip_ptp : Add ptp library
> for Microchip phys
> 
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> content is safe
> 
> On Tue, 12 Nov 2024 19:07:21 +0530 Divya Koppera wrote:
> > +     /* Iterate over all RX timestamps and match it with the received skbs */
> > +     spin_lock_irqsave(&ptp_clock->rx_ts_lock, flags);
> > +     list_for_each_entry_safe(rx_ts, tmp, &ptp_clock->rx_ts_list, list) {
> > +             /* Check if we found the signature we were looking for. */
> > +             if (skb_sig != rx_ts->seq_id)
> > +                     continue;
> > +
> > +             match = true;
> > +             break;
> > +     }
> > +     spin_unlock_irqrestore(&ptp_clock->rx_ts_lock, flags);
> > +
> > +     if (match) {
> > +             shhwtstamps = skb_hwtstamps(skb);
> > +             shhwtstamps->hwtstamp = ktime_set(rx_ts->seconds, rx_ts->nsec);
> > +             netif_rx(skb);
> > +
> > +             list_del(&rx_ts->list);
> > +             kfree(rx_ts);
> > +     } else {
> > +             skb_queue_tail(&ptp_clock->rx_queue, skb);
> > +     }
> 
> coccicheck complains that you are using rx_ts after the loop, even though it's a
> loop iterator. Instead of using bool match make that variable a pointer, set it to
> NULL and act on it only if set. That will make the code easier for static checkers.
> 
> Coincidentally, I haven't looked closely, but you seem to have a spin lock
> protecting the list, and yet you list_del() without holding that spin lock? Sus.

Initial patch was in right way, due to some optimizations and giving less scope to spinlock, it totally went off. Will correct this next revision.

Thanks,
Divya.

> --
> pw-bot: cr