[PATCH] Input: ims-pcu - bound frame parser write index against read_buf size

[PATCH] Input: ims-pcu - bound frame parser write index against read_buf size

[Greg Kroah-Hartman]

ims_pcu_process_data() implements a STX/DLE/ETX byte-stuffing parser
that accumulates frame payload into pcu->read_buf[] using the running
index pcu->read_pos.  read_buf is IMS_PCU_BUF_SIZE (128) bytes and
read_pos is u8 but of course, we don't check the index before actually
writing the data :(

Fix this up by properly rejecting the frame at the first attempt to
write past read_buf and resync on the next STX, mirroring how the parser
handles short and bad-checksum frames on ETX.

Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Fixes: 628329d52474 ("Input: add IMS Passenger Control Unit driver")
Cc: stable <stable@kernel.org>
Assisted-by: gkh_clanker_t1000
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/input/misc/ims-pcu.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/input/misc/ims-pcu.c b/drivers/input/misc/ims-pcu.c
index f69de9762c4e..87c66483b493 100644
--- a/drivers/input/misc/ims-pcu.c
+++ b/drivers/input/misc/ims-pcu.c
@@ -451,6 +451,8 @@ static void ims_pcu_process_data(struct ims_pcu *pcu, struct urb *urb)
 
 		if (pcu->have_dle) {
 			pcu->have_dle = false;
+			if (pcu->read_pos >= IMS_PCU_BUF_SIZE)
+				goto frame_overflow;
 			pcu->read_buf[pcu->read_pos++] = data;
 			pcu->check_sum += data;
 			continue;
@@ -491,10 +493,19 @@ static void ims_pcu_process_data(struct ims_pcu *pcu, struct urb *urb)
 			break;
 
 		default:
+			if (pcu->read_pos >= IMS_PCU_BUF_SIZE)
+				goto frame_overflow;
 			pcu->read_buf[pcu->read_pos++] = data;
 			pcu->check_sum += data;
 			break;
 		}
+		continue;
+
+frame_overflow:
+		dev_warn(pcu->dev, "Frame longer than %d bytes, discarding\n", IMS_PCU_BUF_SIZE);
+		pcu->have_stx = false;
+		pcu->have_dle = false;
+		pcu->read_pos = 0;
 	}
 }
 
-- 
2.53.0

Re: [PATCH] Input: ims-pcu - bound frame parser write index against read_buf size

[Dmitry Torokhov]

Hi Greg,

