* [dpdk-dev] [PATCH 1/2] examples/vhost_scsi: add virtio-1.0 feature bit support @ 2018-05-17 23:32 Changpeng Liu 2018-05-17 23:32 ` [dpdk-dev] [PATCH 2/2] examples/vhost_scsi: fix potential buffer overrun with safe copy API Changpeng Liu 2018-05-18 12:35 ` [dpdk-dev] [PATCH 1/2] examples/vhost_scsi: add virtio-1.0 feature bit support Maxime Coquelin 0 siblings, 2 replies; 9+ messages in thread From: Changpeng Liu @ 2018-05-17 23:32 UTC (permalink / raw) To: changpeng.liu, dev Signed-off-by: Changpeng Liu <changpeng.liu@intel.com> --- examples/vhost_scsi/vhost_scsi.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/examples/vhost_scsi/vhost_scsi.c b/examples/vhost_scsi/vhost_scsi.c index 2908ff6..a1d542b 100644 --- a/examples/vhost_scsi/vhost_scsi.c +++ b/examples/vhost_scsi/vhost_scsi.c @@ -20,9 +20,10 @@ #include "vhost_scsi.h" #include "scsi_spec.h" -#define VIRTIO_SCSI_FEATURES ((1 << VIRTIO_F_NOTIFY_ON_EMPTY) |\ - (1 << VIRTIO_SCSI_F_INOUT) |\ - (1 << VIRTIO_SCSI_F_CHANGE)) +#define VIRTIO_SCSI_FEATURES ((1ULL << VIRTIO_F_NOTIFY_ON_EMPTY) |\ + (1ULL << VIRTIO_SCSI_F_INOUT) |\ + (1ULL << VIRTIO_SCSI_F_CHANGE) |\ + (1ULL << VIRTIO_F_VERSION_1)) /* Path to folder where character device will be created. Can be set by user. */ static char dev_pathname[PATH_MAX] = ""; -- 1.9.3 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [dpdk-dev] [PATCH 2/2] examples/vhost_scsi: fix potential buffer overrun with safe copy API 2018-05-17 23:32 [dpdk-dev] [PATCH 1/2] examples/vhost_scsi: add virtio-1.0 feature bit support Changpeng Liu @ 2018-05-17 23:32 ` Changpeng Liu 2018-05-22 17:47 ` Thomas Monjalon 2018-05-18 12:35 ` [dpdk-dev] [PATCH 1/2] examples/vhost_scsi: add virtio-1.0 feature bit support Maxime Coquelin 1 sibling, 1 reply; 9+ messages in thread From: Changpeng Liu @ 2018-05-17 23:32 UTC (permalink / raw) To: changpeng.liu, dev Signed-off-by: Changpeng Liu <changpeng.liu@intel.com> --- examples/vhost_scsi/scsi.c | 23 ++++++++++++----------- examples/vhost_scsi/vhost_scsi.c | 5 +++-- 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/examples/vhost_scsi/scsi.c b/examples/vhost_scsi/scsi.c index 0c2fa3e..1572098 100644 --- a/examples/vhost_scsi/scsi.c +++ b/examples/vhost_scsi/scsi.c @@ -182,8 +182,8 @@ break; case SPC_VPD_UNIT_SERIAL_NUMBER: hlen = 4; - strlcpy((char *)vpage->params, bdev->name, - sizeof(vpage->params)); + vhost_strcpy_pad((char *)vpage->params, bdev->name, + sizeof(vpage->params), ' '); vpage->alloc_len = rte_cpu_to_be_16(32); break; case SPC_VPD_DEVICE_IDENTIFICATION: @@ -217,10 +217,11 @@ desig->piv = 1; desig->reserved1 = 0; desig->len = 8 + 16 + 32; - strlcpy((char *)desig->desig, "INTEL", 8); + vhost_strcpy_pad((char *)desig->desig, "INTEL", 8, ' '); vhost_strcpy_pad((char *)&desig->desig[8], bdev->product_name, 16, ' '); - strlcpy((char *)&desig->desig[24], bdev->name, 32); + vhost_strcpy_pad((char *)&desig->desig[24], bdev->name, + 32, ' '); len += sizeof(struct scsi_desig_desc) + 8 + 16 + 32; buf += sizeof(struct scsi_desig_desc) + desig->len; @@ -277,17 +278,17 @@ inqdata->flags3 = 0x2; /* T10 VENDOR IDENTIFICATION */ - strlcpy((char *)inqdata->t10_vendor_id, "INTEL", - sizeof(inqdata->t10_vendor_id)); + vhost_strcpy_pad((char *)inqdata->t10_vendor_id, "INTEL", + sizeof(inqdata->t10_vendor_id), ' '); /* PRODUCT IDENTIFICATION */ - snprintf((char *)inqdata->product_id, - RTE_DIM(inqdata->product_id), "%s", - bdev->product_name); + vhost_strcpy_pad((char *)inqdata->product_id, + bdev->product_name, + sizeof(inqdata->product_id), ' '); /* PRODUCT REVISION LEVEL */ - strlcpy((char *)inqdata->product_rev, "0001", - sizeof(inqdata->product_rev)); + vhost_strcpy_pad((char *)inqdata->product_rev, "0001", + sizeof(inqdata->product_rev), ' '); /* Standard inquiry data ends here. Only populate * remaining fields if alloc_len indicates enough diff --git a/examples/vhost_scsi/vhost_scsi.c b/examples/vhost_scsi/vhost_scsi.c index a1d542b..4e57462 100644 --- a/examples/vhost_scsi/vhost_scsi.c +++ b/examples/vhost_scsi/vhost_scsi.c @@ -183,8 +183,9 @@ static uint64_t gpa_to_vva(int vid, uint64_t gpa, uint64_t *len) if (!bdev) return NULL; - strncpy(bdev->name, bdev_name, sizeof(bdev->name)); - strncpy(bdev->product_name, bdev_serial, sizeof(bdev->product_name)); + snprintf(bdev->name, sizeof(bdev->name), "%s", bdev_name); + snprintf(bdev->product_name, sizeof(bdev->product_name), + "%s", bdev_serial); bdev->blocklen = blk_size; bdev->blockcnt = blk_cnt; bdev->write_cache = wce_enable; -- 1.9.3 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH 2/2] examples/vhost_scsi: fix potential buffer overrun with safe copy API 2018-05-17 23:32 ` [dpdk-dev] [PATCH 2/2] examples/vhost_scsi: fix potential buffer overrun with safe copy API Changpeng Liu @ 2018-05-22 17:47 ` Thomas Monjalon 2018-05-22 17:58 ` Liu, Changpeng 0 siblings, 1 reply; 9+ messages in thread From: Thomas Monjalon @ 2018-05-22 17:47 UTC (permalink / raw) To: Changpeng Liu; +Cc: dev 18/05/2018 01:32, Changpeng Liu: > Signed-off-by: Changpeng Liu <changpeng.liu@intel.com> Missing explanations. > - strlcpy((char *)vpage->params, bdev->name, > - sizeof(vpage->params)); > + vhost_strcpy_pad((char *)vpage->params, bdev->name, > + sizeof(vpage->params), ' '); Why do you think vhost_strcpy_pad is safer than strlcpy? > - strncpy(bdev->name, bdev_name, sizeof(bdev->name)); > - strncpy(bdev->product_name, bdev_serial, sizeof(bdev->product_name)); > + snprintf(bdev->name, sizeof(bdev->name), "%s", bdev_name); > + snprintf(bdev->product_name, sizeof(bdev->product_name), > + "%s", bdev_serial); You should use strlcpy. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH 2/2] examples/vhost_scsi: fix potential buffer overrun with safe copy API 2018-05-22 17:47 ` Thomas Monjalon @ 2018-05-22 17:58 ` Liu, Changpeng 2018-05-22 18:18 ` Thomas Monjalon 0 siblings, 1 reply; 9+ messages in thread From: Liu, Changpeng @ 2018-05-22 17:58 UTC (permalink / raw) To: Thomas Monjalon; +Cc: dev > -----Original Message----- > From: Thomas Monjalon [mailto:thomas@monjalon.net] > Sent: Tuesday, May 22, 2018 10:48 AM > To: Liu, Changpeng <changpeng.liu@intel.com> > Cc: dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH 2/2] examples/vhost_scsi: fix potential buffer > overrun with safe copy API > > 18/05/2018 01:32, Changpeng Liu: > > Signed-off-by: Changpeng Liu <changpeng.liu@intel.com> > > Missing explanations. > > > - strlcpy((char *)vpage->params, bdev->name, > > - sizeof(vpage->params)); > > + vhost_strcpy_pad((char *)vpage->params, bdev->name, > > + sizeof(vpage->params), ' '); > > Why do you think vhost_strcpy_pad is safer than strlcpy? A code Coverity issue 279452 reported for strlcpy, so here replace with internal API can avoid it. > > > - strncpy(bdev->name, bdev_name, sizeof(bdev->name)); > > - strncpy(bdev->product_name, bdev_serial, sizeof(bdev->product_name)); > > + snprintf(bdev->name, sizeof(bdev->name), "%s", bdev_name); > > + snprintf(bdev->product_name, sizeof(bdev->product_name), > > + "%s", bdev_serial); > > You should use strlcpy. > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH 2/2] examples/vhost_scsi: fix potential buffer overrun with safe copy API 2018-05-22 17:58 ` Liu, Changpeng @ 2018-05-22 18:18 ` Thomas Monjalon 0 siblings, 0 replies; 9+ messages in thread From: Thomas Monjalon @ 2018-05-22 18:18 UTC (permalink / raw) To: Liu, Changpeng; +Cc: dev 22/05/2018 19:58, Liu, Changpeng: > From: Thomas Monjalon [mailto:thomas@monjalon.net] > > 18/05/2018 01:32, Changpeng Liu: > > > - strlcpy((char *)vpage->params, bdev->name, > > > - sizeof(vpage->params)); > > > + vhost_strcpy_pad((char *)vpage->params, bdev->name, > > > + sizeof(vpage->params), ' '); > > > > Why do you think vhost_strcpy_pad is safer than strlcpy? > > A code Coverity issue 279452 reported for strlcpy, so here replace with internal API can avoid it. I think it is a false positive. Remember that Coverity is just a tool. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH 1/2] examples/vhost_scsi: add virtio-1.0 feature bit support 2018-05-17 23:32 [dpdk-dev] [PATCH 1/2] examples/vhost_scsi: add virtio-1.0 feature bit support Changpeng Liu 2018-05-17 23:32 ` [dpdk-dev] [PATCH 2/2] examples/vhost_scsi: fix potential buffer overrun with safe copy API Changpeng Liu @ 2018-05-18 12:35 ` Maxime Coquelin 2018-05-22 17:51 ` Thomas Monjalon 1 sibling, 1 reply; 9+ messages in thread From: Maxime Coquelin @ 2018-05-18 12:35 UTC (permalink / raw) To: Changpeng Liu, dev On 05/18/2018 01:32 AM, Changpeng Liu wrote: > Signed-off-by: Changpeng Liu <changpeng.liu@intel.com> > --- > examples/vhost_scsi/vhost_scsi.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com> Thanks, Maxime ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH 1/2] examples/vhost_scsi: add virtio-1.0 feature bit support 2018-05-18 12:35 ` [dpdk-dev] [PATCH 1/2] examples/vhost_scsi: add virtio-1.0 feature bit support Maxime Coquelin @ 2018-05-22 17:51 ` Thomas Monjalon 2018-05-22 20:33 ` Thomas Monjalon 0 siblings, 1 reply; 9+ messages in thread From: Thomas Monjalon @ 2018-05-22 17:51 UTC (permalink / raw) To: Changpeng Liu; +Cc: dev, Maxime Coquelin 18/05/2018 14:35, Maxime Coquelin: > > On 05/18/2018 01:32 AM, Changpeng Liu wrote: > > Signed-off-by: Changpeng Liu <changpeng.liu@intel.com> > > --- > > examples/vhost_scsi/vhost_scsi.c | 7 ++++--- > > 1 file changed, 4 insertions(+), 3 deletions(-) > > Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com> Patch 1 applied alone, thanks ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH 1/2] examples/vhost_scsi: add virtio-1.0 feature bit support 2018-05-22 17:51 ` Thomas Monjalon @ 2018-05-22 20:33 ` Thomas Monjalon 2018-05-22 20:45 ` Liu, Changpeng 0 siblings, 1 reply; 9+ messages in thread From: Thomas Monjalon @ 2018-05-22 20:33 UTC (permalink / raw) To: Changpeng Liu; +Cc: dev, Maxime Coquelin 22/05/2018 19:51, Thomas Monjalon: > 18/05/2018 14:35, Maxime Coquelin: > > > > On 05/18/2018 01:32 AM, Changpeng Liu wrote: > > > Signed-off-by: Changpeng Liu <changpeng.liu@intel.com> > > > --- > > > examples/vhost_scsi/vhost_scsi.c | 7 ++++--- > > > 1 file changed, 4 insertions(+), 3 deletions(-) > > > > Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com> > > Patch 1 applied alone, thanks After compilation tests, the patch is not accepted in 18.05, because VIRTIO_F_VERSION_1 seems not defined on some distributions, despite the fallback implemented. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH 1/2] examples/vhost_scsi: add virtio-1.0 feature bit support 2018-05-22 20:33 ` Thomas Monjalon @ 2018-05-22 20:45 ` Liu, Changpeng 0 siblings, 0 replies; 9+ messages in thread From: Liu, Changpeng @ 2018-05-22 20:45 UTC (permalink / raw) To: Thomas Monjalon; +Cc: dev, Maxime Coquelin > -----Original Message----- > From: Thomas Monjalon [mailto:thomas@monjalon.net] > Sent: Tuesday, May 22, 2018 1:33 PM > To: Liu, Changpeng <changpeng.liu@intel.com> > Cc: dev@dpdk.org; Maxime Coquelin <maxime.coquelin@redhat.com> > Subject: Re: [dpdk-dev] [PATCH 1/2] examples/vhost_scsi: add virtio-1.0 feature > bit support > > 22/05/2018 19:51, Thomas Monjalon: > > 18/05/2018 14:35, Maxime Coquelin: > > > > > > On 05/18/2018 01:32 AM, Changpeng Liu wrote: > > > > Signed-off-by: Changpeng Liu <changpeng.liu@intel.com> > > > > --- > > > > examples/vhost_scsi/vhost_scsi.c | 7 ++++--- > > > > 1 file changed, 4 insertions(+), 3 deletions(-) > > > > > > Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com> > > > > Patch 1 applied alone, thanks > > After compilation tests, the patch is not accepted in 18.05, > because VIRTIO_F_VERSION_1 seems not defined on some distributions, > despite the fallback implemented. What a mess for vhost library. :). lib/librte_vhost/vhost.h has the definition VIRTIO_F_VERSION_1 for some older kernel, but didn't include in the public API header <rte_vhost.h>, looks like we should move this definition to rte_vhost.h for some old kernels. > ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2018-05-22 20:46 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-05-17 23:32 [dpdk-dev] [PATCH 1/2] examples/vhost_scsi: add virtio-1.0 feature bit support Changpeng Liu 2018-05-17 23:32 ` [dpdk-dev] [PATCH 2/2] examples/vhost_scsi: fix potential buffer overrun with safe copy API Changpeng Liu 2018-05-22 17:47 ` Thomas Monjalon 2018-05-22 17:58 ` Liu, Changpeng 2018-05-22 18:18 ` Thomas Monjalon 2018-05-18 12:35 ` [dpdk-dev] [PATCH 1/2] examples/vhost_scsi: add virtio-1.0 feature bit support Maxime Coquelin 2018-05-22 17:51 ` Thomas Monjalon 2018-05-22 20:33 ` Thomas Monjalon 2018-05-22 20:45 ` Liu, Changpeng
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).