staging: alarm-dev: information leak in alarm_ioctl()

Smatch complains that if we pass an invalid clock type then "ts" is never set. We need to check for errors earlier, otherwise we end up passing uninitialized stack data to userspace.

Signed-off-by Dan Carpenter <[email protected]>

drivers/staging/android/alarm-dev.c +3 -1

index ceb1c643..c8600d9 100644
@@ -264,6 +264,8 @@ static long alarm_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
  	}
 
  	rv = alarm_do_ioctl(file, cmd, &ts);
+	if (rv)
+		return rv;
 
  	switch (ANDROID_ALARM_BASE_CMD(cmd)) {
  	case ANDROID_ALARM_GET_TIME(0):

@@ -272,7 +274,7 @@ static long alarm_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
  		break;
  	}
 
-	return rv;
+	return 0;
  }
  #ifdef CONFIG_COMPAT
  static long alarm_compat_ioctl(struct file *file, unsigned int cmd,

If we pass an invalid clock type then "ts" is never set. We need to check for errors earlier, otherwise we end up passing uninitialized stack data to userspace.

Reported-by John Stultz <[email protected]>

Signed-off-by Dan Carpenter <[email protected]>

drivers/staging/android/alarm-dev.c +2 -0

index c8600d9..6dc27da 100644
@@ -297,6 +297,8 @@ static long alarm_compat_ioctl(struct file *file, unsigned int cmd,
        }
 
        rv = alarm_do_ioctl(file, cmd, &ts);
+       if (rv)
+               return rv;
 
        switch (ANDROID_ALARM_BASE_CMD(cmd)) {
        case ANDROID_ALARM_GET_TIME(0): /* NOTE: we modified cmd above */

Quoted 24 lines · Dan Carpenter

If we pass an invalid clock type then "ts" is never set. We need to check for errors earlier, otherwise we end up passing uninitialized stack data to userspace.

Reported-by John Stultz <[email protected]>

Signed-off-by Dan Carpenter <[email protected]>

drivers/staging/android/alarm-dev.c +2 -0

index c8600d9..6dc27da 100644
@@ -297,6 +297,8 @@ static long alarm_compat_ioctl(struct file *file, unsigned int cmd,
         }
 
         rv = alarm_do_ioctl(file, cmd, &ts);
+       if (rv)
+               return rv;
 
         switch (ANDROID_ALARM_BASE_CMD(cmd)) {
         case ANDROID_ALARM_GET_TIME(0): /* NOTE: we modified cmd above */

@@ -305,7 +307,7 @@ static long alarm_compat_ioctl(struct file *file, unsigned int cmd,
                 break;
         }
 

drivers/staging/android/alarm-dev.c static long alarm_ioctl(struct file *file, unsigned int cmd, unsigned long arg)

Quoted 5 lines

drivers/staging/android/alarm-dev.c +1 -1

-       return rv;
+       return 0;
  }
  #endif
 

Is there a separate fix for alarm_ioctl? It seems to have the same problem.

Quoted 32 lines · Arve Hjønnevåg

Quoted 29 lines · Dan Carpenter

If we pass an invalid clock type then "ts" is never set. We need to check for errors earlier, otherwise we end up passing uninitialized stack data to userspace.

Reported-by John Stultz <[email protected]>

Signed-off-by Dan Carpenter <[email protected]>

drivers/staging/android/alarm-dev.c +2 -0

index c8600d9..6dc27da 100644
@@ -297,6 +297,8 @@ static long alarm_compat_ioctl(struct file *file,
 unsigned int cmd,
         }
 
         rv = alarm_do_ioctl(file, cmd, &ts);
+       if (rv)
+               return rv;
 
         switch (ANDROID_ALARM_BASE_CMD(cmd)) {

case ANDROID_ALARM_GET_TIME(0): /* NOTE: we modified cmd above
*/

drivers/staging/android/alarm-dev.c +0 -0

@@ -305,7 +307,7 @@ static long alarm_compat_ioctl(struct file *file,
 unsigned int cmd,
                 break;
         }
 

Quoted 7 lines

drivers/staging/android/alarm-dev.c static long alarm_ioctl(struct file *file, unsigned int cmd, unsigned long arg)

Quoted 5 lines

drivers/staging/android/alarm-dev.c +1 -1

-       return rv;
+       return 0;
  }
  #endif

Is there a separate fix for alarm_ioctl? It seems to have the same problem.

Yea, I CC'ed the kernel-team alias yesterday on Dan's original fix for the alarm_ioctl, which Greg has already queued.

This is just the follow-on fix to catch the same issue (as you also noted) in the compat_ioctl.

thanks
-john