Re: [CHECKER][PATCH] awe_wave.c user pointer dereference

[Hollis Blanchard]

On Thursday, Jun 5, 2003, at 17:52 US/Central, Andrew Morton wrote:

> Hollis Blanchard <[e-mail]> wrote:
>>
>> +			return -EFAULT;
>>  		return 0;
>>  		break;
>>
>> @@ -2063,10 +2064,12 @@
>>
>>  	case SNDCTL_SYNTH_MEMAVL:
>>  		return memsize - awe_free_mem_ptr() * 2;
>> +		break;
>>
>>  	default:
>>  		printk(KERN_WARNING "AWE32: unsupported ioctl %d\n", cmd);
>>  		return -EINVAL;
>> +		break;
>
> There's no need for a "break" after a "return"!

I've gotten a couple questions about that. :)

I did it for two reasons: 1) I'm a big fan of consistancy, and 3 of the 
5 ioctl cases already had the break. 2) should the returns be replaced 
with something like "retval = -EINVAL" and a single "return retval" at 
the end of the function, I can easily imagine visually missing the need 
for a break.

However, although a break after a return is fine with gcc -Wall, Joe 
Perches informs me that it will annoy lint and the Intel compiler. So 
here is the updated patch, which removes the (pre-existing! :) breaks 
from the other cases.

-- 
Hollis Blanchard
IBM Linux Technology Center

===== sound/oss/awe_wave.c 1.12 vs edited =====
--- 1.12/sound/oss/awe_wave.c	Thu Apr  3 16:35:48 2003
+++ edited/sound/oss/awe_wave.c	Fri Jun  6 11:17:19 2003
@@ -2046,20 +2046,18 @@
  			awe_info.nr_voices = awe_max_voices;
  		else
  			awe_info.nr_voices = AWE_MAX_CHANNELS;
-		memcpy((char*)arg, &awe_info, sizeof(awe_info));
+		if (copy_to_user((char*)arg, &awe_info, sizeof(awe_info)))
+			return -EFAULT;
  		return 0;
-		break;

  	case SNDCTL_SEQ_RESETSAMPLES:
  		awe_reset(dev);
  		awe_reset_samples();
  		return 0;
-		break;

  	case SNDCTL_SEQ_PERCMODE:
  		/* what's this? */
  		return 0;
-		break;

  	case SNDCTL_SYNTH_MEMAVL:
  		return memsize - awe_free_mem_ptr() * 2;
@@ -4314,7 +4312,8 @@
  	if (((cmd >> 8) & 0xff) != 'M')
  		return -EINVAL;

-	level = *(int*)arg;
+	if (get_user(level, (int *)arg))
+		return -EFAULT;
  	level = ((level & 0xff) + (level >> 8)) / 2;
  	DEBUG(0,printk("AWEMix: cmd=%x val=%d\n", cmd & 0xff, level));

@@ -4370,7 +4369,9 @@
  		level = 0;
  		break;
  	}
-	return *(int*)arg = level;
+	if (put_user(level, (int *)arg))
+		return -EFAULT;
+	return level;
  }
  #endif /* CONFIG_AWE32_MIXER */