linux-kernel - Re: [RFC] [PATCH] v4l2: use unsigned rather than enums in ioctl() structs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 12 Apr 2012 09:04:42 +0100
From:	James Courtier-Dutton <james.dutton@...il.com>
To:	Rémi Denis-Courmont <remi@...lab.net>
Cc:	Mauro Carvalho Chehab <mchehab@...hat.com>, mchehab@...radead.org,
	linux-media@...r.kernel.org, linux-kernel@...r.kernel.org,
	Alan Cox <alan@...rguk.ukuu.org.uk>
Subject: Re: [RFC] [PATCH] v4l2: use unsigned rather than enums in ioctl() structs

2012/4/11 Rémi Denis-Courmont <remi@...lab.net>:
>        Hello,
>
> Le mercredi 11 avril 2012 20:02:00 Mauro Carvalho Chehab, vous avez écrit :
>> Using unsigned instead of enum is not a good idea, from API POV, as
>> unsigned has different sizes on 32 bits and 64 bits.
>
> Fair enough. But then we can do that instead:
> typedef XXX __enum_t;
> where XXX is the unsigned integer with the right number of bits. Since Linux
> does not use short enums, this ought to work fine.
>
>> Yet, using enum was really a very bad idea, and, on all new stuff,
>> we're not accepting any new enum field.
>
> That is unfortunately not true. You do follow that rule for new fields to
> existing V4L2 structure. But you have been royally ignoring that rule when it
> comes to extending existing enumerations:
>
> linux-media does regularly add new enum values to existing enums. That is new
> stuff too, and every single time you do that, you do BREAK THE USERSPACE ABI.
> This is entirely unacceptable and against established kernel development
> policies.
>
> For instance, in Linux 3.1, V4L2_CTRL_TYPE_BITMASK was added. This broke
> userspace. And there are some pending patches adding more of the same thing...
> And V4L2_MEMORY_DMABUF will similarly break the user-to-kernel interface,
> which is yet worse.
>

I agree that breaking user-to-kernel interface is not advised.
We came across a similar problem some years ago with the ALSA sound
kernel drivers.
The solution we used was:
1) If a change is likely to change the user-to-kernel API, add a new
IOCTL for it.
Then old userland software can use the old IOCTL, and new userland
software can use the new IOCTL.
2) Add an version IOCTL that returns the current API level, so that
the app can be written to support more than one API interface,
depending on which kernel it is running on. The version IOCTL simply
returns an u32 value. This is a consistent part of the user-kernel API
that will never change.
3) Add "depreciated" compiler warnings to all the old API IOCTL calls,
so app developers know they should be working to update their apps.
4) After a few years, remove the old IOCTLs.
5) Use "uint32_t" and "uint64_t" types for all IOCTL calls, and not
"unsigned int" or "unsigned long int".
I.e. All structures passed in IOCTLs use fixed bit sized parameters
for everything except of course pointers. Pointers depend on
architecture.
6) Add a #if #endif around the old API, so a user compiling their own
kernel can decide if the old API exists or not. User might want to do
this for security reasons.

Kind Regards

James
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/