From: Jon Mason <jdmason@kudzu.us>
To: Logan Gunthorpe <logang@deltatee.com>
Cc: Takashi Iwai <tiwai@suse.de>, Dave Jiang <dave.jiang@intel.com>,
Allen Hubbe <allenbh@gmail.com>,
linux-ntb@googlegroups.com
Subject: Re: [PATCH] NTB: ntb_transport: Use scnprintf() for avoiding potential buffer overflow
Date: Fri, 13 Mar 2020 09:19:29 -0400 [thread overview]
Message-ID: <20200313131928.GC13046@kudzu.us> (raw)
In-Reply-To: <03f25a1e-c72f-8fd8-d23e-f0da5e8d39b4@deltatee.com>
On Wed, Mar 11, 2020 at 11:08:11AM -0600, Logan Gunthorpe wrote:
>
>
> On 2020-03-11 2:49 a.m., Takashi Iwai wrote:
> > Since snprintf() returns the would-be-output size instead of the
> > actual output size, the succeeding calls may go beyond the given
> > buffer limit. Fix it by replacing with scnprintf().
> >
> > Signed-off-by: Takashi Iwai <tiwai@suse.de>
>
> Makes sense. Looks good to me!
>
> Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
>
> Thanks!
Thanks for the patch (and review). Since this is a fix, I had to add
some "Fixes:" tags to the commit message. Since this is touching a
lot of lines added by a number of patches, it's a bit ugly. But, here
is what I added.
Fixes: fce8a7bb5b4b (PCI-Express Non-Transparent Bridge Support)
Fixes: 282a2feeb9bf (NTB: Use DMA Engine to Transmit and Receive)
Fixes: a754a8fcaf38 (NTB: allocate number transport entries depending on size of ring size)
Fixes: d98ef99e378b (NTB: Clean up QP stats info)
Fixes: e74bfeedad08 (NTB: Add flow control to the ntb_netdev)
Fixes: 569410ca756c (NTB: Use unique DMA channels for TX and RX)
I pulled it in to my ntb branch with the above change.
Thanks,
Jon
>
> > ---
> > drivers/ntb/ntb_transport.c | 58 ++++++++++++++++++++++-----------------------
> > 1 file changed, 29 insertions(+), 29 deletions(-)
> >
> > diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
> > index 00a5d5764993..e6d1f5b298f3 100644
> > --- a/drivers/ntb/ntb_transport.c
> > +++ b/drivers/ntb/ntb_transport.c
> > @@ -481,70 +481,70 @@ static ssize_t debugfs_read(struct file *filp, char __user *ubuf, size_t count,
> > return -ENOMEM;
> >
> > out_offset = 0;
> > - out_offset += snprintf(buf + out_offset, out_count - out_offset,
> > + out_offset += scnprintf(buf + out_offset, out_count - out_offset,
> > "\nNTB QP stats:\n\n");
> > - out_offset += snprintf(buf + out_offset, out_count - out_offset,
> > + out_offset += scnprintf(buf + out_offset, out_count - out_offset,
> > "rx_bytes - \t%llu\n", qp->rx_bytes);
> > - out_offset += snprintf(buf + out_offset, out_count - out_offset,
> > + out_offset += scnprintf(buf + out_offset, out_count - out_offset,
> > "rx_pkts - \t%llu\n", qp->rx_pkts);
> > - out_offset += snprintf(buf + out_offset, out_count - out_offset,
> > + out_offset += scnprintf(buf + out_offset, out_count - out_offset,
> > "rx_memcpy - \t%llu\n", qp->rx_memcpy);
> > - out_offset += snprintf(buf + out_offset, out_count - out_offset,
> > + out_offset += scnprintf(buf + out_offset, out_count - out_offset,
> > "rx_async - \t%llu\n", qp->rx_async);
> > - out_offset += snprintf(buf + out_offset, out_count - out_offset,
> > + out_offset += scnprintf(buf + out_offset, out_count - out_offset,
> > "rx_ring_empty - %llu\n", qp->rx_ring_empty);
> > - out_offset += snprintf(buf + out_offset, out_count - out_offset,
> > + out_offset += scnprintf(buf + out_offset, out_count - out_offset,
> > "rx_err_no_buf - %llu\n", qp->rx_err_no_buf);
> > - out_offset += snprintf(buf + out_offset, out_count - out_offset,
> > + out_offset += scnprintf(buf + out_offset, out_count - out_offset,
> > "rx_err_oflow - \t%llu\n", qp->rx_err_oflow);
> > - out_offset += snprintf(buf + out_offset, out_count - out_offset,
> > + out_offset += scnprintf(buf + out_offset, out_count - out_offset,
> > "rx_err_ver - \t%llu\n", qp->rx_err_ver);
> > - out_offset += snprintf(buf + out_offset, out_count - out_offset,
> > + out_offset += scnprintf(buf + out_offset, out_count - out_offset,
> > "rx_buff - \t0x%p\n", qp->rx_buff);
> > - out_offset += snprintf(buf + out_offset, out_count - out_offset,
> > + out_offset += scnprintf(buf + out_offset, out_count - out_offset,
> > "rx_index - \t%u\n", qp->rx_index);
> > - out_offset += snprintf(buf + out_offset, out_count - out_offset,
> > + out_offset += scnprintf(buf + out_offset, out_count - out_offset,
> > "rx_max_entry - \t%u\n", qp->rx_max_entry);
> > - out_offset += snprintf(buf + out_offset, out_count - out_offset,
> > + out_offset += scnprintf(buf + out_offset, out_count - out_offset,
> > "rx_alloc_entry - \t%u\n\n", qp->rx_alloc_entry);
> >
> > - out_offset += snprintf(buf + out_offset, out_count - out_offset,
> > + out_offset += scnprintf(buf + out_offset, out_count - out_offset,
> > "tx_bytes - \t%llu\n", qp->tx_bytes);
> > - out_offset += snprintf(buf + out_offset, out_count - out_offset,
> > + out_offset += scnprintf(buf + out_offset, out_count - out_offset,
> > "tx_pkts - \t%llu\n", qp->tx_pkts);
> > - out_offset += snprintf(buf + out_offset, out_count - out_offset,
> > + out_offset += scnprintf(buf + out_offset, out_count - out_offset,
> > "tx_memcpy - \t%llu\n", qp->tx_memcpy);
> > - out_offset += snprintf(buf + out_offset, out_count - out_offset,
> > + out_offset += scnprintf(buf + out_offset, out_count - out_offset,
> > "tx_async - \t%llu\n", qp->tx_async);
> > - out_offset += snprintf(buf + out_offset, out_count - out_offset,
> > + out_offset += scnprintf(buf + out_offset, out_count - out_offset,
> > "tx_ring_full - \t%llu\n", qp->tx_ring_full);
> > - out_offset += snprintf(buf + out_offset, out_count - out_offset,
> > + out_offset += scnprintf(buf + out_offset, out_count - out_offset,
> > "tx_err_no_buf - %llu\n", qp->tx_err_no_buf);
> > - out_offset += snprintf(buf + out_offset, out_count - out_offset,
> > + out_offset += scnprintf(buf + out_offset, out_count - out_offset,
> > "tx_mw - \t0x%p\n", qp->tx_mw);
> > - out_offset += snprintf(buf + out_offset, out_count - out_offset,
> > + out_offset += scnprintf(buf + out_offset, out_count - out_offset,
> > "tx_index (H) - \t%u\n", qp->tx_index);
> > - out_offset += snprintf(buf + out_offset, out_count - out_offset,
> > + out_offset += scnprintf(buf + out_offset, out_count - out_offset,
> > "RRI (T) - \t%u\n",
> > qp->remote_rx_info->entry);
> > - out_offset += snprintf(buf + out_offset, out_count - out_offset,
> > + out_offset += scnprintf(buf + out_offset, out_count - out_offset,
> > "tx_max_entry - \t%u\n", qp->tx_max_entry);
> > - out_offset += snprintf(buf + out_offset, out_count - out_offset,
> > + out_offset += scnprintf(buf + out_offset, out_count - out_offset,
> > "free tx - \t%u\n",
> > ntb_transport_tx_free_entry(qp));
> >
> > - out_offset += snprintf(buf + out_offset, out_count - out_offset,
> > + out_offset += scnprintf(buf + out_offset, out_count - out_offset,
> > "\n");
> > - out_offset += snprintf(buf + out_offset, out_count - out_offset,
> > + out_offset += scnprintf(buf + out_offset, out_count - out_offset,
> > "Using TX DMA - \t%s\n",
> > qp->tx_dma_chan ? "Yes" : "No");
> > - out_offset += snprintf(buf + out_offset, out_count - out_offset,
> > + out_offset += scnprintf(buf + out_offset, out_count - out_offset,
> > "Using RX DMA - \t%s\n",
> > qp->rx_dma_chan ? "Yes" : "No");
> > - out_offset += snprintf(buf + out_offset, out_count - out_offset,
> > + out_offset += scnprintf(buf + out_offset, out_count - out_offset,
> > "QP Link - \t%s\n",
> > qp->link_is_up ? "Up" : "Down");
> > - out_offset += snprintf(buf + out_offset, out_count - out_offset,
> > + out_offset += scnprintf(buf + out_offset, out_count - out_offset,
> > "\n");
> >
> > if (out_offset > out_count)
> >
prev parent reply other threads:[~2020-03-13 13:19 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-11 8:49 [PATCH] NTB: ntb_transport: Use scnprintf() for avoiding potential buffer overflow Takashi Iwai
2020-03-11 17:08 ` Logan Gunthorpe
2020-03-13 13:19 ` Jon Mason [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=20200313131928.GC13046@kudzu.us \
--to=jdmason@kudzu.us \
--cc=allenbh@gmail.com \
--cc=dave.jiang@intel.com \
--cc=linux-ntb@googlegroups.com \
--cc=logang@deltatee.com \
--cc=tiwai@suse.de \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox