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
| ||
|
Message-ID: <49028e9cdf59afe3d4184b9eee36f0d2406c8bdb.camel@linux.intel.com> Date: Wed, 23 Jan 2019 14:47:06 -0800 From: Dalon L Westergreen <dalon.westergreen@...ux.intel.com> To: thor.thayer@...ux.intel.com, Atsushi Nemoto <atsushi.nemoto@...d.co.jp>, netdev@...r.kernel.org Cc: Tomonori Sakita <tomonori.sakita@...d.co.jp> Subject: Re: [PATCH] net: altera_tse: fix msgdma_tx_completion on non-zero fill_level case On Tue, 2019-01-22 at 11:20 -0600, Thor Thayer wrote: > On 1/21/19 2:29 AM, Atsushi Nemoto wrote: > > From: Tomonori Sakita <tomonori.sakita@...d.co.jp> > > > > If fill_level was not zero and status was not BUSY, > > result of "tx_prod - tx_cons - inuse" might be zero. > > Subtracting 1 unconditionally results invalid negative return value > > on this case. > > The subtraction is not needed if fill_level was not zero. > > > > Signed-off-by: Tomonori Sakita <tomonori.sakita@...d.co.jp> > > Signed-off-by: Atsushi Nemoto <atsushi.nemoto@...d.co.jp> > > --- > > drivers/net/ethernet/altera/altera_msgdma.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/net/ethernet/altera/altera_msgdma.c > > b/drivers/net/ethernet/altera/altera_msgdma.c > > index 0fb986b..3df73d3 100644 > > --- a/drivers/net/ethernet/altera/altera_msgdma.c > > +++ b/drivers/net/ethernet/altera/altera_msgdma.c > > @@ -145,7 +145,7 @@ u32 msgdma_tx_completions(struct altera_tse_private > > *priv) > > & 0xffff; > > > > if (inuse) { /* Tx FIFO is not empty */ > > - ready = priv->tx_prod - priv->tx_cons - inuse - 1; > > + ready = priv->tx_prod - priv->tx_cons - inuse; dont think my last email went through.. I am not sure about this. This register indicates the number of entries still to be processed by the dma. the -1 is intended to represent the decriptor currently being processed. If ready is priv->tx_prod - priv->tx_cons - inuse couldn't you end up processing 1 too many packets? IE: ready is 1 greater then the actual completed packets? I do agree that we should not be returning a negative value, but i dont think i agree removing the -1 is the answer. perhaps just check that ready is greater than 0? --dalon > > } else { > > /* Check for buffered last packet */ > > status = csrrd32(priv->tx_dma_csr, msgdma_csroffs(status)); > > > > I'm adding Dalon who has done a lot of work on msgdma. I'd like to get > his comments. > > Thanks, > > Thor >
Powered by blists - more mailing lists