[PATCH v5 0/3] media: rc: ir-spi: allocate buffer dynamically

[PATCH v5 0/3] media: rc: ir-spi: allocate buffer dynamically

[Cosmin Tanislav]

Replace the static transmit buffer with a dynamically allocated one,
removing the limit imposed on the number of pulses to transmit.

Add a check to constrain the carrier frequency inside
ir_spi_set_tx_carrier().

Switch to u64 arithmetic to ir_spi_tx() when calculating the number
of pulses to transmit.

V5:
 * add separate patch to solve overflow issues in ir_spi_tx()
 * avoid overflow in carrier frequency constraint

V4:
 * add separate patch to constrain the carrier frequency

V3:
 * move the allocation to be done per-TX operation

V2:
 * use devm_krealloc_array

Cosmin Tanislav (3):
  media: rc: ir-spi: allocate buffer dynamically
  media: rc: ir-spi: constrain carrier frequency
  media: rc: ir-spi: avoid overflow in multiplication

 drivers/media/rc/ir-spi.c | 40 +++++++++++++++++++++++++--------------
 1 file changed, 26 insertions(+), 14 deletions(-)

-- 
2.49.0

[PATCH v5 1/3] media: rc: ir-spi: allocate buffer dynamically

[Cosmin Tanislav]

Replace the static transmit buffer with a dynamically allocated one,
removing the limit imposed on the number of pulses to transmit.

Calculate the number of pulses for each duration in the received buffer
ahead of time, while also adding up the total pulses, to be able to
allocate a buffer that perfectly fits the total number of pulses, then
populate it.

Signed-off-by: Cosmin Tanislav <demonsingur@gmail.com>
---
 drivers/media/rc/ir-spi.c | 33 ++++++++++++++++++++-------------
 1 file changed, 20 insertions(+), 13 deletions(-)

diff --git a/drivers/media/rc/ir-spi.c b/drivers/media/rc/ir-spi.c
index 8fc8e496e6aa..50e30e2fae22 100644
--- a/drivers/media/rc/ir-spi.c
+++ b/drivers/media/rc/ir-spi.c
@@ -21,13 +21,11 @@
 #define IR_SPI_DRIVER_NAME		"ir-spi"
 
 #define IR_SPI_DEFAULT_FREQUENCY	38000
