[PATCH 0/3] drm/ssd130x: fix column and row end address in partial updates of ssd132x and ssd133x devices

[PATCH 0/3] drm/ssd130x: fix column and row end address in partial updates of ssd132x and ssd133x devices

[Amit Barzilai]

While working on adding SSD1351 support to the ssd130x driver - I
noticed that the area of the cursor was refreshing in a weird state.
I would fill the screen red while the flashing cursor was on the screen,
but the screen wouldn't refresh the entire 8x8 rectangle each flash. 

After reading the SSD1351, SSD1331 and SSD132x manuals and manual
testing on the SSD1351, I was able to be sure that the problem was the
end addresses of "Set Column/Row Address" being sent as relative addresses
instead of absolute addresses.

I assume these bugs went under the radar because testing would usually
be done where the dirty rectangles would start from the 0,0 coordinates.
In these situations, the bug would be invisable, since adresses relative
to 0 are the same as their absolute value.

It is important to note that I didn't test these changes on a SSD132x 
screen or a SSD133x screens, only on an SSD1351. Manual testing is
advised.

This series contains the fix for ssd132x_update_rect, a simple clean-up
patch for ssd132x_update_rect to keep the function clean, and the same
fix on ssd133x_update_rect.

Amit Barzilai (3):
  drm/ssd130x: fix column and row end address in partial updates for
    ssd132x
  drm/ssd130x: hoist column and row addresses out of repeated division
  drm/ssd130x: fix column and row end address in partial updates in
    ssd133x

 drivers/gpu/drm/solomon/ssd130x.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

-- 
2.54.0

[PATCH 1/3] drm/ssd130x: fix column and row end address in partial updates for ssd132x

[Amit Barzilai]

On partial screen updates, SSD132X controllers expect to get the
rectangle addresses as arguments of the "Set Column Address" and "Set
Row Address" commands. Each command expects the start address and end
address of the row/column in absolute format, however the end
addresses were being sent in a relative format (relative to the start
address).

The relative end addresses work only when the start address is 0. In
those situations, there is no value difference between relative and
absolute addresses.

Fixes: fdd591e00a9c9 ("drm/ssd130x: Add support for the SSD132x OLED controller family")
Cc: stable@vger.kernel.org
Signed-off-by: Amit Barzilai <amit.barzilai22@gmail.com>
---
 drivers/gpu/drm/solomon/ssd130x.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/solomon/ssd130x.c b/drivers/gpu/drm/solomon/ssd130x.c
