netdev - Re: [PATCH V4 net 3/5] net: stmmac: fix dma physical address of descriptor when display ring

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: <20210222114616.4eaac47c@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
Date:   Mon, 22 Feb 2021 11:46:16 -0800
From:   Jakub Kicinski <kuba@...nel.org>
To:     Joakim Zhang <qiangqing.zhang@....com>
Cc:     "peppe.cavallaro@...com" <peppe.cavallaro@...com>,
        "alexandre.torgue@...com" <alexandre.torgue@...com>,
        "joabreu@...opsys.com" <joabreu@...opsys.com>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        dl-linux-imx <linux-imx@....com>
Subject: Re: [PATCH V4 net 3/5] net: stmmac: fix dma physical address of
 descriptor when display ring

On Sat, 20 Feb 2021 07:43:33 +0000 Joakim Zhang wrote:
> > >  	pr_info("%s descriptor ring:\n", rx ? "RX" : "TX");
> > >
> > >  	for (i = 0; i < size; i++) {
> > > -		pr_info("%03d [0x%x]: 0x%x 0x%x 0x%x 0x%x\n",
> > > -			i, (unsigned int)virt_to_phys(p),
> > > +		pr_info("%03d [0x%llx]: 0x%x 0x%x 0x%x 0x%x\n",
> > > +			i, (unsigned long long)(dma_rx_phy + i * desc_size),
> > >  			le32_to_cpu(p->des0), le32_to_cpu(p->des1),
> > >  			le32_to_cpu(p->des2), le32_to_cpu(p->des3));
> > >  		p++;  
> > 
> > Why do you pass the desc_size in? The virt memory pointer is incremented by
> > sizeof(*p) surely
> > 
> > 	dma_addr + i * sizeof(*p)  
> 
> I think we can't use sizeof(*p), as when display descriptor, only do
> " struct dma_desc *p = (struct dma_desc *)head;", but driver can pass
> "struct dma_desc", " struct dma_edesc" or " struct
> dma_extended_desc",

Looks like some of the functions you change already try to pick the
right type. Which one is problematic?

> so it's necessary to pass desc_size to compatible all cases.

But you still increment the the VMA pointer ('p' in the quote above)
but it's size, so how is that correct if the DMA addr needs a special
size increment?