-#define IR_SPI_MAX_BUFSIZE		 4096
 
 struct ir_spi_data {
 	u32 freq;
 	bool negated;
 
-	u16 tx_buf[IR_SPI_MAX_BUFSIZE];
 	u16 pulse;
 	u16 space;
 
@@ -43,37 +41,42 @@ static int ir_spi_tx(struct rc_dev *dev, unsigned int *buffer, unsigned int coun
 	unsigned int len = 0;
 	struct ir_spi_data *idata = dev->priv;
 	struct spi_transfer xfer;
+	u16 *tx_buf;
 
 	/* convert the pulse/space signal to raw binary signal */
 	for (i = 0; i < count; i++) {
-		unsigned int periods;
+		buffer[i] = DIV_ROUND_CLOSEST(buffer[i] * idata->freq, 1000000);
+		len += buffer[i];
+	}
+
+	tx_buf = kmalloc_array(len, sizeof(*tx_buf), GFP_KERNEL);
+	if (!tx_buf)
+		return -ENOMEM;
+
+	len = 0;
+	for (i = 0; i < count; i++) {
 		int j;
 		u16 val;
 
-		periods = DIV_ROUND_CLOSEST(buffer[i] * idata->freq, 1000000);
-
-		if (len + periods >= IR_SPI_MAX_BUFSIZE)
-			return -EINVAL;
-
 		/*
 		 * The first value in buffer is a pulse, so that 0, 2, 4, ...
 		 * contain a pulse duration. On the contrary, 1, 3, 5, ...
 		 * contain a space duration.
 		 */
 		val = (i % 2) ? idata->space : idata->pulse;
-		for (j = 0; j < periods; j++)
-			idata->tx_buf[len++] = val;
+		for (j = 0; j < buffer[i]; j++)
+			tx_buf[len++] = val;
 	}
 
 	memset(&xfer, 0, sizeof(xfer));
 
 	xfer.speed_hz = idata->freq * 16;
-	xfer.len = len * sizeof(*idata->tx_buf);
-	xfer.tx_buf = idata->tx_buf;
+	xfer.len = len * sizeof(*tx_buf);
+	xfer.tx_buf = tx_buf;
 
 	ret = regulator_enable(idata->regulator);
 	if (ret)
-		return ret;
+		goto err_free_tx_buf;
 
 	ret = spi_sync_transfer(idata->spi, &xfer, 1);
 	if (ret)
@@ -81,6 +84,10 @@ static int ir_spi_tx(struct rc_dev *dev, unsigned int *buffer, unsigned int coun
 
 	regulator_disable(idata->regulator);
 
+err_free_tx_buf:
+
+	kfree(tx_buf);
+
 	return ret ? ret : count;
 }
 
-- 
2.49.0

[PATCH v5 2/3] media: rc: ir-spi: constrain carrier frequency

[Cosmin Tanislav]

Carrier frequency is currently unconstrained, allowing the SPI transfer
to be allocated and filled only for it to be later rejected by the SPI
controller since the frequency is too large.

Add a check to constrain the carrier frequency inside
ir_spi_set_tx_carrier().

Also, move the number of bits per pulse to a macro since it is not used
in multiple places.

Signed-off-by: Cosmin Tanislav <demonsingur@gmail.com>
---
 drivers/media/rc/ir-spi.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/media/rc/ir-spi.c b/drivers/media/rc/ir-spi.c
index 50e30e2fae22..0b54ad74cec0 100644
--- a/drivers/media/rc/ir-spi.c
+++ b/drivers/media/rc/ir-spi.c
@@ -21,6 +21,7 @@
 #define IR_SPI_DRIVER_NAME		"ir-spi"
 
 #define IR_SPI_DEFAULT_FREQUENCY	38000
+#define IR_SPI_BITS_PER_PULSE		16
 
 struct ir_spi_data {
 	u32 freq;
@@ -70,7 +71,7 @@ static int ir_spi_tx(struct rc_dev *dev, unsigned int *buffer, unsigned int coun
 
 	memset(&xfer, 0, sizeof(xfer));
 
-	xfer.speed_hz = idata->freq * 16;
+	xfer.speed_hz = idata->freq * IR_SPI_BITS_PER_PULSE;
 	xfer.len = len * sizeof(*tx_buf);
 	xfer.tx_buf = tx_buf;
 
@@ -98,6 +99,9 @@ static int ir_spi_set_tx_carrier(struct rc_dev *dev, u32 carrier)
 	if (!carrier)
 		return -EINVAL;
 
+	if (carrier > idata->spi->max_speed_hz / IR_SPI_BITS_PER_PULSE)
+		return -EINVAL;
+
 	idata->freq = carrier;
 
 	return 0;
-- 
2.49.0

[PATCH v5 3/3] media: rc: ir-spi: avoid overflow in multiplication

[Cosmin Tanislav]

Switch to u64 arithmetic and use DIV_ROUND_CLOSEST_ULL() to avoid
the overflow.

buffer[i] is unsigned int and is limited by the lirc core to
IR_MAX_DURATION, which is 500000.

idata->freq is u32, which has a max value of 0xFFFFFFFF.

In the case where buffer[i] is 500000, idata->freq overflows the u32
multiplication for any values >= 8590.

0xFFFFFFFF / 500000 ~= 8589

By casting buffer[i] to u64, idata->freq can be any u32 value without
overflowing the multiplication.

0xFFFFFFFFFFFFFFFF / 500000 ~= 36893488147419 (> 4294967295)

The result of the final operation will fit back into the unsigned int
limits without any issues.

500000 * 0xFFFFFFFF / 1000000 = 0x80000000 (< 0xFFFFFFFF)

Signed-off-by: Cosmin Tanislav <demonsingur@gmail.com>
---
 drivers/media/rc/ir-spi.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/media/rc/ir-spi.c b/drivers/media/rc/ir-spi.c
index 0b54ad74cec0..392441e0c116 100644
--- a/drivers/media/rc/ir-spi.c
+++ b/drivers/media/rc/ir-spi.c
@@ -46,7 +46,8 @@ static int ir_spi_tx(struct rc_dev *dev, unsigned int *buffer, unsigned int coun
 
 	/* convert the pulse/space signal to raw binary signal */
 	for (i = 0; i < count; i++) {
-		buffer[i] = DIV_ROUND_CLOSEST(buffer[i] * idata->freq, 1000000);
+		buffer[i] = DIV_ROUND_CLOSEST_ULL((u64)buffer[i] * idata->freq,
+						  1000000);
 		len += buffer[i];
 	}
 
-- 
2.49.0

Re: [PATCH v5 0/3] media: rc: ir-spi: allocate buffer dynamically

[Sean Young]

On Fri, Jun 13, 2025 at 02:21:50PM +0300, Cosmin Tanislav wrote:
> Replace the static transmit buffer with a dynamically allocated one,
> removing the limit imposed on the number of pulses to transmit.
> 
> Add a check to constrain the carrier frequency inside
> ir_spi_set_tx_carrier().
> 
> Switch to u64 arithmetic to ir_spi_tx() when calculating the number
> of pulses to transmit.

Looks good, I've created a PR to have it merged.

Sean

> 
> V5:
>  * add separate patch to solve overflow issues in ir_spi_tx()
>  * avoid overflow in carrier frequency constraint
> 
> V4:
>  * add separate patch to constrain the carrier frequency
> 
> V3:
>  * move the allocation to be done per-TX operation
> 
> V2:
>  * use devm_krealloc_array
> 
> Cosmin Tanislav (3):
>   media: rc: ir-spi: allocate buffer dynamically
>   media: rc: ir-spi: constrain carrier frequency
>   media: rc: ir-spi: avoid overflow in multiplication
> 
>  drivers/media/rc/ir-spi.c | 40 +++++++++++++++++++++++++--------------
>  1 file changed, 26 insertions(+), 14 deletions(-)
> 
> -- 
> 2.49.0
>