UBI/DVB ioctl conflict?

UBI/DVB ioctl conflict?

[Josh Boyer]

I received a bug report where someone noticed that the UBI ioctls can
conflict with the dvb subsystem.  Looking it over, it seems that both
subsystems use 'o' as the magic, and do have a set of somewhat
conflicting sequence numbers as the secondary arg.

Is this a problem?

josh

Re: UBI/DVB ioctl conflict?

[Artem Bityutskiy]

On Sun, 2008-12-07 at 09:58 -0500, Josh Boyer wrote:
> I received a bug report where someone noticed that the UBI ioctls can
> conflict with the dvb subsystem.  Looking it over, it seems that both
> subsystems use 'o' as the magic, and do have a set of somewhat
> conflicting sequence numbers as the secondary arg.
> 
> Is this a problem?

Hmm, thanks for noticing.

I've looked at this, and thankfully it looks like we were lucky and do
not use the same 'ioctl()' numbers, by chance.

Ioctl number has the following structure:

bits 0-15: command (or sequence number)
bits 16-29: parameter size
bits 30-31: mode (read, write, etc).

We have the following overlaps with DVB subsystem:

#define AUDIO_STOP   _IO('o', 1)
#define UBI_IOCRMVOL _IOW('o', 1, int32_t)

#define AUDIO_PLAY   _IO('o', 2)
#define UBI_IOCRSVOL _IOW('o', 2, struct ubi_rsvol_req)

#define AUDIO_PAUSE  _IO('o', 3)
#define UBI_IOCRNVOL _IOW('o', 3, struct ubi_rnvol_req)

These are fine because parameter sizes are different, and because UBI
uses _IOW and DVB uses _IO, so the mode bits are "01" and "00".

And:

#define FE_DISEQC_RECV_SLAVE_REPLY _IOR('o', 64,
                                        struct dvb_diseqc_slave_reply)
#define UBI_IOCATT _IOW('o', 64, struct ubi_attach_req)

#define FE_DISEQC_SEND_BURST _IO('o', 65)
#define UBI_IOCDET _IOW('o', 65, int32_t)

are also fine because parameter sizes are different and the mode bits
are different.

However, we have to be very careful in the future. It seems like DVB
has been in the kernel long before UBI, so this potential conflict
would be my fault.

Neither DVB nor UBI seem not to be documented in
Documentation/ioctl/ioctl-number.txt. Should we do this?

P.S. Added Arnd to CC for suggestions, as well as LKML and DVB
maintainers.

-- 
Best regards,
Artem Bityutskiy (Битюцкий Артём)

Re: UBI/DVB ioctl conflict?

[Laurent Pinchart]

Hi,

On Monday 08 December 2008 07:20:26 Artem Bityutskiy wrote:
> On Sun, 2008-12-07 at 09:58 -0500, Josh Boyer wrote:
> > I received a bug report where someone noticed that the UBI ioctls can
> > conflict with the dvb subsystem.  Looking it over, it seems that both
> > subsystems use 'o' as the magic, and do have a set of somewhat
> > conflicting sequence numbers as the secondary arg.
> >
> > Is this a problem?
>
> Hmm, thanks for noticing.
>
> I've looked at this, and thankfully it looks like we were lucky and do
> not use the same 'ioctl()' numbers, by chance.
>
> Ioctl number has the following structure:
>
> bits 0-15: command (or sequence number)
> bits 16-29: parameter size
> bits 30-31: mode (read, write, etc).
>
> We have the following overlaps with DVB subsystem:
>
> #define AUDIO_STOP   _IO('o', 1)
> #define UBI_IOCRMVOL _IOW('o', 1, int32_t)
>
> #define AUDIO_PLAY   _IO('o', 2)
> #define UBI_IOCRSVOL _IOW('o', 2, struct ubi_rsvol_req)
>
> #define AUDIO_PAUSE  _IO('o', 3)
> #define UBI_IOCRNVOL _IOW('o', 3, struct ubi_rnvol_req)
>
> These are fine because parameter sizes are different, and because UBI
> uses _IOW and DVB uses _IO, so the mode bits are "01" and "00".
>
> And:
>
> #define FE_DISEQC_RECV_SLAVE_REPLY _IOR('o', 64,
>                                         struct dvb_diseqc_slave_reply)
> #define UBI_IOCATT _IOW('o', 64, struct ubi_attach_req)
>
> #define FE_DISEQC_SEND_BURST _IO('o', 65)
> #define UBI_IOCDET _IOW('o', 65, int32_t)
>
> are also fine because parameter sizes are different and the mode bits
> are different.
>
> However, we have to be very careful in the future. It seems like DVB
> has been in the kernel long before UBI, so this potential conflict
> would be my fault.
>
> Neither DVB nor UBI seem not to be documented in
> Documentation/ioctl/ioctl-number.txt. Should we do this?
>
> P.S. Added Arnd to CC for suggestions, as well as LKML and DVB
> maintainers.

