linux-kernel - Re: [PATCH 1/7] net: ethernet: ti: cpdma: am437x: allow descs to be plased in ddr

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: <20161203.153419.482900037601991931.davem@davemloft.net>
Date:   Sat, 03 Dec 2016 15:34:19 -0500 (EST)
From:   David Miller <davem@...emloft.net>
To:     grygorii.strashko@...com
Cc:     netdev@...r.kernel.org, mugunthanvnm@...com, nsekhar@...com,
        linux-kernel@...r.kernel.org, linux-omap@...r.kernel.org,
        ivan.khoronzhuk@...aro.org
Subject: Re: [PATCH 1/7] net: ethernet: ti: cpdma: am437x: allow descs to
 be plased in ddr

From: Grygorii Strashko <grygorii.strashko@...com>
Date: Thu, 1 Dec 2016 17:34:26 -0600

> @@ -167,10 +167,10 @@ static struct cpdma_control_info controls[] = {
>  
>  /* various accessors */
>  #define dma_reg_read(ctlr, ofs)		__raw_readl((ctlr)->dmaregs + (ofs))
> -#define chan_read(chan, fld)		__raw_readl((chan)->fld)
> +#define chan_read(chan, fld)		readl((chan)->fld)
>  #define desc_read(desc, fld)		__raw_readl(&(desc)->fld)
>  #define dma_reg_write(ctlr, ofs, v)	__raw_writel(v, (ctlr)->dmaregs + (ofs))
> -#define chan_write(chan, fld, v)	__raw_writel(v, (chan)->fld)
> +#define chan_write(chan, fld, v)	writel(v, (chan)->fld)
>  #define desc_write(desc, fld, v)	__raw_writel((u32)(v), &(desc)->fld)

Unless you want to keep running into subtle errors all over the
place wrt. register vs. memory write ordering, I strong suggest
you use strongly ordered readl/writel for all register accesses.

I see no tangible, worthwhile, advantage to using these relaxed
ordering primitives.  The only result is potential bugs.

People who use the relaxed ordering primitives properly are only
doing so in extremely carefully coded sequences where a series
of writes has no dependency on main memory operations and is
explicitly completed with a barrier operation such as a read
back of a register in the same device.

That's not at all what is going on here, instead the driver is wildly
using relaxed ordered register accesses for basically everything.
This is extremely unwise and it's why you ran into this bug in the
first place.

Therefore, I absolutely require that you fix this by eliminating
any and all usese of relaxed ordering I/O accessors in this driver.

Thank you.