On Tue, Sep 20, 2022 at 09:13:31AM +0300, Dan Carpenter wrote:

On Tue, Sep 20, 2022 at 09:02:34AM +0300, Dan Carpenter wrote:

On Mon, Jun 20, 2022 at 07:00:10AM -0700, Hyunwoo Kim wrote:

In pxa3xx_gcu_write, a count parameter of
type size_t is passed to words of type int.
Then, copy_from_user may cause a heap overflow because
it is used as the third argument of copy_from_user.

Signed-off-by: Hyunwoo Kim <imv4bel@...>
---
drivers/video/fbdev/pxa3xx-gcu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/video/fbdev/pxa3xx-gcu.c b/drivers/video/fbdev/pxa3xx-gcu.c
index 043cc8f9ef1c..c3cd1e1cc01b 100644
--- a/drivers/video/fbdev/pxa3xx-gcu.c
+++ b/drivers/video/fbdev/pxa3xx-gcu.c
@@ -381,7 +381,7 @@ pxa3xx_gcu_write(struct file *file, const char *buff,
struct pxa3xx_gcu_batch *buffer;
struct pxa3xx_gcu_priv *priv = to_pxa3xx_gcu_priv(file);

- int words = count / 4;
+ size_t words = count / 4;

The count variable is actually capped at MAX_RW_COUNT in vfs_write()
so "words" cannot be negative. This patch helps clean up the code but
it does not affect run time.

Btw, the other thing which prevents this from being expliotable is that
if you pass a negative value to copy_from_user() it will not copy
anything because of the check in check_copy_size(). See commit
6d13de1489b6 ("uaccess: disallow > INT_MAX copy sizes").

Linus has sort of gotten annoyed with me before for pointing this stuff
out because it seemed like maybe I wasn't properly grateful to people
auditing the code and fixing bugs. I am grateful. This patch is
totally the correct thing to do. It's just that it's not really
exploitable as described in the commit message.

I found the code that might have the vulnerability, and submitted a patch without actually debugging it.
This is entirely my fault. sorry.

Should I submit a fix patch that fixes the commit message?

Sorry again.


Regards,
Hyunwoo Kim.