Correct me if I'm wrong, but doesn't this only matters for devices that would 
implement both the UBI and DVB API on the same inode ? That would be quite 
unlikely.

Best regards,

-- 
Laurent Pinchart
CSE Semaphore Belgium

Chaussee de Bruxelles, 732A
B-1410 Waterloo
Belgium

T +32 (2) 387 42 59
F +32 (2) 387 42 75

Re: UBI/DVB ioctl conflict?

[Artem Bityutskiy]

On Mon, 2008-12-08 at 10:41 +0100, Laurent Pinchart wrote:
> Correct me if I'm wrong, but doesn't this only matters for devices that would 
> implement both the UBI and DVB API on the same inode ? That would be quite 
> unlikely.

Yeah, I guess. But this anyway makes sense to keep ioctls
non-overlapping.

-- 
Best regards,
Artem Bityutskiy (Битюцкий Артём)

Re: UBI/DVB ioctl conflict?

[Arnd Bergmann]

On Monday 08 December 2008, Artem Bityutskiy wrote:
> On Mon, 2008-12-08 at 10:41 +0100, Laurent Pinchart wrote:
> > Correct me if I'm wrong, but doesn't this only matters for devices that would 
> > implement both the UBI and DVB API on the same inode ? That would be quite 
> > unlikely.
> 
> Yeah, I guess. But this anyway makes sense to keep ioctls
> non-overlapping.

We try hard (but sometimes fail) to keep every ioctl number unique.
The reason for this is that the device drivers are not the only
pieces of code that look at them. Specifically, three other things
frequently cause problems here:

* strace wants to know about ioctl numbers so that it can show
the arguments in a meaningful way when tracing a program.

* the original 64 bit emulation for ioctl numbers in fs/compat_ioctl.c
assumes that it should translate specific calls in a given way. This
is not important if both device drivers handle all their ioctls through
their own ->compat_ioctl file operation.

* A number of binary emulation layers try to convert between different
formats (endianess, word size, ioctl numbers). The most common ones are
Linux-on-BSD, x86-on-ia64, x86-on-powerpc and Unix-on-Linux emulation
layers in user space.

	Arnd <><

Re: UBI/DVB ioctl conflict?

[Artem Bityutskiy]

Arnd Bergmann wrote:
> On Monday 08 December 2008, Artem Bityutskiy wrote:
>> On Mon, 2008-12-08 at 10:41 +0100, Laurent Pinchart wrote:
>>> Correct me if I'm wrong, but doesn't this only matters for devices that would 
>>> implement both the UBI and DVB API on the same inode ? That would be quite 
>>> unlikely.
>> Yeah, I guess. But this anyway makes sense to keep ioctls
>> non-overlapping.
> 
> We try hard (but sometimes fail) to keep every ioctl number unique.
> The reason for this is that the device drivers are not the only
> pieces of code that look at them. Specifically, three other things
> frequently cause problems here:

Thanks for the reply. Do you know the status of the
Documentation/ioctl/ioctl-number.txt file - it does not seem to be up-to-date.
Should I document add UBI ioctls there?

-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)

Re: UBI/DVB ioctl conflict?

[Arnd Bergmann]

On Monday 08 December 2008, Artem Bityutskiy wrote:
> Arnd Bergmann wrote:
> > We try hard (but sometimes fail) to keep every ioctl number unique.
> > The reason for this is that the device drivers are not the only
> > pieces of code that look at them. Specifically, three other things
> > frequently cause problems here:
> 
> Thanks for the reply. Do you know the status of the
> Documentation/ioctl/ioctl-number.txt file - it does not seem to be up-to-date.
> Should I document add UBI ioctls there?

Yes, please. The best we can do is if everyone documents his own stuff.

	Arnd <><

Re: UBI/DVB ioctl conflict?

[Jamie Lokier]

Laurent Pinchart wrote:
> Correct me if I'm wrong, but doesn't this only matters for devices
> that would implement both the UBI and DVB API on the same inode ?
> That would be quite unlikely.

Overlapping ioctls prevent strace(1) from showing ioctls properly.

For example, here's something you see often in strace of programs
using stdio, when it calls isatty():

ioctl(1, SNDCTL_TMR_TIMEBASE or TCGETS, 0xbfaaa864) = -1 ENOTTY (Inappropriate ioctl for device)

Or if it is a terminal:

ioctl(1, SNDCTL_TMR_TIMEBASE or TCGETS, {c_iflags=0x6d02, c_oflags=0x5, c_cflags=0x4bf, c_lflags=0x8a3b, c_line=0, c_cc="\x03\x1c\x7f\x15\x04\x00\x01\xff\x11\x13\x1a\xff\x12\x0f\x17\x16\xff\x00\x00"}) = 0

-- Jamie