index c77455b1834d..77aa5585a46c 100644
--- a/drivers/gpu/drm/solomon/ssd130x.c
+++ b/drivers/gpu/drm/solomon/ssd130x.c
@@ -864,12 +864,13 @@ static int ssd132x_update_rect(struct ssd130x_device *ssd130x,
 	 */
 
 	/* Set column start and end */
-	ret = ssd130x_write_cmd(ssd130x, 3, SSD132X_SET_COL_RANGE, x / segment_width, columns - 1);
+	ret = ssd130x_write_cmd(ssd130x, 3, SSD132X_SET_COL_RANGE, x / segment_width,
+				x / segment_width + columns - 1);
 	if (ret < 0)
 		return ret;
 
 	/* Set row start and end */
-	ret = ssd130x_write_cmd(ssd130x, 3, SSD132X_SET_ROW_RANGE, y, rows - 1);
+	ret = ssd130x_write_cmd(ssd130x, 3, SSD132X_SET_ROW_RANGE, y, y + rows - 1);
 	if (ret < 0)
 		return ret;
 
-- 
2.54.0

Re: [PATCH 1/3] drm/ssd130x: fix column and row end address in partial updates for ssd132x

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Potential 1-byte out-of-bounds read past the heap buffer in `ssd132x_update_rect` when device width is configured to an odd value.
--

commit de1c71cf327dfb3f916baab562b7b1c9945af5e9
Author: Amit Barzilai <amit.barzilai22@gmail.com>

drm/ssd130x: fix column and row end address in partial updates for ssd132x

This commit corrects the relative end addresses used in the "Set Column Address"
and "Set Row Address" commands for SSD132X controllers during partial screen
updates. It ensures that the absolute addresses are correctly calculated and
sent to the controller.

Cc: stable@vger.kernel.org

> diff --git a/drivers/gpu/drm/solomon/ssd130x.c b/drivers/gpu/drm/solomon/ssd130x.c
> --- a/drivers/gpu/drm/solomon/ssd130x.c
> +++ b/drivers/gpu/drm/solomon/ssd130x.c

[ ... ]

> @@ -864,12 +864,13 @@ static int ssd132x_update_rect(struct ssd130x_device *ssd130x,
>  	 */
>  
>  	/* Set column start and end */
> -	ret = ssd130x_write_cmd(ssd130x, 3, SSD132X_SET_COL_RANGE, x / segment_width, columns - 1);
> +	ret = ssd130x_write_cmd(ssd130x, 3, SSD132X_SET_COL_RANGE, x / segment_width,
> +				x / segment_width + columns - 1);
>  	if (ret < 0)
>  		return ret;
>  
>  	/* Set row start and end */
> -	ret = ssd130x_write_cmd(ssd130x, 3, SSD132X_SET_ROW_RANGE, y, rows - 1);
> +	ret = ssd130x_write_cmd(ssd130x, 3, SSD132X_SET_ROW_RANGE, y, y + rows - 1);
>  	if (ret < 0)
>  		return ret;
>  
>  	for (i = 0; i < height; i++) {
>  		/* Process pair of pixels and combine them into a single byte */
>  		for (j = 0; j < width; j += segment_width) {
>  			u8 n1 = buf[i * width + j];
>  			u8 n2 = buf[i * width + j + 1];

[Severity: High]
This is a pre-existing issue, but could this loop read out of bounds if the
device tree specifies an odd solomon,width (e.g., 129)?

If the device width is configured to an odd value, the update rectangle's
width can also be odd. The buffer array is allocated in
ssd132x_primary_plane_atomic_check() with exactly pitch * height bytes
(where pitch equals width).

During the last iteration of the final row, j + 1 would evaluate to width.
This causes the n2 assignment to access buf[height * width], which is one
byte past the requested allocation size.

Can this trigger a KASAN out-of-bounds read warning or scramble display
output on odd-width panels by reading across row boundaries?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260622122604.32500-1-amit.barzilai22@gmail.com?part=1

Re: [PATCH 1/3] drm/ssd130x: fix column and row end address in partial updates for ssd132x

[Amit Barzilai]

On Mon, Jun 22, 2026 at 12:36:30PM +0000, sashiko-bot@kernel.org wrote:
> Pre-existing issues:
> - [High] Potential 1-byte out-of-bounds read past the heap buffer in
>   `ssd132x_update_rect` when device width is configured to an odd value.

As stated in the review, this is a pre-existing issue and is not touched by
this series, which only corrects the SET_COL_RANGE/SET_ROW_RANGE end
addresses sent to the controller.

Gentle ping on the series -- it's been about a week with no comments. Is
there anything you'd like changed for it to be picked up? Happy to spin a
v2 if needed.

-- 
Thanks,
Amit

Re: [PATCH 1/3] drm/ssd130x: fix column and row end address in partial updates for ssd132x

[Javier Martinez Canillas]

Amit Barzilai <amit.barzilai22@gmail.com> writes:

Hello Amit,

Thank for your patches.

> On Mon, Jun 22, 2026 at 12:36:30PM +0000, sashiko-bot@kernel.org wrote:
>> Pre-existing issues:
>> - [High] Potential 1-byte out-of-bounds read past the heap buffer in
>>   `ssd132x_update_rect` when device width is configured to an odd value.
>
> As stated in the review, this is a pre-existing issue and is not touched by
> this series, which only corrects the SET_COL_RANGE/SET_ROW_RANGE end
> addresses sent to the controller.
>
> Gentle ping on the series -- it's been about a week with no comments. Is
> there anything you'd like changed for it to be picked up? Happy to spin a
> v2 if needed.
>

Sorry, I was on PTO and couldn't take a look to your series. I plan to do
it this week.

One question, though. How did you find these issues and came up with the
fixes ?

It was just reading the code and the datasheets or were the issues (and
fixes?) identified by an LLM? If the latter, then you need to add an
Assisted-by tag to your pa+tches.

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH 1/3] drm/ssd130x: fix column and row end address in partial updates for ssd132x

[Amit Barzilai]

Javier Martinez Canillas <javierm@redhat.com> writes:
> Sorry, I was on PTO and couldn't take a look to your series. I plan to do
> it this week.

Thank you for the update and there is no need to apologize.

> One question, though. How did you find these issues and came up with the
> fixes ?
>
> It was just reading the code and the datasheets or were the issues (and
> fixes?) identified by an LLM? If the latter, then you need to add an
> Assisted-by tag to your pa+tches.

I found the bug and solution while debugging the ssd1351 extention to ssd130x.
I used an LLM to make sure my patches made sense and to recheck my patch logically
and I wasn't sure whether that was enough to warrant the tag.
I'll submit a v2 of this series with the tag - let me know if you'd prefer it handled
differently.

-- 
Thanks,
Amit

Re: [PATCH 1/3] drm/ssd130x: fix column and row end address in partial updates for ssd132x

[Javier Martinez Canillas]

Amit Barzilai <amit.barzilai22@gmail.com> writes:

Hello Amit,

> Javier Martinez Canillas <javierm@redhat.com> writes:
>> Sorry, I was on PTO and couldn't take a look to your series. I plan to do
>> it this week.
>
> Thank you for the update and there is no need to apologize.
>
>> One question, though. How did you find these issues and came up with the
>> fixes ?
>>
>> It was just reading the code and the datasheets or were the issues (and
>> fixes?) identified by an LLM? If the latter, then you need to add an
>> Assisted-by tag to your pa+tches.
>
> I found the bug and solution while debugging the ssd1351 extention to ssd130x.
> I used an LLM to make sure my patches made sense and to recheck my patch logically
> and I wasn't sure whether that was enough to warrant the tag.

That's a good question. I don't really know to be honest.

If was only to review your manually written changes then I guess there's
no need to add the tag.

> I'll submit a v2 of this series with the tag - let me know if you'd prefer it handled
> differently.
>

Up to you. As said, I don't know were we draw the line so I will let you decide
if you want a v2 with the tags.

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH 1/3] drm/ssd130x: fix column and row end address in partial updates for ssd132x

[Amit Barzilai]

Javier Martinez Canillas <javierm@redhat.com> writes:
> Up to you. As said, I don't know were we draw the line so I will let you decide
> if you want a v2 with the tags.

I don't believe the usage was enough to warrant the tag, since the LLM only reviewed changes I'd
already written and reasoned out myself - let's keep v1 as is.

-- 
Thanks,
Amit

Re: [PATCH 1/3] drm/ssd130x: fix column and row end address in partial updates for ssd132x

[Javier Martinez Canillas]

Amit Barzilai <amit.barzilai22@gmail.com> writes:

> Javier Martinez Canillas <javierm@redhat.com> writes:
>> Up to you. As said, I don't know were we draw the line so I will let you decide
>> if you want a v2 with the tags.
>
> I don't believe the usage was enough to warrant the tag, since the LLM only reviewed changes I'd
> already written and reasoned out myself - let's keep v1 as is.
>

Works for me. As metioned, I'll try to review your patches this week.

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

[PATCH 2/3] drm/ssd130x: hoist column and row addresses out of repeated division

[Amit Barzilai]

The ssd132x_update_rect had to calculate the column address twice in
the same function by dividing the byte address and the segment_width.
Optimize this by hoisting the result into a col variable.

Renamed the "y" variable to "row" to match the change made in the x
axis.

Signed-off-by: Amit Barzilai <amit.barzilai22@gmail.com>
---
 drivers/gpu/drm/solomon/ssd130x.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/solomon/ssd130x.c b/drivers/gpu/drm/solomon/ssd130x.c
index 77aa5585a46c..fee35496a324 100644
--- a/drivers/gpu/drm/solomon/ssd130x.c
+++ b/drivers/gpu/drm/solomon/ssd130x.c
@@ -835,9 +835,9 @@ static int ssd132x_update_rect(struct ssd130x_device *ssd130x,
 			       struct drm_rect *rect, u8 *buf,
 			       u8 *data_array)
 {
-	unsigned int x = rect->x1;
-	unsigned int y = rect->y1;
 	unsigned int segment_width = SSD132X_SEGMENT_WIDTH;
+	unsigned int col = rect->x1 / segment_width;
+	unsigned int row = rect->y1;
 	unsigned int width = drm_rect_width(rect);
 	unsigned int height = drm_rect_height(rect);
 	unsigned int columns = DIV_ROUND_UP(width, segment_width);
@@ -847,7 +847,7 @@ static int ssd132x_update_rect(struct ssd130x_device *ssd130x,
 	unsigned int i, j;
 	int ret;
 
-	drm_WARN_ONCE(drm, x % segment_width != 0, "x must be aligned to screen segment\n");
+	drm_WARN_ONCE(drm, rect->x1 % segment_width != 0, "x must be aligned to screen segment\n");
 
 	/*
 	 * The screen is divided in Segment and Common outputs, where
@@ -864,13 +864,12 @@ static int ssd132x_update_rect(struct ssd130x_device *ssd130x,
 	 */
 
 	/* Set column start and end */
-	ret = ssd130x_write_cmd(ssd130x, 3, SSD132X_SET_COL_RANGE, x / segment_width,
-				x / segment_width + columns - 1);
+	ret = ssd130x_write_cmd(ssd130x, 3, SSD132X_SET_COL_RANGE, col, col + columns - 1);
 	if (ret < 0)
 		return ret;
 
 	/* Set row start and end */
-	ret = ssd130x_write_cmd(ssd130x, 3, SSD132X_SET_ROW_RANGE, y, y + rows - 1);
+	ret = ssd130x_write_cmd(ssd130x, 3, SSD132X_SET_ROW_RANGE, row, row + rows - 1);
 	if (ret < 0)
 		return ret;
 
-- 
2.54.0

[PATCH 3/3] drm/ssd130x: fix column and row end address in partial updates in ssd133x

[Amit Barzilai]

On partial screen updates, SSD133X controllers expect to get the
rectangle addresses as arguments of the "Set Column Address" and "Set
Row Address" commands. Each command expects the start address and end
address of the row/column in absolute format, however the end
addresses were being sent in a relative format (relative to the start
address).

The relative end addresses work only when the start address is 0. In
those situations, there is no value difference between relative and
absolute addresses.

Fixes: b4299c936d8fd ("drm/ssd130x: Add support for the SSD133x OLED controller family")
Cc: stable@vger.kernel.org
Signed-off-by: Amit Barzilai <amit.barzilai22@gmail.com>
---
 drivers/gpu/drm/solomon/ssd130x.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/solomon/ssd130x.c b/drivers/gpu/drm/solomon/ssd130x.c
index fee35496a324..4e4b879d8b24 100644
--- a/drivers/gpu/drm/solomon/ssd130x.c
+++ b/drivers/gpu/drm/solomon/ssd130x.c
@@ -915,12 +915,12 @@ static int ssd133x_update_rect(struct ssd130x_device *ssd130x,
 	 */
 
 	/* Set column start and end */
-	ret = ssd130x_write_cmd(ssd130x, 3, SSD133X_SET_COL_RANGE, x, columns - 1);
+	ret = ssd130x_write_cmd(ssd130x, 3, SSD133X_SET_COL_RANGE, x, x + columns - 1);
 	if (ret < 0)
 		return ret;
 
 	/* Set row start and end */
-	ret = ssd130x_write_cmd(ssd130x, 3, SSD133X_SET_ROW_RANGE, y, rows - 1);
+	ret = ssd130x_write_cmd(ssd130x, 3, SSD133X_SET_ROW_RANGE, y, y + rows - 1);
 	if (ret < 0)
 		return ret;
 
-- 
2.54.0

Re: [PATCH 0/3] drm/ssd130x: fix column and row end address in partial updates of ssd132x and ssd133x devices

[Javier Martinez Canillas]

Amit Barzilai <amit.barzilai22@gmail.com> writes:

Hello Amit,

Thanks for your patches.

> While working on adding SSD1351 support to the ssd130x driver - I
> noticed that the area of the cursor was refreshing in a weird state.
> I would fill the screen red while the flashing cursor was on the screen,
> but the screen wouldn't refresh the entire 8x8 rectangle each flash. 
>
> After reading the SSD1351, SSD1331 and SSD132x manuals and manual
> testing on the SSD1351, I was able to be sure that the problem was the
> end addresses of "Set Column/Row Address" being sent as relative addresses
> instead of absolute addresses.
>

Indeed.

> I assume these bugs went under the radar because testing would usually
> be done where the dirty rectangles would start from the 0,0 coordinates.
> In these situations, the bug would be invisable, since adresses relative
> to 0 are the same as their absolute value.
>

Yes, I believe that was the case. Thanks a lot for fixing these issues!

> It is important to note that I didn't test these changes on a SSD132x 
> scree0n or a SSD133x screens, only on an SSD1351. Manual testing is
> advised.
>
> This series contains the fix for ssd132x_update_rect, a simple clean-up
> patch for ssd132x_update_rect to keep the function clean, and the same
> fix on ssd133x_update_rect.
>

For the whole series:

Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>

I plan to push them to drm-misc-next rather than to drm-misc-fixes, since
these have been long standing bugs and the changes are likely to conflict
with your SSD1351 patches that are in-flight.

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH 0/3] drm/ssd130x: fix column and row end address in partial updates of ssd132x and ssd133x devices

[Javier Martinez Canillas]

Javier Martinez Canillas <javierm@redhat.com> writes:

> Amit Barzilai <amit.barzilai22@gmail.com> writes:
>
> Hello Amit,
>
> Thanks for your patches.
>
>> While working on adding SSD1351 support to the ssd130x driver - I
>> noticed that the area of the cursor was refreshing in a weird state.
>> I would fill the screen red while the flashing cursor was on the screen,
>> but the screen wouldn't refresh the entire 8x8 rectangle each flash. 
>>
>> After reading the SSD1351, SSD1331 and SSD132x manuals and manual
>> testing on the SSD1351, I was able to be sure that the problem was the
>> end addresses of "Set Column/Row Address" being sent as relative addresses
>> instead of absolute addresses.
>>

[...]

> For the whole series:
>
> Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
>
> I plan to push them to drm-misc-next rather than to drm-misc-fixes, since
> these have been long standing bugs and the changes are likely to conflict
> with your SSD1351 patches that are in-flight.

Pushed to drm-misc (drm-misc-next). Thanks!

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat