[PATCH][next] iommu/dma: Use kvcalloc() instead of kvzalloc()
Use 2-factor argument form kvcalloc() instead of kvzalloc(). Link: https://github.com/KSPP/linux/issues/162 Signed-off-by: Gustavo A. R. Silva --- drivers/iommu/dma-iommu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c index 896bea04c347..18c6edbe5fbf 100644 --- a/drivers/iommu/dma-iommu.c +++ b/drivers/iommu/dma-iommu.c @@ -616,7 +616,7 @@ static struct page **__iommu_dma_alloc_pages(struct device *dev, if (!order_mask) return NULL; - pages = kvzalloc(count * sizeof(*pages), GFP_KERNEL); + pages = kvcalloc(count, sizeof(*pages), GFP_KERNEL); if (!pages) return NULL; -- 2.27.0 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH][next] iommu/vt-d: Fix out-bounds-warning in intel_svm_page_response()
Hi Balou, On 4/14/21 00:24, Lu Baolu wrote: > Hi Gustavo, > > On 4/14/21 3:54 AM, Gustavo A. R. Silva wrote: >> Replace call to memcpy() with just a couple of simple assignments in >> order to fix the following out-of-bounds warning: >> >> drivers/iommu/intel/svm.c:1198:4: warning: 'memcpy' offset [25, 32] from the >> object at 'desc' is out of the bounds of referenced subobject 'qw2' with type >> 'long long unsigned int' at offset 16 [-Warray-bounds] >> >> The problem is that the original code is trying to copy data into a >> couple of struct members adjacent to each other in a single call to >> memcpy(). This causes a legitimate compiler warning because memcpy() >> overruns the length of &desc.qw2. >> >> This helps with the ongoing efforts to globally enable -Warray-bounds >> and get us closer to being able to tighten the FORTIFY_SOURCE routines >> on memcpy(). >> >> Link: https://github.com/KSPP/linux/issues/109 >> Signed-off-by: Gustavo A. R. Silva >> --- >> drivers/iommu/intel/svm.c | 7 --- >> 1 file changed, 4 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/iommu/intel/svm.c b/drivers/iommu/intel/svm.c >> index 5165cea90421..65909f504c50 100644 >> --- a/drivers/iommu/intel/svm.c >> +++ b/drivers/iommu/intel/svm.c >> @@ -1194,9 +1194,10 @@ int intel_svm_page_response(struct device *dev, >> desc.qw1 = QI_PGRP_IDX(prm->grpid) | QI_PGRP_LPIG(last_page); >> desc.qw2 = 0; >> desc.qw3 = 0; >> - if (private_present) >> - memcpy(&desc.qw2, prm->private_data, >> - sizeof(prm->private_data)); > > The same memcpy() is used in multiple places in this file. Did they > compile the same warnings? Or there are multiple patches to fix them > one by one? I just see one more instance of this same case: 1023 if (req->priv_data_present) 1024 memcpy(&resp.qw2, req->priv_data, 1025sizeof(req->priv_data)); I missed it and I'll address it in v2. Do you see another one? Thanks for the feedback! -- Gustavo ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2][next] iommu/vt-d: Fix out-bounds-warning in intel_svm_page_response()
Replace a couple of calls to memcpy() with simple assignments in order to fix the following out-of-bounds warning: drivers/iommu/intel/svm.c:1198:4: warning: 'memcpy' offset [25, 32] from the object at 'desc' is out of the bounds of referenced subobject 'qw2' with type 'long long unsigned int' at offset 16 [-Warray-bounds] The problem is that the original code is trying to copy data into a couple of struct members adjacent to each other in a single call to memcpy(). This causes a legitimate compiler warning because memcpy() overruns the length of &desc.qw2 and &resp.qw2, respectively. This helps with the ongoing efforts to globally enable -Warray-bounds and get us closer to being able to tighten the FORTIFY_SOURCE routines on memcpy(). Link: https://github.com/KSPP/linux/issues/109 Signed-off-by: Gustavo A. R. Silva --- Changes in v2: - Fix another instance of this same issue in prq_event_thread(). drivers/iommu/intel/svm.c | 14 -- 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/drivers/iommu/intel/svm.c b/drivers/iommu/intel/svm.c index 5165cea90421..332365ec3195 100644 --- a/drivers/iommu/intel/svm.c +++ b/drivers/iommu/intel/svm.c @@ -1020,9 +1020,10 @@ static irqreturn_t prq_event_thread(int irq, void *d) resp.qw2 = 0; resp.qw3 = 0; - if (req->priv_data_present) - memcpy(&resp.qw2, req->priv_data, - sizeof(req->priv_data)); + if (req->priv_data_present) { + resp.qw2 = req->priv_data[0]; + resp.qw3 = req->priv_data[1]; + } qi_submit_sync(iommu, &resp, 1, 0); } prq_advance: @@ -1194,9 +1195,10 @@ int intel_svm_page_response(struct device *dev, desc.qw1 = QI_PGRP_IDX(prm->grpid) | QI_PGRP_LPIG(last_page); desc.qw2 = 0; desc.qw3 = 0; - if (private_present) - memcpy(&desc.qw2, prm->private_data, - sizeof(prm->private_data)); + if (private_present) { + desc.qw2 = prm->private_data[0]; + desc.qw3 = prm->private_data[1]; + } qi_submit_sync(iommu, &desc, 1, 0); } -- 2.27.0 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH][next] iommu/vt-d: Fix out-bounds-warning in intel_svm_page_response()
Replace call to memcpy() with just a couple of simple assignments in order to fix the following out-of-bounds warning: drivers/iommu/intel/svm.c:1198:4: warning: 'memcpy' offset [25, 32] from the object at 'desc' is out of the bounds of referenced subobject 'qw2' with type 'long long unsigned int' at offset 16 [-Warray-bounds] The problem is that the original code is trying to copy data into a couple of struct members adjacent to each other in a single call to memcpy(). This causes a legitimate compiler warning because memcpy() overruns the length of &desc.qw2. This helps with the ongoing efforts to globally enable -Warray-bounds and get us closer to being able to tighten the FORTIFY_SOURCE routines on memcpy(). Link: https://github.com/KSPP/linux/issues/109 Signed-off-by: Gustavo A. R. Silva --- drivers/iommu/intel/svm.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/iommu/intel/svm.c b/drivers/iommu/intel/svm.c index 5165cea90421..65909f504c50 100644 --- a/drivers/iommu/intel/svm.c +++ b/drivers/iommu/intel/svm.c @@ -1194,9 +1194,10 @@ int intel_svm_page_response(struct device *dev, desc.qw1 = QI_PGRP_IDX(prm->grpid) | QI_PGRP_LPIG(last_page); desc.qw2 = 0; desc.qw3 = 0; - if (private_present) - memcpy(&desc.qw2, prm->private_data, - sizeof(prm->private_data)); + if (private_present) { + desc.qw2 = prm->private_data[0]; + desc.qw3 = prm->private_data[1]; + } qi_submit_sync(iommu, &desc, 1, 0); } -- 2.27.0 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [trivial PATCH] treewide: Convert switch/case fallthrough; to break;
On 9/9/20 15:06, Joe Perches wrote: > fallthrough to a separate case/default label break; isn't very readable. > > Convert pseudo-keyword fallthrough; statements to a simple break; when > the next label is case or default and the only statement in the next > label block is break; > > Found using: > > $ grep-2.5.4 -rP --include=*.[ch] -n > "fallthrough;(\s*(case\s+\w+|default)\s*:\s*){1,7}break;" * > > Miscellanea: > > o Move or coalesce a couple label blocks above a default: block. > > Signed-off-by: Joe Perches Acked-by: Gustavo A. R. Silva Thanks -- Gustavo > --- > > Compiled allyesconfig x86-64 only. > A few files for other arches were not compiled. > > arch/arm/mach-mmp/pm-pxa910.c | 2 +- > arch/arm64/kvm/handle_exit.c | 2 +- > arch/mips/kernel/cpu-probe.c | 2 +- > arch/mips/math-emu/cp1emu.c | 2 +- > arch/s390/pci/pci.c | 2 +- > crypto/tcrypt.c | 4 ++-- > drivers/ata/sata_mv.c | 2 +- > drivers/atm/lanai.c | 2 +- > drivers/gpu/drm/i915/display/intel_sprite.c | 2 +- > drivers/gpu/drm/nouveau/nvkm/engine/disp/hdmi.c | 2 +- > drivers/hid/wacom_wac.c | 2 +- > drivers/i2c/busses/i2c-i801.c | 2 +- > drivers/infiniband/ulp/rtrs/rtrs-clt.c| 14 +++--- > drivers/infiniband/ulp/rtrs/rtrs-srv.c| 6 +++--- > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 2 +- > drivers/irqchip/irq-vic.c | 4 ++-- > drivers/md/dm.c | 2 +- > drivers/media/dvb-frontends/drxd_hard.c | 2 +- > drivers/media/i2c/ov5640.c| 2 +- > drivers/media/i2c/ov6650.c| 5 ++--- > drivers/media/i2c/smiapp/smiapp-core.c| 2 +- > drivers/media/i2c/tvp5150.c | 2 +- > drivers/media/pci/ddbridge/ddbridge-core.c| 2 +- > drivers/media/usb/cpia2/cpia2_core.c | 2 +- > drivers/mfd/iqs62x.c | 3 +-- > drivers/mmc/host/atmel-mci.c | 2 +- > drivers/mtd/nand/raw/nandsim.c| 2 +- > drivers/net/ethernet/intel/e1000e/phy.c | 2 +- > drivers/net/ethernet/intel/fm10k/fm10k_pf.c | 2 +- > drivers/net/ethernet/intel/i40e/i40e_adminq.c | 2 +- > drivers/net/ethernet/intel/i40e/i40e_txrx.c | 2 +- > drivers/net/ethernet/intel/iavf/iavf_txrx.c | 2 +- > drivers/net/ethernet/intel/igb/e1000_phy.c| 2 +- > drivers/net/ethernet/intel/ixgbe/ixgbe_82599.c| 2 +- > drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 2 +- > drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c| 2 +- > drivers/net/ethernet/intel/ixgbevf/vf.c | 2 +- > drivers/net/ethernet/netronome/nfp/nfpcore/nfp6000_pcie.c | 2 +- > drivers/net/ethernet/qlogic/qed/qed_mcp.c | 2 +- > drivers/net/ethernet/sfc/falcon/farch.c | 2 +- > drivers/net/ethernet/sfc/farch.c | 2 +- > drivers/net/phy/adin.c| 3 +-- > drivers/net/usb/pegasus.c | 4 ++-- > drivers/net/usb/usbnet.c | 2 +- > drivers/net/wireless/ath/ath5k/eeprom.c | 2 +- > drivers/net/wireless/mediatek/mt7601u/dma.c | 8 > drivers/nvme/host/core.c | 12 ++-- > drivers/pcmcia/db1xxx_ss.c| 4 ++-- > drivers/power/supply/abx500_chargalg.c| 2 +- > drivers/power/supply/charger-manager.c| 2 +- > drivers/rtc/rtc-pcf85063.c| 2 +- > drivers/s390/scsi/zfcp_fsf.c | 2 +- > drivers/scsi/aic7xxx/aic79xx_core.c | 4 ++-- > drivers/scsi/aic94xx/aic94xx_tmf.c| 2 +- > drivers/scsi/lpfc/lpfc_sli.c | 2 +- > drivers/scsi/smartpqi/smartpqi_init.c | 2 +- > drivers/scsi/sr.c | 2 +- > drivers/tty/serial/sunsu.c
[PATCH] iommu/qcom: Replace zero-length array with flexible-array member
The current codebase makes use of the zero-length array language extension to the C90 standard, but the preferred mechanism to declare variable-length types such as these ones is a flexible array member[1][2], introduced in C99: struct foo { int stuff; struct boo array[]; }; By making use of the mechanism above, we will get a compiler warning in case the flexible array does not occur last in the structure, which will help us prevent some kind of undefined behavior bugs from being inadvertently introduced[3] to the codebase from now on. Also, notice that, dynamic memory allocations won't be affected by this change: "Flexible array members have incomplete type, and so the sizeof operator may not be applied. As a quirk of the original implementation of zero-length arrays, sizeof evaluates to zero."[1] This issue was found with the help of Coccinelle. [1] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html [2] https://github.com/KSPP/linux/issues/21 [3] commit 76497732932f ("cxgb3/l2t: Fix undefined behaviour") Signed-off-by: Gustavo A. R. Silva --- drivers/iommu/qcom_iommu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/iommu/qcom_iommu.c b/drivers/iommu/qcom_iommu.c index 39759db4f003..f1e175ca5e4a 100644 --- a/drivers/iommu/qcom_iommu.c +++ b/drivers/iommu/qcom_iommu.c @@ -48,7 +48,7 @@ struct qcom_iommu_dev { void __iomem*local_base; u32 sec_id; u8 num_ctxs; - struct qcom_iommu_ctx *ctxs[0]; /* indexed by asid-1 */ + struct qcom_iommu_ctx *ctxs[]; /* indexed by asid-1 */ }; struct qcom_iommu_ctx { -- 2.23.0 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH] iommu/qcom_iommu: Use struct_size() helper
One of the more common cases of allocation size calculations is finding the size of a structure that has a zero-sized array at the end, along with memory for some number of elements for that array. For example: struct qcom_iommu_dev { ... struct qcom_iommu_ctx *ctxs[0]; /* indexed by asid-1 */ }; Make use of the struct_size() helper instead of an open-coded version in order to avoid any potential type mistakes. So, replace the following form: sizeof(*qcom_iommu) + (max_asid * sizeof(qcom_iommu->ctxs[0])) with: struct_size(qcom_iommu, ctxs, max_asid) Also, notice that, in this case, variable sz is not necessary, hence it is removed. This code was detected with the help of Coccinelle. Signed-off-by: Gustavo A. R. Silva --- drivers/iommu/qcom_iommu.c | 7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/drivers/iommu/qcom_iommu.c b/drivers/iommu/qcom_iommu.c index 3608f58f1ea8..c18168fd7fe7 100644 --- a/drivers/iommu/qcom_iommu.c +++ b/drivers/iommu/qcom_iommu.c @@ -801,7 +801,7 @@ static int qcom_iommu_device_probe(struct platform_device *pdev) struct qcom_iommu_dev *qcom_iommu; struct device *dev = &pdev->dev; struct resource *res; - int ret, sz, max_asid = 0; + int ret, max_asid = 0; /* find the max asid (which is 1:1 to ctx bank idx), so we know how * many child ctx devices we have: @@ -809,9 +809,8 @@ static int qcom_iommu_device_probe(struct platform_device *pdev) for_each_child_of_node(dev->of_node, child) max_asid = max(max_asid, get_asid(child)); - sz = sizeof(*qcom_iommu) + (max_asid * sizeof(qcom_iommu->ctxs[0])); - - qcom_iommu = devm_kzalloc(dev, sz, GFP_KERNEL); + qcom_iommu = devm_kzalloc(dev, struct_size(qcom_iommu, ctxs, max_asid), + GFP_KERNEL); if (!qcom_iommu) return -ENOMEM; qcom_iommu->num_ctxs = max_asid; -- 2.23.0
Re: [PATCH] iommu/dmar: Use struct_size() helper
On 4/26/19 9:44 AM, Joerg Roedel wrote: > On Thu, Apr 18, 2019 at 01:46:24PM -0500, Gustavo A. R. Silva wrote: >> Make use of the struct_size() helper instead of an open-coded version >> in order to avoid any potential type mistakes, in particular in the >> context in which this code is being used. >> >> So, replace code of the following form: >> >> size = sizeof(*info) + level * sizeof(info->path[0]); >> >> with: >> >> size = struct_size(info, path, level); >> >> Signed-off-by: Gustavo A. R. Silva > > Applied, thanks. > Great. :) Thanks, Joerg. -- Gustavo ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH] iommu/dmar: Use struct_size() helper
Make use of the struct_size() helper instead of an open-coded version in order to avoid any potential type mistakes, in particular in the context in which this code is being used. So, replace code of the following form: size = sizeof(*info) + level * sizeof(info->path[0]); with: size = struct_size(info, path, level); Signed-off-by: Gustavo A. R. Silva --- drivers/iommu/dmar.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/iommu/dmar.c b/drivers/iommu/dmar.c index 9c49300e9fb7..6d969a172fbb 100644 --- a/drivers/iommu/dmar.c +++ b/drivers/iommu/dmar.c @@ -145,7 +145,7 @@ dmar_alloc_pci_notify_info(struct pci_dev *dev, unsigned long event) for (tmp = dev; tmp; tmp = tmp->bus->self) level++; - size = sizeof(*info) + level * sizeof(info->path[0]); + size = struct_size(info, path, level); if (size <= sizeof(dmar_pci_notify_info_buf)) { info = (struct dmar_pci_notify_info *)dmar_pci_notify_info_buf; } else { -- 2.21.0 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH] iommu: remove unnecessary code
did_old is an unsigned variable and, greater-than-or-equal-to-zero comparison of an unsigned variable is always true. Addresses-Coverity-ID: 1398477 Signed-off-by: Gustavo A. R. Silva --- drivers/iommu/intel-iommu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c index d412a31..98daf4a 100644 --- a/drivers/iommu/intel-iommu.c +++ b/drivers/iommu/intel-iommu.c @@ -2050,7 +2050,7 @@ static int domain_context_mapping_one(struct dmar_domain *domain, if (context_copied(context)) { u16 did_old = context_domain_id(context); - if (did_old >= 0 && did_old < cap_ndoms(iommu->cap)) + if (did_old < cap_ndoms(iommu->cap)) iommu->flush.flush_context(iommu, did_old, (((u16)bus) << 8) | devfn, DMA_CCMD_MASK_NOBIT, -- 2.5.0 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu