From: "Allen Hubbe" <Allen.Hubbe@emc.com>
To: dave.jiang@intel.com
Cc: linux-ntb@googlegroups.com
Subject: RE: NTB: Use unique DMA channels for TX and RX
Date: Wed, 16 Sep 2015 10:41:50 -0400 [thread overview]
Message-ID: <005701d0f08d$d2c58780$78509680$@emc.com> (raw)
In-Reply-To: <20150916141253.GA7674@mwanda>
From: linux-ntb@googlegroups.com On Behalf Of Dan Carpenter
> Subject: re: NTB: Use unique DMA channels for TX and RX
>
> drivers/ntb/ntb_transport.c:2008 ntb_transport_max_size()
> error: we previously assumed 'qp->tx_dma_chan' could be null (see
> line 2005)
>
> drivers/ntb/ntb_transport.c
> 2004
> 2005 if (!qp->tx_dma_chan && !qp->rx_dma_chan)
I seem to recall commenting on this in patch v1:
>> - if (!qp->dma_chan)
>> + if (!qp->tx_dma_chan && !qp->rx_dma_chan)
>
> Unusual to see "if (not && not)" in code, but it looks correct in this case.
>
> if (no dma at all)
> return without any alignment restriction;
>
> Is that right?
Maybe I was wrong to say "looks correct" because this static checker warning sure looks real.
>
> Patch adds new checks for NULL. Was || intended here perhaps?
>
> 2006 return qp->tx_max_frame - sizeof(struct
> ntb_payload_header);
> 2007
> 2008 copy_align = max(qp->tx_dma_chan->device->copy_align,
> 2009 qp->rx_dma_chan->device->copy_align);
I think this is the part that actually needs to be fixed. Maybe this whole function should be:
copy_align = max(dma_copy_align(qp->tx_dma_chan),
dma_copy_align(qp->rx_dma_chan));
where dma_copy_align is:
if (!chan)
return 1;
return chan->device->copy_align;
>
> Patch adds unchecked dereferences.
>
> 2010
>
prev parent reply other threads:[~2015-09-16 14:42 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-09-16 14:12 NTB: Use unique DMA channels for TX and RX Dan Carpenter
2015-09-16 14:41 ` Allen Hubbe [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='005701d0f08d$d2c58780$78509680$@emc.com' \
--to=allen.hubbe@emc.com \
--cc=dave.jiang@intel.com \
--cc=linux-ntb@googlegroups.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.