Re: 2.6.8-rc2-mm1

[Michael Hunold <hunold@convergence.de>]

Hi,

On 07/29/04 01:24, Johannes Stezenbach wrote:
> On Wed, Jul 28, 2004 at 11:44:23PM +0100, viro@parcelfarce.linux.theplanet.co.uk wrote:
> 
>>On Thu, Jul 29, 2004 at 12:24:55AM +0200, Johannes Stezenbach wrote:
>>
>>>Signed-off-by: Johannes Stezenbach <js@convergence.de>
>>>
>>>--- linux-2.6.8-rc2/drivers/media/dvb/dvb-core/dvb_functions.c.orig	2004-07-29 00:19:50.000000000 +0200
>>>+++ linux-2.6.8-rc2/drivers/media/dvb/dvb-core/dvb_functions.c	2004-07-29 00:20:05.000000000 +0200
>>>@@ -36,7 +36,7 @@ int dvb_usercopy(struct inode *inode, st
>>>         /*  Copy arguments into temp kernel buffer  */
>>>         switch (_IOC_DIR(cmd)) {
>>>         case _IOC_NONE:
>>>-                parg = NULL;
>>>+                parg = (void *) arg;
>>
>>Mind explaining why it is the right thing to do?  You are creating a kernel
>>pointer out of value passed to you by userland and feed it to a function
>>that expects a kernel pointer.  Which is an invitation for trouble - if
>>it ends up dereferenced, we are screwed and won't notice that.
> 
> 
> This is a hack introduced by someone years ago. The "pointer" is
> actually an integer argument, e.g. in include/linux/dvb/audio.h:
> 
> #define AUDIO_SET_MUTE             _IO('o', 6)
> 
> actually takes an integer argument (!0 mute, 0 unmute), so one can write
> 
> 	if (ioctl(fd, AUDIO_SET_MUTE, 1) == -1)
> 		perror("mute");
> 
> It is unusual (maybe even wrong?), but we cannot change it without
> losing binary API compatibility. However, I see that sparse might
> flag this as a possible bug :-(

Is this convenient trick considered harmful?
Or is it a creative way of using ioctls?

We're currently using this stuff in the overhauled DVB v4 API, too. So 
before we finally establish the DVB v4 API, I'd like to know if this is 
a no-no.

Comments?

CU
Michael.