* [PATCH v2 0/2] Fixes NULL pointer access in spi-mt65xx.c
@ 2024-03-21 7:08 ` Fei Shao
0 siblings, 0 replies; 15+ messages in thread
From: Fei Shao @ 2024-03-21 7:08 UTC (permalink / raw
To: Mark Brown, AngeloGioacchino Del Regno
Cc: Fei Shao, Daniel Kurtz, Matthias Brugger, linux-arm-kernel,
linux-kernel, linux-mediatek, linux-spi
Hi,
This series contains two patches for spi-mt65xx.c, both focusing on its
interrupt handler mtk_spi_interrupt().
The first patch is to fix a NULL pointer access in the interrupt
handler, which is first found on a MT8186 Chromebook device when the
system tries to establish host communication with its embedded
controller.
The second one is a decorative clean-up when I'm working on the first
patch, which simply renames a variable to better follow the rest of the
code.
I put this after the first fix because I think that will make
maintainers and users slightly easier to only backport the fix if
needed.
Looking forward to any feedback, thank you.
Regards,
Fei
Changes in v2:
- Restore a missing curly brace being dropped during rebase
- Fix a typo in commit message (trans, not xfer)
Fei Shao (2):
spi: spi-mt65xx: Fix NULL pointer access in interrupt handler
spi: spi-mt65xx: Rename a variable in interrupt handler
drivers/spi/spi-mt65xx.c | 48 ++++++++++++++++++++--------------------
1 file changed, 24 insertions(+), 24 deletions(-)
--
2.44.0.396.g6e790dbe36-goog
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 0/2] Fixes NULL pointer access in spi-mt65xx.c
@ 2024-03-21 7:08 ` Fei Shao
0 siblings, 0 replies; 15+ messages in thread
From: Fei Shao @ 2024-03-21 7:08 UTC (permalink / raw
To: Mark Brown, AngeloGioacchino Del Regno
Cc: Fei Shao, Daniel Kurtz, Matthias Brugger, linux-arm-kernel,
linux-kernel, linux-mediatek, linux-spi
Hi,
This series contains two patches for spi-mt65xx.c, both focusing on its
interrupt handler mtk_spi_interrupt().
The first patch is to fix a NULL pointer access in the interrupt
handler, which is first found on a MT8186 Chromebook device when the
system tries to establish host communication with its embedded
controller.
The second one is a decorative clean-up when I'm working on the first
patch, which simply renames a variable to better follow the rest of the
code.
I put this after the first fix because I think that will make
maintainers and users slightly easier to only backport the fix if
needed.
Looking forward to any feedback, thank you.
Regards,
Fei
Changes in v2:
- Restore a missing curly brace being dropped during rebase
- Fix a typo in commit message (trans, not xfer)
Fei Shao (2):
spi: spi-mt65xx: Fix NULL pointer access in interrupt handler
spi: spi-mt65xx: Rename a variable in interrupt handler
drivers/spi/spi-mt65xx.c | 48 ++++++++++++++++++++--------------------
1 file changed, 24 insertions(+), 24 deletions(-)
--
2.44.0.396.g6e790dbe36-goog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 0/2] Fixes NULL pointer access in spi-mt65xx.c
@ 2024-03-21 7:08 ` Fei Shao
0 siblings, 0 replies; 15+ messages in thread
From: Fei Shao @ 2024-03-21 7:08 UTC (permalink / raw
To: Mark Brown, AngeloGioacchino Del Regno
Cc: linux-kernel, Daniel Kurtz, linux-spi, linux-mediatek,
Matthias Brugger, linux-arm-kernel
Hi,
This series contains two patches for spi-mt65xx.c, both focusing on its
interrupt handler mtk_spi_interrupt().
The first patch is to fix a NULL pointer access in the interrupt
handler, which is first found on a MT8186 Chromebook device when the
system tries to establish host communication with its embedded
controller.
The second one is a decorative clean-up when I'm working on the first
patch, which simply renames a variable to better follow the rest of the
code.
I put this after the first fix because I think that will make
maintainers and users slightly easier to only backport the fix if
needed.
Looking forward to any feedback, thank you.
Regards,
Fei
Changes in v2:
- Restore a missing curly brace being dropped during rebase
- Fix a typo in commit message (trans, not xfer)
Fei Shao (2):
spi: spi-mt65xx: Fix NULL pointer access in interrupt handler
spi: spi-mt65xx: Rename a variable in interrupt handler
drivers/spi/spi-mt65xx.c | 48 ++++++++++++++++++++--------------------
1 file changed, 24 insertions(+), 24 deletions(-)
--
2.44.0.396.g6e790dbe36-goog
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 1/2] spi: spi-mt65xx: Fix NULL pointer access in interrupt handler
2024-03-21 7:08 ` Fei Shao
(?)
@ 2024-03-21 7:08 ` Fei Shao
-1 siblings, 0 replies; 15+ messages in thread
From: Fei Shao @ 2024-03-21 7:08 UTC (permalink / raw
To: Mark Brown, AngeloGioacchino Del Regno
Cc: Fei Shao, Daniel Kurtz, Matthias Brugger, linux-arm-kernel,
linux-kernel, linux-mediatek, linux-spi
The TX buffer in spi_transfer can be a NULL pointer, so the interrupt
handler may end up writing to the invalid memory and cause crashes.
Add a check to trans->tx_buf before using it.
Fixes: 1ce24864bff4 ("spi: mediatek: Only do dma for 4-byte aligned buffers")
Signed-off-by: Fei Shao <fshao@chromium.org>
---
Changes in v2:
- Restore a missing curly brace being dropped during rebase
- Fix a typo in commit message (trans, not xfer)
drivers/spi/spi-mt65xx.c | 22 ++++++++++++----------
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/drivers/spi/spi-mt65xx.c b/drivers/spi/spi-mt65xx.c
index 8d4633b353ee..e4cb22fe0075 100644
--- a/drivers/spi/spi-mt65xx.c
+++ b/drivers/spi/spi-mt65xx.c
@@ -788,17 +788,19 @@ static irqreturn_t mtk_spi_interrupt(int irq, void *dev_id)
mdata->xfer_len = min(MTK_SPI_MAX_FIFO_SIZE, len);
mtk_spi_setup_packet(host);
- cnt = mdata->xfer_len / 4;
- iowrite32_rep(mdata->base + SPI_TX_DATA_REG,
- trans->tx_buf + mdata->num_xfered, cnt);
+ if (trans->tx_buf) {
+ cnt = mdata->xfer_len / 4;
+ iowrite32_rep(mdata->base + SPI_TX_DATA_REG,
+ trans->tx_buf + mdata->num_xfered, cnt);
- remainder = mdata->xfer_len % 4;
- if (remainder > 0) {
- reg_val = 0;
- memcpy(®_val,
- trans->tx_buf + (cnt * 4) + mdata->num_xfered,
- remainder);
- writel(reg_val, mdata->base + SPI_TX_DATA_REG);
+ remainder = mdata->xfer_len % 4;
+ if (remainder > 0) {
+ reg_val = 0;
+ memcpy(®_val,
+ trans->tx_buf + (cnt * 4) + mdata->num_xfered,
+ remainder);
+ writel(reg_val, mdata->base + SPI_TX_DATA_REG);
+ }
}
mtk_spi_enable_transfer(host);
--
2.44.0.396.g6e790dbe36-goog
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 1/2] spi: spi-mt65xx: Fix NULL pointer access in interrupt handler
@ 2024-03-21 7:08 ` Fei Shao
0 siblings, 0 replies; 15+ messages in thread
From: Fei Shao @ 2024-03-21 7:08 UTC (permalink / raw
To: Mark Brown, AngeloGioacchino Del Regno
Cc: linux-kernel, Daniel Kurtz, linux-spi, linux-mediatek,
Matthias Brugger, linux-arm-kernel
The TX buffer in spi_transfer can be a NULL pointer, so the interrupt
handler may end up writing to the invalid memory and cause crashes.
Add a check to trans->tx_buf before using it.
Fixes: 1ce24864bff4 ("spi: mediatek: Only do dma for 4-byte aligned buffers")
Signed-off-by: Fei Shao <fshao@chromium.org>
---
Changes in v2:
- Restore a missing curly brace being dropped during rebase
- Fix a typo in commit message (trans, not xfer)
drivers/spi/spi-mt65xx.c | 22 ++++++++++++----------
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/drivers/spi/spi-mt65xx.c b/drivers/spi/spi-mt65xx.c
index 8d4633b353ee..e4cb22fe0075 100644
--- a/drivers/spi/spi-mt65xx.c
+++ b/drivers/spi/spi-mt65xx.c
@@ -788,17 +788,19 @@ static irqreturn_t mtk_spi_interrupt(int irq, void *dev_id)
mdata->xfer_len = min(MTK_SPI_MAX_FIFO_SIZE, len);
mtk_spi_setup_packet(host);
- cnt = mdata->xfer_len / 4;
- iowrite32_rep(mdata->base + SPI_TX_DATA_REG,
- trans->tx_buf + mdata->num_xfered, cnt);
+ if (trans->tx_buf) {
+ cnt = mdata->xfer_len / 4;
+ iowrite32_rep(mdata->base + SPI_TX_DATA_REG,
+ trans->tx_buf + mdata->num_xfered, cnt);
- remainder = mdata->xfer_len % 4;
- if (remainder > 0) {
- reg_val = 0;
- memcpy(®_val,
- trans->tx_buf + (cnt * 4) + mdata->num_xfered,
- remainder);
- writel(reg_val, mdata->base + SPI_TX_DATA_REG);
+ remainder = mdata->xfer_len % 4;
+ if (remainder > 0) {
+ reg_val = 0;
+ memcpy(®_val,
+ trans->tx_buf + (cnt * 4) + mdata->num_xfered,
+ remainder);
+ writel(reg_val, mdata->base + SPI_TX_DATA_REG);
+ }
}
mtk_spi_enable_transfer(host);
--
2.44.0.396.g6e790dbe36-goog
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 1/2] spi: spi-mt65xx: Fix NULL pointer access in interrupt handler
@ 2024-03-21 7:08 ` Fei Shao
0 siblings, 0 replies; 15+ messages in thread
From: Fei Shao @ 2024-03-21 7:08 UTC (permalink / raw
To: Mark Brown, AngeloGioacchino Del Regno
Cc: Fei Shao, Daniel Kurtz, Matthias Brugger, linux-arm-kernel,
linux-kernel, linux-mediatek, linux-spi
The TX buffer in spi_transfer can be a NULL pointer, so the interrupt
handler may end up writing to the invalid memory and cause crashes.
Add a check to trans->tx_buf before using it.
Fixes: 1ce24864bff4 ("spi: mediatek: Only do dma for 4-byte aligned buffers")
Signed-off-by: Fei Shao <fshao@chromium.org>
---
Changes in v2:
- Restore a missing curly brace being dropped during rebase
- Fix a typo in commit message (trans, not xfer)
drivers/spi/spi-mt65xx.c | 22 ++++++++++++----------
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/drivers/spi/spi-mt65xx.c b/drivers/spi/spi-mt65xx.c
index 8d4633b353ee..e4cb22fe0075 100644
--- a/drivers/spi/spi-mt65xx.c
+++ b/drivers/spi/spi-mt65xx.c
@@ -788,17 +788,19 @@ static irqreturn_t mtk_spi_interrupt(int irq, void *dev_id)
mdata->xfer_len = min(MTK_SPI_MAX_FIFO_SIZE, len);
mtk_spi_setup_packet(host);
- cnt = mdata->xfer_len / 4;
- iowrite32_rep(mdata->base + SPI_TX_DATA_REG,
- trans->tx_buf + mdata->num_xfered, cnt);
+ if (trans->tx_buf) {
+ cnt = mdata->xfer_len / 4;
+ iowrite32_rep(mdata->base + SPI_TX_DATA_REG,
+ trans->tx_buf + mdata->num_xfered, cnt);
- remainder = mdata->xfer_len % 4;
- if (remainder > 0) {
- reg_val = 0;
- memcpy(®_val,
- trans->tx_buf + (cnt * 4) + mdata->num_xfered,
- remainder);
- writel(reg_val, mdata->base + SPI_TX_DATA_REG);
+ remainder = mdata->xfer_len % 4;
+ if (remainder > 0) {
+ reg_val = 0;
+ memcpy(®_val,
+ trans->tx_buf + (cnt * 4) + mdata->num_xfered,
+ remainder);
+ writel(reg_val, mdata->base + SPI_TX_DATA_REG);
+ }
}
mtk_spi_enable_transfer(host);
--
2.44.0.396.g6e790dbe36-goog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 2/2] spi: spi-mt65xx: Rename a variable in interrupt handler
2024-03-21 7:08 ` Fei Shao
(?)
@ 2024-03-21 7:08 ` Fei Shao
-1 siblings, 0 replies; 15+ messages in thread
From: Fei Shao @ 2024-03-21 7:08 UTC (permalink / raw
To: Mark Brown, AngeloGioacchino Del Regno
Cc: Fei Shao, Matthias Brugger, linux-arm-kernel, linux-kernel,
linux-mediatek, linux-spi
All the spi_transfer variables in this file use the name "xfer" except
the one in mtk_spi_interrupt(). Align the naming for consistency and
easier searching.
While at it, reformat one memcpy() usage since the coding style allows
100 column lines today.
This commit has no functional change.
Signed-off-by: Fei Shao <fshao@chromium.org>
---
(no changes since v1)
drivers/spi/spi-mt65xx.c | 32 +++++++++++++++-----------------
1 file changed, 15 insertions(+), 17 deletions(-)
diff --git a/drivers/spi/spi-mt65xx.c b/drivers/spi/spi-mt65xx.c
index e4cb22fe0075..36c2f52cd6b8 100644
--- a/drivers/spi/spi-mt65xx.c
+++ b/drivers/spi/spi-mt65xx.c
@@ -748,7 +748,7 @@ static irqreturn_t mtk_spi_interrupt(int irq, void *dev_id)
u32 cmd, reg_val, cnt, remainder, len;
struct spi_controller *host = dev_id;
struct mtk_spi *mdata = spi_controller_get_devdata(host);
- struct spi_transfer *trans = mdata->cur_transfer;
+ struct spi_transfer *xfer = mdata->cur_transfer;
reg_val = readl(mdata->base + SPI_STATUS0_REG);
if (reg_val & MTK_SPI_PAUSE_INT_STATUS)
@@ -762,42 +762,40 @@ static irqreturn_t mtk_spi_interrupt(int irq, void *dev_id)
return IRQ_HANDLED;
}
- if (!host->can_dma(host, NULL, trans)) {
- if (trans->rx_buf) {
+ if (!host->can_dma(host, NULL, xfer)) {
+ if (xfer->rx_buf) {
cnt = mdata->xfer_len / 4;
ioread32_rep(mdata->base + SPI_RX_DATA_REG,
- trans->rx_buf + mdata->num_xfered, cnt);
+ xfer->rx_buf + mdata->num_xfered, cnt);
remainder = mdata->xfer_len % 4;
if (remainder > 0) {
reg_val = readl(mdata->base + SPI_RX_DATA_REG);
- memcpy(trans->rx_buf +
- mdata->num_xfered +
- (cnt * 4),
+ memcpy(xfer->rx_buf + (cnt * 4) + mdata->num_xfered,
®_val,
remainder);
}
}
mdata->num_xfered += mdata->xfer_len;
- if (mdata->num_xfered == trans->len) {
+ if (mdata->num_xfered == xfer->len) {
spi_finalize_current_transfer(host);
return IRQ_HANDLED;
}
- len = trans->len - mdata->num_xfered;
+ len = xfer->len - mdata->num_xfered;
mdata->xfer_len = min(MTK_SPI_MAX_FIFO_SIZE, len);
mtk_spi_setup_packet(host);
- if (trans->tx_buf) {
+ if (xfer->tx_buf) {
cnt = mdata->xfer_len / 4;
iowrite32_rep(mdata->base + SPI_TX_DATA_REG,
- trans->tx_buf + mdata->num_xfered, cnt);
+ xfer->tx_buf + mdata->num_xfered, cnt);
remainder = mdata->xfer_len % 4;
if (remainder > 0) {
reg_val = 0;
memcpy(®_val,
- trans->tx_buf + (cnt * 4) + mdata->num_xfered,
+ xfer->tx_buf + (cnt * 4) + mdata->num_xfered,
remainder);
writel(reg_val, mdata->base + SPI_TX_DATA_REG);
}
@@ -809,21 +807,21 @@ static irqreturn_t mtk_spi_interrupt(int irq, void *dev_id)
}
if (mdata->tx_sgl)
- trans->tx_dma += mdata->xfer_len;
+ xfer->tx_dma += mdata->xfer_len;
if (mdata->rx_sgl)
- trans->rx_dma += mdata->xfer_len;
+ xfer->rx_dma += mdata->xfer_len;
if (mdata->tx_sgl && (mdata->tx_sgl_len == 0)) {
mdata->tx_sgl = sg_next(mdata->tx_sgl);
if (mdata->tx_sgl) {
- trans->tx_dma = sg_dma_address(mdata->tx_sgl);
+ xfer->tx_dma = sg_dma_address(mdata->tx_sgl);
mdata->tx_sgl_len = sg_dma_len(mdata->tx_sgl);
}
}
if (mdata->rx_sgl && (mdata->rx_sgl_len == 0)) {
mdata->rx_sgl = sg_next(mdata->rx_sgl);
if (mdata->rx_sgl) {
- trans->rx_dma = sg_dma_address(mdata->rx_sgl);
+ xfer->rx_dma = sg_dma_address(mdata->rx_sgl);
mdata->rx_sgl_len = sg_dma_len(mdata->rx_sgl);
}
}
@@ -841,7 +839,7 @@ static irqreturn_t mtk_spi_interrupt(int irq, void *dev_id)
mtk_spi_update_mdata_len(host);
mtk_spi_setup_packet(host);
- mtk_spi_setup_dma_addr(host, trans);
+ mtk_spi_setup_dma_addr(host, xfer);
mtk_spi_enable_transfer(host);
return IRQ_HANDLED;
--
2.44.0.396.g6e790dbe36-goog
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 2/2] spi: spi-mt65xx: Rename a variable in interrupt handler
@ 2024-03-21 7:08 ` Fei Shao
0 siblings, 0 replies; 15+ messages in thread
From: Fei Shao @ 2024-03-21 7:08 UTC (permalink / raw
To: Mark Brown, AngeloGioacchino Del Regno
Cc: linux-kernel, linux-spi, linux-mediatek, Matthias Brugger,
linux-arm-kernel
All the spi_transfer variables in this file use the name "xfer" except
the one in mtk_spi_interrupt(). Align the naming for consistency and
easier searching.
While at it, reformat one memcpy() usage since the coding style allows
100 column lines today.
This commit has no functional change.
Signed-off-by: Fei Shao <fshao@chromium.org>
---
(no changes since v1)
drivers/spi/spi-mt65xx.c | 32 +++++++++++++++-----------------
1 file changed, 15 insertions(+), 17 deletions(-)
diff --git a/drivers/spi/spi-mt65xx.c b/drivers/spi/spi-mt65xx.c
index e4cb22fe0075..36c2f52cd6b8 100644
--- a/drivers/spi/spi-mt65xx.c
+++ b/drivers/spi/spi-mt65xx.c
@@ -748,7 +748,7 @@ static irqreturn_t mtk_spi_interrupt(int irq, void *dev_id)
u32 cmd, reg_val, cnt, remainder, len;
struct spi_controller *host = dev_id;
struct mtk_spi *mdata = spi_controller_get_devdata(host);
- struct spi_transfer *trans = mdata->cur_transfer;
+ struct spi_transfer *xfer = mdata->cur_transfer;
reg_val = readl(mdata->base + SPI_STATUS0_REG);
if (reg_val & MTK_SPI_PAUSE_INT_STATUS)
@@ -762,42 +762,40 @@ static irqreturn_t mtk_spi_interrupt(int irq, void *dev_id)
return IRQ_HANDLED;
}
- if (!host->can_dma(host, NULL, trans)) {
- if (trans->rx_buf) {
+ if (!host->can_dma(host, NULL, xfer)) {
+ if (xfer->rx_buf) {
cnt = mdata->xfer_len / 4;
ioread32_rep(mdata->base + SPI_RX_DATA_REG,
- trans->rx_buf + mdata->num_xfered, cnt);
+ xfer->rx_buf + mdata->num_xfered, cnt);
remainder = mdata->xfer_len % 4;
if (remainder > 0) {
reg_val = readl(mdata->base + SPI_RX_DATA_REG);
- memcpy(trans->rx_buf +
- mdata->num_xfered +
- (cnt * 4),
+ memcpy(xfer->rx_buf + (cnt * 4) + mdata->num_xfered,
®_val,
remainder);
}
}
mdata->num_xfered += mdata->xfer_len;
- if (mdata->num_xfered == trans->len) {
+ if (mdata->num_xfered == xfer->len) {
spi_finalize_current_transfer(host);
return IRQ_HANDLED;
}
- len = trans->len - mdata->num_xfered;
+ len = xfer->len - mdata->num_xfered;
mdata->xfer_len = min(MTK_SPI_MAX_FIFO_SIZE, len);
mtk_spi_setup_packet(host);
- if (trans->tx_buf) {
+ if (xfer->tx_buf) {
cnt = mdata->xfer_len / 4;
iowrite32_rep(mdata->base + SPI_TX_DATA_REG,
- trans->tx_buf + mdata->num_xfered, cnt);
+ xfer->tx_buf + mdata->num_xfered, cnt);
remainder = mdata->xfer_len % 4;
if (remainder > 0) {
reg_val = 0;
memcpy(®_val,
- trans->tx_buf + (cnt * 4) + mdata->num_xfered,
+ xfer->tx_buf + (cnt * 4) + mdata->num_xfered,
remainder);
writel(reg_val, mdata->base + SPI_TX_DATA_REG);
}
@@ -809,21 +807,21 @@ static irqreturn_t mtk_spi_interrupt(int irq, void *dev_id)
}
if (mdata->tx_sgl)
- trans->tx_dma += mdata->xfer_len;
+ xfer->tx_dma += mdata->xfer_len;
if (mdata->rx_sgl)
- trans->rx_dma += mdata->xfer_len;
+ xfer->rx_dma += mdata->xfer_len;
if (mdata->tx_sgl && (mdata->tx_sgl_len == 0)) {
mdata->tx_sgl = sg_next(mdata->tx_sgl);
if (mdata->tx_sgl) {
- trans->tx_dma = sg_dma_address(mdata->tx_sgl);
+ xfer->tx_dma = sg_dma_address(mdata->tx_sgl);
mdata->tx_sgl_len = sg_dma_len(mdata->tx_sgl);
}
}
if (mdata->rx_sgl && (mdata->rx_sgl_len == 0)) {
mdata->rx_sgl = sg_next(mdata->rx_sgl);
if (mdata->rx_sgl) {
- trans->rx_dma = sg_dma_address(mdata->rx_sgl);
+ xfer->rx_dma = sg_dma_address(mdata->rx_sgl);
mdata->rx_sgl_len = sg_dma_len(mdata->rx_sgl);
}
}
@@ -841,7 +839,7 @@ static irqreturn_t mtk_spi_interrupt(int irq, void *dev_id)
mtk_spi_update_mdata_len(host);
mtk_spi_setup_packet(host);
- mtk_spi_setup_dma_addr(host, trans);
+ mtk_spi_setup_dma_addr(host, xfer);
mtk_spi_enable_transfer(host);
return IRQ_HANDLED;
--
2.44.0.396.g6e790dbe36-goog
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 2/2] spi: spi-mt65xx: Rename a variable in interrupt handler
@ 2024-03-21 7:08 ` Fei Shao
0 siblings, 0 replies; 15+ messages in thread
From: Fei Shao @ 2024-03-21 7:08 UTC (permalink / raw
To: Mark Brown, AngeloGioacchino Del Regno
Cc: Fei Shao, Matthias Brugger, linux-arm-kernel, linux-kernel,
linux-mediatek, linux-spi
All the spi_transfer variables in this file use the name "xfer" except
the one in mtk_spi_interrupt(). Align the naming for consistency and
easier searching.
While at it, reformat one memcpy() usage since the coding style allows
100 column lines today.
This commit has no functional change.
Signed-off-by: Fei Shao <fshao@chromium.org>
---
(no changes since v1)
drivers/spi/spi-mt65xx.c | 32 +++++++++++++++-----------------
1 file changed, 15 insertions(+), 17 deletions(-)
diff --git a/drivers/spi/spi-mt65xx.c b/drivers/spi/spi-mt65xx.c
index e4cb22fe0075..36c2f52cd6b8 100644
--- a/drivers/spi/spi-mt65xx.c
+++ b/drivers/spi/spi-mt65xx.c
@@ -748,7 +748,7 @@ static irqreturn_t mtk_spi_interrupt(int irq, void *dev_id)
u32 cmd, reg_val, cnt, remainder, len;
struct spi_controller *host = dev_id;
struct mtk_spi *mdata = spi_controller_get_devdata(host);
- struct spi_transfer *trans = mdata->cur_transfer;
+ struct spi_transfer *xfer = mdata->cur_transfer;
reg_val = readl(mdata->base + SPI_STATUS0_REG);
if (reg_val & MTK_SPI_PAUSE_INT_STATUS)
@@ -762,42 +762,40 @@ static irqreturn_t mtk_spi_interrupt(int irq, void *dev_id)
return IRQ_HANDLED;
}
- if (!host->can_dma(host, NULL, trans)) {
- if (trans->rx_buf) {
+ if (!host->can_dma(host, NULL, xfer)) {
+ if (xfer->rx_buf) {
cnt = mdata->xfer_len / 4;
ioread32_rep(mdata->base + SPI_RX_DATA_REG,
- trans->rx_buf + mdata->num_xfered, cnt);
+ xfer->rx_buf + mdata->num_xfered, cnt);
remainder = mdata->xfer_len % 4;
if (remainder > 0) {
reg_val = readl(mdata->base + SPI_RX_DATA_REG);
- memcpy(trans->rx_buf +
- mdata->num_xfered +
- (cnt * 4),
+ memcpy(xfer->rx_buf + (cnt * 4) + mdata->num_xfered,
®_val,
remainder);
}
}
mdata->num_xfered += mdata->xfer_len;
- if (mdata->num_xfered == trans->len) {
+ if (mdata->num_xfered == xfer->len) {
spi_finalize_current_transfer(host);
return IRQ_HANDLED;
}
- len = trans->len - mdata->num_xfered;
+ len = xfer->len - mdata->num_xfered;
mdata->xfer_len = min(MTK_SPI_MAX_FIFO_SIZE, len);
mtk_spi_setup_packet(host);
- if (trans->tx_buf) {
+ if (xfer->tx_buf) {
cnt = mdata->xfer_len / 4;
iowrite32_rep(mdata->base + SPI_TX_DATA_REG,
- trans->tx_buf + mdata->num_xfered, cnt);
+ xfer->tx_buf + mdata->num_xfered, cnt);
remainder = mdata->xfer_len % 4;
if (remainder > 0) {
reg_val = 0;
memcpy(®_val,
- trans->tx_buf + (cnt * 4) + mdata->num_xfered,
+ xfer->tx_buf + (cnt * 4) + mdata->num_xfered,
remainder);
writel(reg_val, mdata->base + SPI_TX_DATA_REG);
}
@@ -809,21 +807,21 @@ static irqreturn_t mtk_spi_interrupt(int irq, void *dev_id)
}
if (mdata->tx_sgl)
- trans->tx_dma += mdata->xfer_len;
+ xfer->tx_dma += mdata->xfer_len;
if (mdata->rx_sgl)
- trans->rx_dma += mdata->xfer_len;
+ xfer->rx_dma += mdata->xfer_len;
if (mdata->tx_sgl && (mdata->tx_sgl_len == 0)) {
mdata->tx_sgl = sg_next(mdata->tx_sgl);
if (mdata->tx_sgl) {
- trans->tx_dma = sg_dma_address(mdata->tx_sgl);
+ xfer->tx_dma = sg_dma_address(mdata->tx_sgl);
mdata->tx_sgl_len = sg_dma_len(mdata->tx_sgl);
}
}
if (mdata->rx_sgl && (mdata->rx_sgl_len == 0)) {
mdata->rx_sgl = sg_next(mdata->rx_sgl);
if (mdata->rx_sgl) {
- trans->rx_dma = sg_dma_address(mdata->rx_sgl);
+ xfer->rx_dma = sg_dma_address(mdata->rx_sgl);
mdata->rx_sgl_len = sg_dma_len(mdata->rx_sgl);
}
}
@@ -841,7 +839,7 @@ static irqreturn_t mtk_spi_interrupt(int irq, void *dev_id)
mtk_spi_update_mdata_len(host);
mtk_spi_setup_packet(host);
- mtk_spi_setup_dma_addr(host, trans);
+ mtk_spi_setup_dma_addr(host, xfer);
mtk_spi_enable_transfer(host);
return IRQ_HANDLED;
--
2.44.0.396.g6e790dbe36-goog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/2] spi: spi-mt65xx: Fix NULL pointer access in interrupt handler
2024-03-21 7:08 ` Fei Shao
@ 2024-03-21 8:58 ` AngeloGioacchino Del Regno
-1 siblings, 0 replies; 15+ messages in thread
From: AngeloGioacchino Del Regno @ 2024-03-21 8:58 UTC (permalink / raw
To: Fei Shao, Mark Brown
Cc: Daniel Kurtz, Matthias Brugger, linux-arm-kernel, linux-kernel,
linux-mediatek, linux-spi
Il 21/03/24 08:08, Fei Shao ha scritto:
> The TX buffer in spi_transfer can be a NULL pointer, so the interrupt
> handler may end up writing to the invalid memory and cause crashes.
>
> Add a check to trans->tx_buf before using it.
>
> Fixes: 1ce24864bff4 ("spi: mediatek: Only do dma for 4-byte aligned buffers")
> Signed-off-by: Fei Shao <fshao@chromium.org>
Honestly, the code in the !host->can_dma conditional is probably a good candidate
for just going into its own function, as it is effectively an alternative flow for
the ISR (fifo vs dma xceiv) but whatever, as long as it doesn't get any "longer",
it's still fine I guess.... so:
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/2] spi: spi-mt65xx: Fix NULL pointer access in interrupt handler
@ 2024-03-21 8:58 ` AngeloGioacchino Del Regno
0 siblings, 0 replies; 15+ messages in thread
From: AngeloGioacchino Del Regno @ 2024-03-21 8:58 UTC (permalink / raw
To: Fei Shao, Mark Brown
Cc: Daniel Kurtz, Matthias Brugger, linux-arm-kernel, linux-kernel,
linux-mediatek, linux-spi
Il 21/03/24 08:08, Fei Shao ha scritto:
> The TX buffer in spi_transfer can be a NULL pointer, so the interrupt
> handler may end up writing to the invalid memory and cause crashes.
>
> Add a check to trans->tx_buf before using it.
>
> Fixes: 1ce24864bff4 ("spi: mediatek: Only do dma for 4-byte aligned buffers")
> Signed-off-by: Fei Shao <fshao@chromium.org>
Honestly, the code in the !host->can_dma conditional is probably a good candidate
for just going into its own function, as it is effectively an alternative flow for
the ISR (fifo vs dma xceiv) but whatever, as long as it doesn't get any "longer",
it's still fine I guess.... so:
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 2/2] spi: spi-mt65xx: Rename a variable in interrupt handler
2024-03-21 7:08 ` Fei Shao
@ 2024-03-21 8:58 ` AngeloGioacchino Del Regno
-1 siblings, 0 replies; 15+ messages in thread
From: AngeloGioacchino Del Regno @ 2024-03-21 8:58 UTC (permalink / raw
To: Fei Shao, Mark Brown
Cc: Matthias Brugger, linux-arm-kernel, linux-kernel, linux-mediatek,
linux-spi
Il 21/03/24 08:08, Fei Shao ha scritto:
> All the spi_transfer variables in this file use the name "xfer" except
> the one in mtk_spi_interrupt(). Align the naming for consistency and
> easier searching.
>
> While at it, reformat one memcpy() usage since the coding style allows
> 100 column lines today.
>
> This commit has no functional change.
>
> Signed-off-by: Fei Shao <fshao@chromium.org>
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 2/2] spi: spi-mt65xx: Rename a variable in interrupt handler
@ 2024-03-21 8:58 ` AngeloGioacchino Del Regno
0 siblings, 0 replies; 15+ messages in thread
From: AngeloGioacchino Del Regno @ 2024-03-21 8:58 UTC (permalink / raw
To: Fei Shao, Mark Brown
Cc: Matthias Brugger, linux-arm-kernel, linux-kernel, linux-mediatek,
linux-spi
Il 21/03/24 08:08, Fei Shao ha scritto:
> All the spi_transfer variables in this file use the name "xfer" except
> the one in mtk_spi_interrupt(). Align the naming for consistency and
> easier searching.
>
> While at it, reformat one memcpy() usage since the coding style allows
> 100 column lines today.
>
> This commit has no functional change.
>
> Signed-off-by: Fei Shao <fshao@chromium.org>
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 0/2] Fixes NULL pointer access in spi-mt65xx.c
2024-03-21 7:08 ` Fei Shao
@ 2024-03-21 15:01 ` Mark Brown
-1 siblings, 0 replies; 15+ messages in thread
From: Mark Brown @ 2024-03-21 15:01 UTC (permalink / raw
To: AngeloGioacchino Del Regno, Fei Shao
Cc: Daniel Kurtz, Matthias Brugger, linux-arm-kernel, linux-kernel,
linux-mediatek, linux-spi
On Thu, 21 Mar 2024 15:08:56 +0800, Fei Shao wrote:
> This series contains two patches for spi-mt65xx.c, both focusing on its
> interrupt handler mtk_spi_interrupt().
>
> The first patch is to fix a NULL pointer access in the interrupt
> handler, which is first found on a MT8186 Chromebook device when the
> system tries to establish host communication with its embedded
> controller.
>
> [...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
Thanks!
[1/2] spi: spi-mt65xx: Fix NULL pointer access in interrupt handler
commit: a20ad45008a7c82f1184dc6dee280096009ece55
[2/2] spi: spi-mt65xx: Rename a variable in interrupt handler
(no commit info)
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 0/2] Fixes NULL pointer access in spi-mt65xx.c
@ 2024-03-21 15:01 ` Mark Brown
0 siblings, 0 replies; 15+ messages in thread
From: Mark Brown @ 2024-03-21 15:01 UTC (permalink / raw
To: AngeloGioacchino Del Regno, Fei Shao
Cc: Daniel Kurtz, Matthias Brugger, linux-arm-kernel, linux-kernel,
linux-mediatek, linux-spi
On Thu, 21 Mar 2024 15:08:56 +0800, Fei Shao wrote:
> This series contains two patches for spi-mt65xx.c, both focusing on its
> interrupt handler mtk_spi_interrupt().
>
> The first patch is to fix a NULL pointer access in the interrupt
> handler, which is first found on a MT8186 Chromebook device when the
> system tries to establish host communication with its embedded
> controller.
>
> [...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
Thanks!
[1/2] spi: spi-mt65xx: Fix NULL pointer access in interrupt handler
commit: a20ad45008a7c82f1184dc6dee280096009ece55
[2/2] spi: spi-mt65xx: Rename a variable in interrupt handler
(no commit info)
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2024-03-21 15:02 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-03-21 7:08 [PATCH v2 0/2] Fixes NULL pointer access in spi-mt65xx.c Fei Shao
2024-03-21 7:08 ` Fei Shao
2024-03-21 7:08 ` Fei Shao
2024-03-21 7:08 ` [PATCH v2 1/2] spi: spi-mt65xx: Fix NULL pointer access in interrupt handler Fei Shao
2024-03-21 7:08 ` Fei Shao
2024-03-21 7:08 ` Fei Shao
2024-03-21 8:58 ` AngeloGioacchino Del Regno
2024-03-21 8:58 ` AngeloGioacchino Del Regno
2024-03-21 7:08 ` [PATCH v2 2/2] spi: spi-mt65xx: Rename a variable " Fei Shao
2024-03-21 7:08 ` Fei Shao
2024-03-21 7:08 ` Fei Shao
2024-03-21 8:58 ` AngeloGioacchino Del Regno
2024-03-21 8:58 ` AngeloGioacchino Del Regno
2024-03-21 15:01 ` [PATCH v2 0/2] Fixes NULL pointer access in spi-mt65xx.c Mark Brown
2024-03-21 15:01 ` Mark Brown
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.