netdev - Re: [PATCH v2 net-next 3/7] net: dsa: sja1105: Switch to hardware operations for PTP

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: <CA+h21hoBQ=4pSCgwcYWErA7k7BQ02LMun_qZ476-bB4eY6RjjQ@mail.gmail.com>
Date:   Thu, 12 Sep 2019 11:17:11 +0100
From:   Vladimir Oltean <olteanv@...il.com>
To:     David Miller <davem@...emloft.net>
Cc:     f.fainelli@...il.com, vivien.didelot@...il.com, andrew@...n.ch,
        richardcochran@...il.com, netdev@...r.kernel.org
Subject: Re: [PATCH v2 net-next 3/7] net: dsa: sja1105: Switch to hardware
 operations for PTP

Hi Dave,

On 12/09/2019, David Miller <davem@...emloft.net> wrote:
> From: Vladimir Oltean <olteanv@...il.com>
> Date: Tue, 10 Sep 2019 04:34:57 +0300
>
>>  static int sja1105_ptp_adjfine(struct ptp_clock_info *ptp, long
>> scaled_ppm)
>>  {
>>  	struct sja1105_private *priv = ptp_to_sja1105(ptp);
>> +	const struct sja1105_regs *regs = priv->info->regs;
>>  	s64 clkrate;
>> +	int rc;
>  ..
>> -static int sja1105_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta)
>> -{
>> -	struct sja1105_private *priv = ptp_to_sja1105(ptp);
>> +	rc = sja1105_spi_send_int(priv, SPI_WRITE, regs->ptpclkrate,
>> +				  &clkrate, 4);
>
> You're sending an arbitrary 4 bytes of a 64-bit value.  This works on little
> endian
> but will not on big endian.
>
> Please properly copy this clkrate into a "u32" variable and pass that into
> sja1105_spi_send_int().
>
> It also seems to suggest that you want to use abs() to perform that weird
> centering around 1 << 31 calculation.
>
> Thank you.
>

It looks 'wrong' but it isn't. The driver uses the 'packing' framework
(lib/packing.c) which is endian-agnostic (converts between CPU and
peripheral endianness) and operates on u64 as the CPU word size. On
the contrary, u32 would not work with the 'packing' API in its current
form, but I don't see yet any reasons to extend it (packing64,
packing32 etc).

Thanks,
-Vladimir