Re: Memory corruption with r8169 across several device revisions and kernels

[David Miller <davem@davemloft.net>]

From: Oliver Freyermuth <o.freyermuth@googlemail.com>
Date: Mon, 22 Jan 2018 23:55:58 +0100

> Checking through the driver sources, I find rtnl_link_stats64 can
> not be the culprit, since it has rx_packets and only after
> tx_packets.  However, struct rtl8169_counters looks like:
>
> struct rtl8169_counters {
> 	__le64	tx_packets;
> 	__le64	rx_packets;
> 	__le64	tx_errors;
> 	__le32	rx_errors;
> 	__le16	rx_missed;
> 	__le16	align_errors;
> 	__le32	tx_one_collision;
> 	__le32	tx_multi_collision;
> 	__le64	rx_unicast;
> 	__le64	rx_broadcast;
> 	__le32	rx_multicast;
> 	__le16	tx_aborted;
> 	__le16	tx_underun;
> };
>
> This looks like it could very well match the structure found in
> memory, so something would be broken related to rtl8169_do_counters,
> in the DMA transfer.
> 
> Does this help - can I provide more info? I get the feeling this
> affects many tens of thousands of systems and just has been hidden
> due to network stats being read rarely...

Looking at how these DMA counters are handled, there appears to be a
requirement that the memory buffer is 64-byte aligned.

This is because the low bits in the counter address register are used
for various commands, for example:

	/* ResetCounterCommand */
	CounterReset	= 0x1,

	/* DumpCounterCommand */
	CounterDump	= 0x8,

Looking at the FreeBSD driver, the requirement seems to be 64-bytes of
alignment.  (see RL_DUMP_ALIGN define)

However, nothing is being done in r8169.c to enforce this alignment at
counter allocation time:

	tp->counters = dmam_alloc_coherent (&pdev->dev, sizeof(*tp->counters),
					    &tp->counters_phys_addr,

There is no alignment guaranteed by this allocation interface.  On a
lot of platforms you get PAGE_SIZE aligned buffers, but this is not
a universal thing at all.

Therefore the driver needs to allocate "size + (64 - 1)" bytes and do
the 64-byte alignment of the CPU pointer and the DMA address by hand.