Re: [PATCH] Input: joydev - prevent potential read overflow in ioctl

On Fri, Feb 19, 2021 at 11:32:15AM +0300, Dan Carpenter wrote:

On Thu, Feb 18, 2021 at 07:17:44PM -0800, Dmitry Torokhov wrote:

quoted

Hi Dan,

On Wed, Feb 17, 2021 at 09:10:15AM +0300, Dan Carpenter wrote:

quoted

The problem here is that "len" might be less than "joydev->nabs" so the
loops which verfy abspam[i] and keypam[] might read beyond the buffer.

Fixes: 999b874f4aa3 ("Input: joydev - validate axis/button maps before clobbering current ones")
Signed-off-by: Dan Carpenter <redacted>

Thank you for the patch.

quoted

---
 drivers/input/joydev.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/input/joydev.c b/drivers/input/joydev.c
index a2b5fbba2d3b..750f4513fe20 100644
--- a/drivers/input/joydev.c
+++ b/drivers/input/joydev.c

@@ -456,7 +456,7 @@ static int joydev_handle_JSIOCSAXMAP(struct joydev *joydev,
 	if (IS_ERR(abspam))
 		return PTR_ERR(abspam);
 
-	for (i = 0; i < joydev->nabs; i++) {
+	for (i = 0; i < len && i < joydev->nabs; i++) {
 		if (abspam[i] > ABS_MAX) {
 			retval = -EINVAL;
 			goto out;

@@ -487,7 +487,7 @@ static int joydev_handle_JSIOCSBTNMAP(struct joydev *joydev,
 	if (IS_ERR(keypam))
 		return PTR_ERR(keypam);
 
-	for (i = 0; i < joydev->nkey; i++) {
+	for (i = 0; i < (len / 2) && i < joydev->nkey; i++) {

I think we also need to make sure that len is even. Do you mind if I add
the check at the top of the function to return -EINVAL if it is odd?

Yes.  You are right.

Btw, thank you for fixing my patches, but feel free to just ask me to
resend if that's easier for you.

It is not really fixing your patch, rather addressing another issue
while we are there. Some might even say it could have been a separate
patch.

Anyway, for small fixups like this I feel it's less noise on the lists
without extra resubmissions, that is why I often do them on my side.

Thanks!

-- 
Dmitry