[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 >> >> 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(, 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(, 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 and , 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(, 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, , 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(, 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, , 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
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(, 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, , 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| 2
[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 = >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 <garsi...@embeddedor.com>
---
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
