From mboxrd@z Thu Jan  1 00:00:00 1970
From: Claude Code Review Bot <claude-review@example.com>
To: dri-devel-reviews@example.com
Subject: Claude review: lib/fonts: Store font data for user space with font_data_export()
Date: Tue, 03 Mar 2026 13:19:16 +1000
Message-ID: <review-patch12-20260302141255.518657-13-tzimmermann@suse.de>
In-Reply-To: <20260302141255.518657-13-tzimmermann@suse.de>
References: <20260302141255.518657-1-tzimmermann@suse.de> <20260302141255.518657-13-tzimmermann@suse.de>
X-Mailer: Claude Code Patch Reviewer
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0

Patch Review

This patch has the most significant functional change in the series.

**Issue 1 (potential underflow)**: `font_data_export()` computes:
```c
memset(data + glyphsize, 0, pitch * vpitch - glyphsize);
```
If `vpitch < font->height`, then `pitch * vpitch < glyphsize` and this unsigned subtraction wraps to a huge value, causing a buffer overflow. The caller (`fbcon_get_font`) checks `font->height > vpitch` beforehand, but `font_data_export()` is `EXPORT_SYMBOL_GPL` and could be called by other code without that check. Adding a defensive `if (vpitch < font->height) return -ENOSPC;` in `font_data_export()` would be prudent.

**Issue 2 (behavioral change for width 17-24)**: The old code handled fonts with width 17-24 using a special 4-byte internal pitch with 3-byte export:
```c
for (j = 0; j < vc->vc_font.height; j++) {
    *data++ = fontdata[0];
    *data++ = fontdata[1];
    *data++ = fontdata[2];
    fontdata += sizeof(u32);
}
```
The new code uses a straight 3-byte pitch (DIV_ROUND_UP(24, 8) = 3) without the 4-byte stride on the source side. The commit message argues this is correct and cites the setfont utility. However, this changes the interpretation of stored font data for 17-24px wide fonts. If any existing internal font data uses a 4-byte internal pitch, this would read incorrect glyph data. There don't appear to be any built-in fonts with width 17-24, so this may be safe in practice, but the change should be tested with user-space 17-24px fonts loaded via setfont.

**Issue 3**: The overflow check `font->charcount * glyphsize > font_data_size(fd)` doesn't use `check_mul_overflow`, unlike `font_data_import()`. While realistic values won't overflow, consistency with the import path would be better.

---
Generated by Claude Code Patch Reviewer