netdev - Re: [PATCH] net: fec: ptp: avoid register access when ipg clock is disabled

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: <20140721164004.GA13039@localhost.localdomain>
Date:	Mon, 21 Jul 2014 18:40:04 +0200
From:	Richard Cochran <richardcochran@...il.com>
To:	"fugang.duan@...escale.com" <fugang.duan@...escale.com>
Cc:	"shawn.guo@...aro.org" <shawn.guo@...aro.org>,
	"Frank.Li@...escale.com" <Frank.Li@...escale.com>,
	"davem@...emloft.net" <davem@...emloft.net>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH] net: fec: ptp: avoid register access when ipg clock is
 disabled

On Mon, Jul 21, 2014 at 10:46:31AM +0000, fugang.duan@...escale.com wrote:

> >> +	if (fep->bufdesc_ex && (fep->hwts_tx_en || fep->hwts_rx_en))
> >> +		fec_ptp_stop(ndev);
> >
> >So if user space turns time stamping off, then you keep running the timer.
> >That is wrong, isn't it?
> >
> If user close the ethx interface, or do suspend operation, fec driver will
> Close all clocks to save power, we also need to stop ptp, otherwise ptp timer
> Keep access register that cause system hang.
> 
> I don't think this is a good method, maybe I need some time to struct better solution.

What?

Look at the code that you wrote. You have:

	IF   x AND (fep->hwts_tx_en || fep->hwts_rx_en)
	THEN fec_ptp_stop;

So if the user clears 'hwts_tx_en' and 'hwts_rx_en' using the ioctl,
you don't stop the timer. But in order to fix the bug, you should stop
the timer in any case.

> >Why not use a work queue for this?
>
> There need one period timer to sync hw timer to avoid 32 bit hw counter overflow.

So why not use schedule_delayed_work?

Thanks,
Richard
--
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