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
| ||
|
Date: Tue, 6 Apr 2021 21:55:40 -0700 From: Shannon Nelson <snelson@...sando.io> To: Richard Cochran <richardcochran@...il.com> Cc: netdev@...r.kernel.org, davem@...emloft.net, kuba@...nel.org, drivers@...sando.io, Allen Hubbe <allenbh@...sando.io> Subject: Re: [PATCH net-next 05/12] ionic: add hw timestamp support files On 4/6/21 5:27 PM, Richard Cochran wrote: > On Tue, Apr 06, 2021 at 04:18:00PM -0700, Shannon Nelson wrote: >> On 4/5/21 11:17 AM, Richard Cochran wrote: >>> On Mon, Apr 05, 2021 at 09:16:39AM -0700, Shannon Nelson wrote: >>>> On 4/4/21 4:05 PM, Richard Cochran wrote: >>>>> This check is unneeded, because the ioctl layer never passes NULL here. >>>> Yes, the ioctl layer never calls this with NULL, but we call it from within >>>> the driver when we spin operations back up after a FW reset. >>> So why not avoid the special case and pass a proper request? >> We do this because our firmware reset path is a special case that we have to >> handle, and we do so by replaying the previous configuration request. >> Passing the NULL request gives the code the ability to watch for this case >> while keeping the special case handling simple: the code that drives the >> replay logic doesn't need to know the hwstamp details, it just needs to >> signal the replay and let the hwstamp code keep track of its own data and >> request history. >> >> I can update the comment to make that replay case more obvious. > No, please, I am asking you to provide a hwtstamp_config from your > driver. What is so hard about that? > What I think you are asking is that we not extend the current assumption that *ifr will always be a useful pointer, a perfectly reasonable request. Our ioctl() handler follows this as expected. It is our internal usage that might look like an extension. We'd like to keep the replay related code and data to a minimum, and this current implementation is a simple way to do so, keeping the state and config info within the ptp side. I suppose one alternative is that we pull the copy_from_user() bits into ionic_ioctl() and hand a hwtstamp_config struct pointer to ionic_lif_hwstamp_set(). In our reset case we can either hand a NULL pointer to ionic_lif_hwstamp_set() or add a reset parameter to the call where ionic_lif_hwstamp_set() simply won't inspect the hwtstamp_config pointer. Another alternative would be to split the top layer of ionic_lif_hwstamp_set() into two different functions, one called by the ioctl handler with an *ifr, the other called by the fw reset logic without an *ifr, that both call into the remaining with a hwtstamp_config pointer. This seems somewhat similar to the ixgbe approach. Either way, in the end it seems like extra code to get to the same result. sln
Powered by blists - more mailing lists