On Mon, Sep 19, 2022 at 11:22:41PM -0700, Hyunwoo Kim wrote:
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.
- 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.
No no. No need to appologize or resend anything. This is just information to help in future research.