Hi, all,
While working on a bugfix on RTRS[1], I noticed there are quite a few other
drivers have the same problem, due to the fact dma_map_sg return 0 on error,
not like most of the cases, return negative value for error.
I "grep -A 5 dma_map_sg' in kernel tree, and audit/fix the one I feel is buggy,
hence this serry. All the patch are independent, so can go either through the
maintainer tree once it is reviewed.
Thanks!
[1] https://lore.kernel.org/linux-rdma/[email protected]/T/#t
Jack Wang (19):
mailbox: bcm-ferxrm-mailbox: Fix error check for dma_map_sg
infiniband/mthca: Fix dma_map_sg error check
HSI: omap_ssi_port: Fix dma_map_sg error check
mtd: rawnand: stm32_fmc2: Fix dma_map_sg error check
mtd: rawnand: marvell: Fix error handle regarding dma_map_sg
usb/hcd: Fix dma_map_sg error check
crypto: gemin: Fix error check for dma_map_sg
crypto: sahara: Fix error check for dma_map_sg
crypto: qce: Fix dma_map_sg error check
crypto: amlogic: Fix dma_map_sg error check
crypto: allwinner: Fix dma_map_sg error check
crypto: ccree: Fix dma_map_sg error check
intel_th: Fix dma_map_sg error check
nvme-rdma: Fix error check for ib_dma_map_sg
nvme-fc: Fix the error check for dma_map_sg
scsi/qlogicpti: Fix dma_map_sg check
mmc: meson-mx-sdhc: Fix error check for dma_map_sg
mmc: jz4740_mmc: Fix error check for dma_map_sg
net/mlx4: Fix error check for dma_map_sg
drivers/crypto/allwinner/sun8i-ce/sun8i-ce-cipher.c | 6 +++---
drivers/crypto/allwinner/sun8i-ce/sun8i-ce-hash.c | 2 +-
drivers/crypto/allwinner/sun8i-ss/sun8i-ss-cipher.c | 4 ++--
drivers/crypto/allwinner/sun8i-ss/sun8i-ss-hash.c | 2 +-
drivers/crypto/amlogic/amlogic-gxl-cipher.c | 6 +++---
drivers/crypto/ccree/cc_buffer_mgr.c | 2 +-
drivers/crypto/gemini/sl3516-ce-cipher.c | 6 +++---
drivers/crypto/qce/aead.c | 4 ++--
drivers/crypto/qce/sha.c | 8 +++++---
drivers/crypto/qce/skcipher.c | 8 ++++----
drivers/crypto/sahara.c | 4 ++--
drivers/hsi/controllers/omap_ssi_port.c | 8 ++++----
drivers/hwtracing/intel_th/msu.c | 2 +-
drivers/infiniband/hw/mthca/mthca_memfree.c | 7 ++++---
drivers/mailbox/bcm-flexrm-mailbox.c | 8 ++++----
drivers/mmc/host/jz4740_mmc.c | 2 +-
drivers/mmc/host/meson-mx-sdhc-mmc.c | 2 +-
drivers/mtd/nand/raw/marvell_nand.c | 8 +++++++-
drivers/mtd/nand/raw/stm32_fmc2_nand.c | 8 +++++---
drivers/net/ethernet/mellanox/mlx4/icm.c | 4 ++--
drivers/nvme/host/fc.c | 2 +-
drivers/nvme/host/rdma.c | 4 ++--
drivers/nvme/target/fc.c | 5 +++++
drivers/scsi/qlogicpti.c | 3 ++-
drivers/usb/core/hcd.c | 2 +-
25 files changed, 67 insertions(+), 50 deletions(-)
--
2.34.1
ib_dma_map_sg return 0 on error.
Cc: Keith Busch <[email protected]>
Cc: Jens Axboe <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Cc: Sagi Grimberg <[email protected]>
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Jack Wang <[email protected]>
---
drivers/nvme/host/rdma.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c
index 3100643be299..d70bccbcba3e 100644
--- a/drivers/nvme/host/rdma.c
+++ b/drivers/nvme/host/rdma.c
@@ -1551,7 +1551,7 @@ static int nvme_rdma_dma_map_req(struct ib_device *ibdev, struct request *rq,
*count = ib_dma_map_sg(ibdev, req->data_sgl.sg_table.sgl,
req->data_sgl.nents, rq_dma_dir(rq));
- if (unlikely(*count <= 0)) {
+ if (unlikely(!*count)) {
ret = -EIO;
goto out_free_table;
}
@@ -1574,7 +1574,7 @@ static int nvme_rdma_dma_map_req(struct ib_device *ibdev, struct request *rq,
req->metadata_sgl->sg_table.sgl,
req->metadata_sgl->nents,
rq_dma_dir(rq));
- if (unlikely(*pi_count <= 0)) {
+ if (unlikely(!*pi_count)) {
ret = -EIO;
goto out_free_pi_table;
}
--
2.34.1
dma_map_sg return 0 on error, add missing check.
Cc: James Smart <[email protected]>
Cc: Keith Busch <[email protected]>
Cc: Jens Axboe <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Cc: Sagi Grimberg <[email protected]>
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Jack Wang <[email protected]>
---
drivers/nvme/host/fc.c | 2 +-
drivers/nvme/target/fc.c | 5 +++++
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/nvme/host/fc.c b/drivers/nvme/host/fc.c
index 127abaf9ba5d..95050f2e0a06 100644
--- a/drivers/nvme/host/fc.c
+++ b/drivers/nvme/host/fc.c
@@ -2616,7 +2616,7 @@ nvme_fc_map_data(struct nvme_fc_ctrl *ctrl, struct request *rq,
WARN_ON(op->nents > blk_rq_nr_phys_segments(rq));
freq->sg_cnt = fc_dma_map_sg(ctrl->lport->dev, freq->sg_table.sgl,
op->nents, rq_dma_dir(rq));
- if (unlikely(freq->sg_cnt <= 0)) {
+ if (unlikely(!freq->sg_cnt)) {
sg_free_table_chained(&freq->sg_table, NVME_INLINE_SG_CNT);
freq->sg_cnt = 0;
return -EFAULT;
diff --git a/drivers/nvme/target/fc.c b/drivers/nvme/target/fc.c
index ab2627e17bb9..3749ca28860b 100644
--- a/drivers/nvme/target/fc.c
+++ b/drivers/nvme/target/fc.c
@@ -2089,6 +2089,11 @@ nvmet_fc_alloc_tgt_pgs(struct nvmet_fc_fcp_iod *fod)
((fod->io_dir == NVMET_FCP_WRITE) ?
DMA_FROM_DEVICE : DMA_TO_DEVICE));
/* note: write from initiator perspective */
+ if (!fod->data_sg_cnt) {
+ sgl_free(fod->data_sg);
+ fod->data_sg = NULL;
+ goto out;
+ }
fod->next_sg = fod->data_sg;
return 0;
--
2.34.1
dma_map_sg return 0 on error.
Cc: Paul Cercueil <[email protected]>
Cc: Ulf Hansson <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Jack Wang <[email protected]>
---
drivers/mmc/host/jz4740_mmc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mmc/host/jz4740_mmc.c b/drivers/mmc/host/jz4740_mmc.c
index b1d563b2ed1b..666600055b06 100644
--- a/drivers/mmc/host/jz4740_mmc.c
+++ b/drivers/mmc/host/jz4740_mmc.c
@@ -308,7 +308,7 @@ static int jz4740_mmc_prepare_dma_data(struct jz4740_mmc_host *host,
data->sg_len,
dir);
- if (sg_count <= 0) {
+ if (!sg_count) {
dev_err(mmc_dev(host->mmc),
"Failed to map scatterlist for DMA operation\n");
return -EINVAL;
--
2.34.1
dma_map_sg return 0 on error.
Cc: Tariq Toukan <[email protected]>
Cc: "David S. Miller" <[email protected]>
Cc: Eric Dumazet <[email protected]>
Cc: Jakub Kicinski <[email protected]>
Cc: Paolo Abeni <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Jack Wang <[email protected]>
---
drivers/net/ethernet/mellanox/mlx4/icm.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx4/icm.c b/drivers/net/ethernet/mellanox/mlx4/icm.c
index d89a3da89e5a..59b8b3c73582 100644
--- a/drivers/net/ethernet/mellanox/mlx4/icm.c
+++ b/drivers/net/ethernet/mellanox/mlx4/icm.c
@@ -208,7 +208,7 @@ struct mlx4_icm *mlx4_alloc_icm(struct mlx4_dev *dev, int npages,
chunk->sg, chunk->npages,
DMA_BIDIRECTIONAL);
- if (chunk->nsg <= 0)
+ if (!chunk->nsg)
goto fail;
}
@@ -222,7 +222,7 @@ struct mlx4_icm *mlx4_alloc_icm(struct mlx4_dev *dev, int npages,
chunk->nsg = dma_map_sg(&dev->persist->pdev->dev, chunk->sg,
chunk->npages, DMA_BIDIRECTIONAL);
- if (chunk->nsg <= 0)
+ if (!chunk->nsg)
goto fail;
}
--
2.34.1
dma_map_sg return 0 on error.
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Alan Stern <[email protected]>
Cc: Kishon Vijay Abraham I <[email protected]>
Cc: Alexey Sheplyakov <[email protected]>
Cc: Stephen Boyd <[email protected]>
Cc: Weitao Wang <[email protected]>
Cc: Matthias Kaehlcke <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Jack Wang <[email protected]>
---
drivers/usb/core/hcd.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 94b305bbd621..90dd32a24e5b 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -1474,7 +1474,7 @@ int usb_hcd_map_urb_for_dma(struct usb_hcd *hcd, struct urb *urb,
urb->sg,
urb->num_sgs,
dir);
- if (n <= 0)
+ if (!n)
ret = -EAGAIN;
else
urb->transfer_flags |= URB_DMA_MAP_SG;
--
2.34.1
dma_map_sg return 0 on error.
Cc: Ulf Hansson <[email protected]>
Cc: Neil Armstrong <[email protected]>
Cc: Kevin Hilman <[email protected]>
Cc: Jerome Brunet <[email protected]>
Cc: Martin Blumenstingl <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Jack Wang <[email protected]>
---
drivers/mmc/host/meson-mx-sdhc-mmc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mmc/host/meson-mx-sdhc-mmc.c b/drivers/mmc/host/meson-mx-sdhc-mmc.c
index e92e63cb5641..b32364f20c32 100644
--- a/drivers/mmc/host/meson-mx-sdhc-mmc.c
+++ b/drivers/mmc/host/meson-mx-sdhc-mmc.c
@@ -388,7 +388,7 @@ static int meson_mx_sdhc_map_dma(struct mmc_host *mmc, struct mmc_request *mrq)
dma_len = dma_map_sg(mmc_dev(mmc), data->sg, data->sg_len,
mmc_get_dma_dir(data));
- if (dma_len <= 0) {
+ if (!dma_len) {
dev_err(mmc_dev(mmc), "dma_map_sg failed\n");
return -ENOMEM;
}
--
2.34.1
dma_map_sg return 0 on error, in case of error return -EIO,
also add the dma_unmap_sg as rollback on the following error.
Cc: Miquel Raynal <[email protected]>
Cc: Richard Weinberger <[email protected]>
Cc: Vignesh Raghavendra <[email protected]>
Cc: Maxime Coquelin <[email protected]>
Cc: Alexandre Torgue <[email protected]>
Cc: Philipp Zabel <[email protected]>
Cc: Christophe Kerello <[email protected]>
Cc: Cai Huoqing <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Jack Wang <[email protected]>
---
drivers/mtd/nand/raw/marvell_nand.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/mtd/nand/raw/marvell_nand.c b/drivers/mtd/nand/raw/marvell_nand.c
index 2455a581fd70..d9f2f1d0b5ef 100644
--- a/drivers/mtd/nand/raw/marvell_nand.c
+++ b/drivers/mtd/nand/raw/marvell_nand.c
@@ -865,13 +865,19 @@ static int marvell_nfc_xfer_data_dma(struct marvell_nfc *nfc,
marvell_nfc_enable_dma(nfc);
/* Prepare the DMA transfer */
sg_init_one(&sg, nfc->dma_buf, dma_len);
- dma_map_sg(nfc->dma_chan->device->dev, &sg, 1, direction);
+ ret = dma_map_sg(nfc->dma_chan->device->dev, &sg, 1, direction);
+ if (!ret) {
+ dev_err(nfc->dev, "Could not map DMA S/G list\n");
+ return -ENXIO;
+ }
+
tx = dmaengine_prep_slave_sg(nfc->dma_chan, &sg, 1,
direction == DMA_FROM_DEVICE ?
DMA_DEV_TO_MEM : DMA_MEM_TO_DEV,
DMA_PREP_INTERRUPT);
if (!tx) {
dev_err(nfc->dev, "Could not prepare DMA S/G list\n");
+ dma_unmap_sg(nfc->dma_chan->device->dev, &sg, 1, direction);
return -ENXIO;
}
--
2.34.1
Add the missing error check for dma_map_sg.
Cc: "James E.J. Bottomley" <[email protected]>
Cc: "Martin K. Petersen" <[email protected]>
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Jack Wang <[email protected]>
---
drivers/scsi/qlogicpti.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/qlogicpti.c b/drivers/scsi/qlogicpti.c
index 57f2f4135a06..8c961ff03fcd 100644
--- a/drivers/scsi/qlogicpti.c
+++ b/drivers/scsi/qlogicpti.c
@@ -909,7 +909,8 @@ static inline int load_cmd(struct scsi_cmnd *Cmnd, struct Command_Entry *cmd,
sg_count = dma_map_sg(&qpti->op->dev, sg,
scsi_sg_count(Cmnd),
Cmnd->sc_data_direction);
-
+ if (!sg_count)
+ return -1;
ds = cmd->dataseg;
cmd->segment_cnt = sg_count;
--
2.34.1
dma_map_sg return 0 on error, in case of error return -EIO
to caller.
Cc: Sebastian Reichel <[email protected]>
Cc: [email protected] (open list)
Signed-off-by: Jack Wang <[email protected]>
---
drivers/hsi/controllers/omap_ssi_port.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/hsi/controllers/omap_ssi_port.c b/drivers/hsi/controllers/omap_ssi_port.c
index a0cb5be246e1..b9495b720f1b 100644
--- a/drivers/hsi/controllers/omap_ssi_port.c
+++ b/drivers/hsi/controllers/omap_ssi_port.c
@@ -230,10 +230,10 @@ static int ssi_start_dma(struct hsi_msg *msg, int lch)
if (msg->ttype == HSI_MSG_READ) {
err = dma_map_sg(&ssi->device, msg->sgt.sgl, msg->sgt.nents,
DMA_FROM_DEVICE);
- if (err < 0) {
+ if (!err) {
dev_dbg(&ssi->device, "DMA map SG failed !\n");
pm_runtime_put_autosuspend(omap_port->pdev);
- return err;
+ return -EIO;
}
csdp = SSI_DST_BURST_4x32_BIT | SSI_DST_MEMORY_PORT |
SSI_SRC_SINGLE_ACCESS0 | SSI_SRC_PERIPHERAL_PORT |
@@ -247,10 +247,10 @@ static int ssi_start_dma(struct hsi_msg *msg, int lch)
} else {
err = dma_map_sg(&ssi->device, msg->sgt.sgl, msg->sgt.nents,
DMA_TO_DEVICE);
- if (err < 0) {
+ if (!err) {
dev_dbg(&ssi->device, "DMA map SG failed !\n");
pm_runtime_put_autosuspend(omap_port->pdev);
- return err;
+ return -EIO;
}
csdp = SSI_SRC_BURST_4x32_BIT | SSI_SRC_MEMORY_PORT |
SSI_DST_SINGLE_ACCESS0 | SSI_DST_PERIPHERAL_PORT |
--
2.34.1
dma_map_sg return 0 on error.
Cc: Alexander Shishkin <[email protected]>
Cc: [email protected]
Signed-off-by: Jack Wang <[email protected]>
---
drivers/hwtracing/intel_th/msu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hwtracing/intel_th/msu.c b/drivers/hwtracing/intel_th/msu.c
index 6c8215a47a60..b49237d56a60 100644
--- a/drivers/hwtracing/intel_th/msu.c
+++ b/drivers/hwtracing/intel_th/msu.c
@@ -931,7 +931,7 @@ static int msc_buffer_contig_alloc(struct msc *msc, unsigned long size)
ret = dma_map_sg(msc_dev(msc)->parent->parent, msc->single_sgt.sgl, 1,
DMA_FROM_DEVICE);
- if (ret < 0)
+ if (!ret)
goto err_free_pages;
msc->nr_pages = nr_pages;
--
2.34.1
dma_map_sg return 0 on error, fix the error check, and return -EIO
to caller.
Cc: Jassi Brar <[email protected]>
Cc: [email protected]
Signed-off-by: Jack Wang <[email protected]>
---
drivers/mailbox/bcm-flexrm-mailbox.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/mailbox/bcm-flexrm-mailbox.c b/drivers/mailbox/bcm-flexrm-mailbox.c
index fda16f76401e..bf6e86b0ed09 100644
--- a/drivers/mailbox/bcm-flexrm-mailbox.c
+++ b/drivers/mailbox/bcm-flexrm-mailbox.c
@@ -622,15 +622,15 @@ static int flexrm_spu_dma_map(struct device *dev, struct brcm_message *msg)
rc = dma_map_sg(dev, msg->spu.src, sg_nents(msg->spu.src),
DMA_TO_DEVICE);
- if (rc < 0)
- return rc;
+ if (!rc)
+ return -EIO;
rc = dma_map_sg(dev, msg->spu.dst, sg_nents(msg->spu.dst),
DMA_FROM_DEVICE);
- if (rc < 0) {
+ if (!rc) {
dma_unmap_sg(dev, msg->spu.src, sg_nents(msg->spu.src),
DMA_TO_DEVICE);
- return rc;
+ return -EIO;
}
return 0;
--
2.34.1
dma_map_sg return 0 on error, in case of error return -EIO.
Cc: Miquel Raynal <[email protected]>
Cc: Richard Weinberger <[email protected]>
Cc: Vignesh Raghavendra <[email protected]>
Cc: Maxime Coquelin <[email protected]>
Cc: Alexandre Torgue <[email protected]>
Cc: Philipp Zabel <[email protected]>
Cc: Christophe Kerello <[email protected]>
Cc: Cai Huoqing <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Jack Wang <[email protected]>
---
drivers/mtd/nand/raw/stm32_fmc2_nand.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/mtd/nand/raw/stm32_fmc2_nand.c b/drivers/mtd/nand/raw/stm32_fmc2_nand.c
index 87c1c7dd97eb..a0c825af19fa 100644
--- a/drivers/mtd/nand/raw/stm32_fmc2_nand.c
+++ b/drivers/mtd/nand/raw/stm32_fmc2_nand.c
@@ -862,8 +862,8 @@ static int stm32_fmc2_nfc_xfer(struct nand_chip *chip, const u8 *buf,
ret = dma_map_sg(nfc->dev, nfc->dma_data_sg.sgl,
eccsteps, dma_data_dir);
- if (ret < 0)
- return ret;
+ if (!ret)
+ return -EIO;
desc_data = dmaengine_prep_slave_sg(dma_ch, nfc->dma_data_sg.sgl,
eccsteps, dma_transfer_dir,
@@ -893,8 +893,10 @@ static int stm32_fmc2_nfc_xfer(struct nand_chip *chip, const u8 *buf,
ret = dma_map_sg(nfc->dev, nfc->dma_ecc_sg.sgl,
eccsteps, dma_data_dir);
- if (ret < 0)
+ if (!ret) {
+ ret = -EIO;
goto err_unmap_data;
+ }
desc_ecc = dmaengine_prep_slave_sg(nfc->dma_ecc_ch,
nfc->dma_ecc_sg.sgl,
--
2.34.1
dma_map_sg return 0 on error, in case of error set
EIO as return code.
Cc: Jason Gunthorpe <[email protected]>
Cc: Leon Romanovsky <[email protected]>
Cc: Christophe JAILLET <[email protected]>
Cc: Kees Cook <[email protected]>
Cc: "Håkon Bugge" <[email protected]>
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Jack Wang <[email protected]>
---
drivers/infiniband/hw/mthca/mthca_memfree.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/infiniband/hw/mthca/mthca_memfree.c b/drivers/infiniband/hw/mthca/mthca_memfree.c
index f2734a5c5f26..44fd5fdf64d5 100644
--- a/drivers/infiniband/hw/mthca/mthca_memfree.c
+++ b/drivers/infiniband/hw/mthca/mthca_memfree.c
@@ -189,7 +189,7 @@ struct mthca_icm *mthca_alloc_icm(struct mthca_dev *dev, int npages,
chunk->npages,
DMA_BIDIRECTIONAL);
- if (chunk->nsg <= 0)
+ if (!chunk->nsg)
goto fail;
}
@@ -208,7 +208,7 @@ struct mthca_icm *mthca_alloc_icm(struct mthca_dev *dev, int npages,
chunk->nsg = dma_map_sg(&dev->pdev->dev, chunk->mem,
chunk->npages, DMA_BIDIRECTIONAL);
- if (chunk->nsg <= 0)
+ if (!chunk->nsg)
goto fail;
}
@@ -482,8 +482,9 @@ int mthca_map_user_db(struct mthca_dev *dev, struct mthca_uar *uar,
ret = dma_map_sg(&dev->pdev->dev, &db_tab->page[i].mem, 1,
DMA_TO_DEVICE);
- if (ret < 0) {
+ if (!ret) {
unpin_user_page(pages[0]);
+ ret = -EIO;
goto out;
}
--
2.34.1
On Fri, Aug 19, 2022 at 08:07:44AM +0200, Jack Wang wrote:
> dma_map_sg return 0 on error, in case of error set
> EIO as return code.
>
> Cc: Jason Gunthorpe <[email protected]>
> Cc: Leon Romanovsky <[email protected]>
> Cc: Christophe JAILLET <[email protected]>
> Cc: Kees Cook <[email protected]>
> Cc: "H?kon Bugge" <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Jack Wang <[email protected]>
Reviewed-by: Kees Cook <[email protected]>
--
Kees Cook
Hello Jack,
first of all: thank you for this patch!
On Fri, Aug 19, 2022 at 8:08 AM Jack Wang <[email protected]> wrote:
[...]
> @@ -388,7 +388,7 @@ static int meson_mx_sdhc_map_dma(struct mmc_host *mmc, struct mmc_request *mrq)
>
> dma_len = dma_map_sg(mmc_dev(mmc), data->sg, data->sg_len,
Does it also make sense to change the type of the dma_len variable
from (signed) int to "unsigned int" (a few lines above)?
Best regards,
Martin
Please don't send me just two random patches out of a series, as I
have no way to review them. If the patches are independent, send them
independently and if they depend on common prep work send the entire
series to everyone.
On Fri, Aug 19, 2022 at 08:07:44AM +0200, Jack Wang wrote:
> dma_map_sg return 0 on error, in case of error set
> EIO as return code.
>
> Cc: Jason Gunthorpe <[email protected]>
> Cc: Leon Romanovsky <[email protected]>
> Cc: Christophe JAILLET <[email protected]>
> Cc: Kees Cook <[email protected]>
> Cc: "H?kon Bugge" <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Jack Wang <[email protected]>
> ---
> drivers/infiniband/hw/mthca/mthca_memfree.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/infiniband/hw/mthca/mthca_memfree.c b/drivers/infiniband/hw/mthca/mthca_memfree.c
> index f2734a5c5f26..44fd5fdf64d5 100644
> --- a/drivers/infiniband/hw/mthca/mthca_memfree.c
> +++ b/drivers/infiniband/hw/mthca/mthca_memfree.c
> @@ -189,7 +189,7 @@ struct mthca_icm *mthca_alloc_icm(struct mthca_dev *dev, int npages,
> chunk->npages,
> DMA_BIDIRECTIONAL);
>
> - if (chunk->nsg <= 0)
> + if (!chunk->nsg)
> goto fail;
> }
>
> @@ -208,7 +208,7 @@ struct mthca_icm *mthca_alloc_icm(struct mthca_dev *dev, int npages,
> chunk->nsg = dma_map_sg(&dev->pdev->dev, chunk->mem,
> chunk->npages, DMA_BIDIRECTIONAL);
>
> - if (chunk->nsg <= 0)
> + if (!chunk->nsg)
> goto fail;
> }
>
> @@ -482,8 +482,9 @@ int mthca_map_user_db(struct mthca_dev *dev, struct mthca_uar *uar,
>
> ret = dma_map_sg(&dev->pdev->dev, &db_tab->page[i].mem, 1,
> DMA_TO_DEVICE);
> - if (ret < 0) {
> + if (!ret) {
This code is not equivalent to original code. mthca didn't count ret == 0
as an error. Most likely, it is a bug, but I don't want to change old and
unmaintained driver without any real need.
Thanks
> unpin_user_page(pages[0]);
> + ret = -EIO;
> goto out;
> }
>
> --
> 2.34.1
>
On Fri, Aug 19, 2022 at 08:08:01AM +0200, Jack Wang wrote:
> dma_map_sg return 0 on error.
>
> Cc: Tariq Toukan <[email protected]>
> Cc: "David S. Miller" <[email protected]>
> Cc: Eric Dumazet <[email protected]>
> Cc: Jakub Kicinski <[email protected]>
> Cc: Paolo Abeni <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
>
> Signed-off-by: Jack Wang <[email protected]>
> ---
> drivers/net/ethernet/mellanox/mlx4/icm.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
Thanks,
Reviewed-by: Leon Romanovsky <[email protected]>
On Fri, Aug 19, 2022 at 08:07:44AM +0200, Jack Wang wrote:
> dma_map_sg return 0 on error, in case of error set
> EIO as return code.
>
The first two chunks are just cleanups but the third one is a bug fix
so it needs a Fixes tag.
regards,
dan carpenter
On Sun, Aug 21, 2022 at 7:57 AM Christoph Hellwig <[email protected]> wrote:
>
> Please don't send me just two random patches out of a series, as I
> have no way to review them. If the patches are independent, send them
> independently and if they depend on common prep work send the entire
> series to everyone.
Hi Christoph,
As mentioned in the cover letter, the bugfixes are all independent,
for each of the drivers.
I will wait a bit to see if there is other comments, resend as you
suggested later.
Thx!
On Fri, Aug 19, 2022 at 08:07:43AM +0200, Jack Wang wrote:
> dma_map_sg return 0 on error, fix the error check, and return -EIO
> to caller.
>
> Cc: Jassi Brar <[email protected]>
> Cc: [email protected]
> Signed-off-by: Jack Wang <[email protected]>
This needs a Fixes tag.
Fixes: dbc049eee730 ("mailbox: Add driver for Broadcom FlexRM ring manager")
regards,
dan carpenter
Hi Martin,
On Sat, Aug 20, 2022 at 7:26 PM Martin Blumenstingl
<[email protected]> wrote:
>
> Hello Jack,
>
> first of all: thank you for this patch!
welcome.
>
> On Fri, Aug 19, 2022 at 8:08 AM Jack Wang <[email protected]> wrote:
> [...]
> > @@ -388,7 +388,7 @@ static int meson_mx_sdhc_map_dma(struct mmc_host *mmc, struct mmc_request *mrq)
> >
> > dma_len = dma_map_sg(mmc_dev(mmc), data->sg, data->sg_len,
> Does it also make sense to change the type of the dma_len variable
> from (signed) int to "unsigned int" (a few lines above)?
I double checked, it seems a good idea, I will do it in v2.
>
>
> Best regards,
> Martin
Thx!
On Mon, Aug 22, 2022 at 07:12:23AM +0200, Jinpu Wang wrote:
> On Sun, Aug 21, 2022 at 7:57 AM Christoph Hellwig <[email protected]> wrote:
> >
> > Please don't send me just two random patches out of a series, as I
> > have no way to review them. If the patches are independent, send them
> > independently and if they depend on common prep work send the entire
> > series to everyone.
> Hi Christoph,
>
> As mentioned in the cover letter, the bugfixes are all independent,
> for each of the drivers.
I still haven't seen the cover letter as it was never sent to me.
But if patches are independent please send a series per subsystem
as that makes that very clear.
On Mon, Aug 22, 2022 at 8:25 AM Christoph Hellwig <[email protected]> wrote:
>
> On Mon, Aug 22, 2022 at 07:12:23AM +0200, Jinpu Wang wrote:
> > On Sun, Aug 21, 2022 at 7:57 AM Christoph Hellwig <[email protected]> wrote:
> > >
> > > Please don't send me just two random patches out of a series, as I
> > > have no way to review them. If the patches are independent, send them
> > > independently and if they depend on common prep work send the entire
> > > series to everyone.
> > Hi Christoph,
> >
> > As mentioned in the cover letter, the bugfixes are all independent,
> > for each of the drivers.
>
> I still haven't seen the cover letter as it was never sent to me.
> But if patches are independent please send a series per subsystem
> as that makes that very clear.
Thanks for the hint, this definitely helps the subsystem maintainer to
get a clear background.
I'll do it in the future.
On Mon, Aug 22, 2022 at 7:07 AM Dan Carpenter <[email protected]> wrote:
>
> On Fri, Aug 19, 2022 at 08:07:43AM +0200, Jack Wang wrote:
> > dma_map_sg return 0 on error, fix the error check, and return -EIO
> > to caller.
> >
> > Cc: Jassi Brar <[email protected]>
> > Cc: [email protected]
> > Signed-off-by: Jack Wang <[email protected]>
>
> This needs a Fixes tag.
>
> Fixes: dbc049eee730 ("mailbox: Add driver for Broadcom FlexRM ring manager")
Will add in the next version.
Thank you!
>
> regards,
> dan carpenter
>
On Fri, 19 Aug 2022 08:08:01 +0200 Jack Wang wrote:
> dma_map_sg return 0 on error.
You need to resend it as an individual patch, not part of a series if
you want it to be applied to the networking tree. Please keep Leon's
review tag.
Hi Jack,
On 8/19/22 08:07, Jack Wang wrote:
> dma_map_sg return 0 on error, in case of error return -EIO.
>
> Cc: Miquel Raynal <[email protected]>
> Cc: Richard Weinberger <[email protected]>
> Cc: Vignesh Raghavendra <[email protected]>
> Cc: Maxime Coquelin <[email protected]>
> Cc: Alexandre Torgue <[email protected]>
> Cc: Philipp Zabel <[email protected]>
> Cc: Christophe Kerello <[email protected]>
> Cc: Cai Huoqing <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Jack Wang <[email protected]>
> ---
> drivers/mtd/nand/raw/stm32_fmc2_nand.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/stm32_fmc2_nand.c b/drivers/mtd/nand/raw/stm32_fmc2_nand.c
> index 87c1c7dd97eb..a0c825af19fa 100644
> --- a/drivers/mtd/nand/raw/stm32_fmc2_nand.c
> +++ b/drivers/mtd/nand/raw/stm32_fmc2_nand.c
> @@ -862,8 +862,8 @@ static int stm32_fmc2_nfc_xfer(struct nand_chip *chip, const u8 *buf,
>
> ret = dma_map_sg(nfc->dev, nfc->dma_data_sg.sgl,
> eccsteps, dma_data_dir);
> - if (ret < 0)
> - return ret;
> + if (!ret)
> + return -EIO;
>
> desc_data = dmaengine_prep_slave_sg(dma_ch, nfc->dma_data_sg.sgl,
> eccsteps, dma_transfer_dir,
> @@ -893,8 +893,10 @@ static int stm32_fmc2_nfc_xfer(struct nand_chip *chip, const u8 *buf,
>
> ret = dma_map_sg(nfc->dev, nfc->dma_ecc_sg.sgl,
> eccsteps, dma_data_dir);
> - if (ret < 0)
> + if (!ret) {
> + ret = -EIO;
> goto err_unmap_data;
> + }
>
> desc_ecc = dmaengine_prep_slave_sg(nfc->dma_ecc_ch,
> nfc->dma_ecc_sg.sgl,
Reviewed-by: Christophe Kerello <[email protected]>
Regards,
Christophe Kerello.
On Mon, Aug 22, 2022 at 8:20 PM Jakub Kicinski <[email protected]> wrote:
>
> On Fri, 19 Aug 2022 08:08:01 +0200 Jack Wang wrote:
> > dma_map_sg return 0 on error.
>
> You need to resend it as an individual patch, not part of a series if
> you want it to be applied to the networking tree. Please keep Leon's
> review tag.
Got it. Will do
On Fri, 2022-08-19 at 06:07:46 UTC, Jack Wang wrote:
> dma_map_sg return 0 on error, in case of error return -EIO.
>
> Cc: Miquel Raynal <[email protected]>
> Cc: Richard Weinberger <[email protected]>
> Cc: Vignesh Raghavendra <[email protected]>
> Cc: Maxime Coquelin <[email protected]>
> Cc: Alexandre Torgue <[email protected]>
> Cc: Philipp Zabel <[email protected]>
> Cc: Christophe Kerello <[email protected]>
> Cc: Cai Huoqing <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Jack Wang <[email protected]>
> Reviewed-by: Christophe Kerello <[email protected]>
Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git nand/next, thanks.
Miquel
On Fri, 2022-08-19 at 06:07:47 UTC, Jack Wang wrote:
> dma_map_sg return 0 on error, in case of error return -EIO,
> also add the dma_unmap_sg as rollback on the following error.
>
> Cc: Miquel Raynal <[email protected]>
> Cc: Richard Weinberger <[email protected]>
> Cc: Vignesh Raghavendra <[email protected]>
> Cc: Maxime Coquelin <[email protected]>
> Cc: Alexandre Torgue <[email protected]>
> Cc: Philipp Zabel <[email protected]>
> Cc: Christophe Kerello <[email protected]>
> Cc: Cai Huoqing <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
>
> Signed-off-by: Jack Wang <[email protected]>
Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git nand/next, thanks.
Miquel