On Mon, Apr 20, 2026 at 09:05:31PM +0200, Greg Kroah-Hartman wrote:
> ims_pcu_process_data() implements a STX/DLE/ETX byte-stuffing parser
> that accumulates frame payload into pcu->read_buf[] using the running
> index pcu->read_pos.  read_buf is IMS_PCU_BUF_SIZE (128) bytes and
> read_pos is u8 but of course, we don't check the index before actually
> writing the data :(
> 
> Fix this up by properly rejecting the frame at the first attempt to
> write past read_buf and resync on the next STX, mirroring how the parser
> handles short and bad-checksum frames on ETX.
> 
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Fixes: 628329d52474 ("Input: add IMS Passenger Control Unit driver")
> Cc: stable <stable@kernel.org>
> Assisted-by: gkh_clanker_t1000
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

I already have a patch for this, thanks.

-- 
Dmitry

Re: [PATCH] Input: ims-pcu - bound frame parser write index against read_buf size

[Greg Kroah-Hartman]

On Wed, Apr 22, 2026 at 06:36:24PM -0700, Dmitry Torokhov wrote:
> Hi Greg,
> 
> On Mon, Apr 20, 2026 at 09:05:31PM +0200, Greg Kroah-Hartman wrote:
> > ims_pcu_process_data() implements a STX/DLE/ETX byte-stuffing parser
> > that accumulates frame payload into pcu->read_buf[] using the running
> > index pcu->read_pos.  read_buf is IMS_PCU_BUF_SIZE (128) bytes and
> > read_pos is u8 but of course, we don't check the index before actually
> > writing the data :(
> > 
> > Fix this up by properly rejecting the frame at the first attempt to
> > write past read_buf and resync on the next STX, mirroring how the parser
> > handles short and bad-checksum frames on ETX.
> > 
> > Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > Fixes: 628329d52474 ("Input: add IMS Passenger Control Unit driver")
> > Cc: stable <stable@kernel.org>
> > Assisted-by: gkh_clanker_t1000
> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> 
> I already have a patch for this, thanks.

Ah, missed that, sorry, I was working against Linus's tree.  I am
guessing you are referring to commit 875115b82c29 ("Input: ims-pcu - fix
heap-buffer-overflow in ims_pcu_process_data()")?  If so, why wasn't
that tagged for stable inclusion?

thanks,

greg k-h

Re: [PATCH] Input: ims-pcu - bound frame parser write index against read_buf size

[Dmitry Torokhov]

On Thu, Apr 23, 2026 at 06:52:23AM +0200, Greg Kroah-Hartman wrote:
> On Wed, Apr 22, 2026 at 06:36:24PM -0700, Dmitry Torokhov wrote:
> > Hi Greg,
> > 
> > On Mon, Apr 20, 2026 at 09:05:31PM +0200, Greg Kroah-Hartman wrote:
> > > ims_pcu_process_data() implements a STX/DLE/ETX byte-stuffing parser
> > > that accumulates frame payload into pcu->read_buf[] using the running
> > > index pcu->read_pos.  read_buf is IMS_PCU_BUF_SIZE (128) bytes and
> > > read_pos is u8 but of course, we don't check the index before actually
> > > writing the data :(
> > > 
> > > Fix this up by properly rejecting the frame at the first attempt to
> > > write past read_buf and resync on the next STX, mirroring how the parser
> > > handles short and bad-checksum frames on ETX.
> > > 
> > > Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > > Fixes: 628329d52474 ("Input: add IMS Passenger Control Unit driver")
> > > Cc: stable <stable@kernel.org>
> > > Assisted-by: gkh_clanker_t1000
> > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > 
> > I already have a patch for this, thanks.
> 
> Ah, missed that, sorry, I was working against Linus's tree.  I am
> guessing you are referring to commit 875115b82c29 ("Input: ims-pcu - fix
> heap-buffer-overflow in ims_pcu_process_data()")?  If so, why wasn't
> that tagged for stable inclusion?

I do not believe it is worth it. The driver is for specialized hardware,
so common distros will not be enabling it, and systems where it is used
likely do not allow plugging weird stuff into them and probably do not
use stable either.

I actually wonder if we need to carry the driver or if we should simply
drop it. The only non-cleanup change to it was done in 2014.

Thanks.

-- 
Dmitry

Re: [PATCH] Input: ims-pcu - bound frame parser write index against read_buf size

[Greg Kroah-Hartman]

On Thu, Apr 23, 2026 at 10:24:08AM -0700, Dmitry Torokhov wrote:
> On Thu, Apr 23, 2026 at 06:52:23AM +0200, Greg Kroah-Hartman wrote:
> > On Wed, Apr 22, 2026 at 06:36:24PM -0700, Dmitry Torokhov wrote:
> > > Hi Greg,
> > > 
> > > On Mon, Apr 20, 2026 at 09:05:31PM +0200, Greg Kroah-Hartman wrote:
> > > > ims_pcu_process_data() implements a STX/DLE/ETX byte-stuffing parser
> > > > that accumulates frame payload into pcu->read_buf[] using the running
> > > > index pcu->read_pos.  read_buf is IMS_PCU_BUF_SIZE (128) bytes and
> > > > read_pos is u8 but of course, we don't check the index before actually
> > > > writing the data :(
> > > > 
> > > > Fix this up by properly rejecting the frame at the first attempt to
> > > > write past read_buf and resync on the next STX, mirroring how the parser
> > > > handles short and bad-checksum frames on ETX.
> > > > 
> > > > Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > > > Fixes: 628329d52474 ("Input: add IMS Passenger Control Unit driver")
> > > > Cc: stable <stable@kernel.org>
> > > > Assisted-by: gkh_clanker_t1000
> > > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > 
> > > I already have a patch for this, thanks.
> > 
> > Ah, missed that, sorry, I was working against Linus's tree.  I am
> > guessing you are referring to commit 875115b82c29 ("Input: ims-pcu - fix
> > heap-buffer-overflow in ims_pcu_process_data()")?  If so, why wasn't
> > that tagged for stable inclusion?
> 
> I do not believe it is worth it. The driver is for specialized hardware,
> so common distros will not be enabling it, and systems where it is used
> likely do not allow plugging weird stuff into them and probably do not
> use stable either.

Android allows a lot of odd things to be plugged into it :(

> I actually wonder if we need to carry the driver or if we should simply
> drop it. The only non-cleanup change to it was done in 2014.

I'll gladly send a patch to delete it if you want me to.

thanks,

greg k-h