* Re: [PATCH] Documentation: Add entry for dell-laptop sysfs interface
From: Brian Norris @ 2015-01-10 8:21 UTC (permalink / raw)
To: Darren Hart
Cc: Gabriele Mazzotta, mjg59, linux-api, platform-driver-x86,
Linux Kernel, libsmbios-devel, Srinivas_G_Gowda, Michael_E_Brown,
pali.rohar, Aaron Sloman
In-Reply-To: <20150110065714.GB104814@vmdeb7>
On Fri, Jan 9, 2015 at 10:57 PM, Darren Hart <dvhart@infradead.org> wrote:
> On Wed, Dec 31, 2014 at 12:20:59PM +0100, Gabriele Mazzotta wrote:
>> You are perfectly right, the documentation is wrong and I'm sorry for
>> that. als_setting should allow you to define when the keyboard
>> backlight has to be turned on or off depending on the value reported
>> by the ambient light sensor.
>>
>> Please note that not everything that is in libsmbios [1] is implemented
>> in the kernel driver. Take a look at it, recently there have been few
>> changes related to the ALS settings. Probably they are not needed given
>> that you are able to set the threshold with the current kernel driver,
>> but you might find something interesting.
>>
>> I'll submit a patch to correct the documentation, thanks for reporting.
>
> Have I missed a patch from you for this? I'd like to see this corrected in the
> 3.19 rc cycle.
https://lkml.org/lkml/2015/1/2/63
^ permalink raw reply
* Re: [PATCHv10 man-pages 5/5] execveat.2: initial man page for execveat(2)
From: Michael Kerrisk (man-pages) @ 2015-01-10 8:27 UTC (permalink / raw)
To: David Drysdale, Rich Felker
Cc: mtk.manpages, Eric W. Biederman, Andy Lutomirski, Alexander Viro,
Meredydd Luff, linux-kernel@vger.kernel.org, Andrew Morton,
David Miller, Thomas Gleixner, Stephen Rothwell, Oleg Nesterov,
Ingo Molnar, H. Peter Anvin, Kees Cook, Arnd Bergmann,
Christoph Hellwig, X86 ML, linux-arch, Linux API, sparclinux
In-Reply-To: <CAHse=S88Jy5ZKM_VY5onfvxX7dTMngnxuHfuLeSuzvKvQNP19A@mail.gmail.com>
On 01/09/2015 06:46 PM, David Drysdale wrote:
> On Fri, Jan 9, 2015 at 4:13 PM, Rich Felker <dalias@aerifal.cx> wrote:
>> On Fri, Jan 09, 2015 at 04:47:31PM +0100, Michael Kerrisk (man-pages) wrote:
>>> On 11/24/2014 12:53 PM, David Drysdale wrote:
>>>> Signed-off-by: David Drysdale <drysdale@google.com>
>>>> ---
>>>> man2/execveat.2 | 153 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>> 1 file changed, 153 insertions(+)
>>>> create mode 100644 man2/execveat.2
>>>
>>> David,
>>>
>>> Thanks for the very nicely prepared man page. I've done
>>> a few very light edits, and will release the version below
>>> with the next man-pages release.
>>>
>>> I have one question. In the message accompanying
>>> commit 51f39a1f0cea1cacf8c787f652f26dfee9611874 you wrote:
>>>
>>> The filename fed to the executed program as argv[0] (or the name of the
>>> script fed to a script interpreter) will be of the form "/dev/fd/<fd>"
>>> (for an empty filename) or "/dev/fd/<fd>/<filename>", effectively
>>> reflecting how the executable was found. This does however mean that
>>> execution of a script in a /proc-less environment won't work; also, script
>>> execution via an O_CLOEXEC file descriptor fails (as the file will not be
>>> accessible after exec).
>>>
>>> How does one produce this situation where the execed program sees
>>> argv[0] as a /dev/fd path? (i.e., what would the execveat()
>>> call look like?) I tried to produce this scenario, but could not.
>>
>> I think this is wrong. argv[0] is an arbitrary string provided by the
>> caller and would never be derived from the fd passed.
>
> Yeah, I think I just wrote that wrong, it's only relevant for scripts.
> As Rich says, for normal binaries argv[0] is just the argv[0] that
> was passed into the execve[at] call. For a script, the code in
> fs/binfmt_script.c will remove the original argv[0] and put the
> interpreter name and the script filename (e.g. "/bin/sh",
> "/dev/fd/6/script") in as 2 arguments in its place.
So, on reflection, I think it's worth saying something about this, and
I added the following text to the man page:
NOTES
When asked to execute a script file, the argv[0] that is passed
to the script interpreter is a string of the form /dev/fd/N or
/dev/fd/N/P, where N is the number of the file descriptor passed
via the dirfd argument. A string of the first form occurs when
AT_EMPTY_PATH is employed. A string of the second form occurs
when the script is specified via both dirfd and pathname; in this
case, P is the value given in pathname.
Thanks,
Michael
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
^ permalink raw reply
* [PATCH 0/2] Input: uinput - fix ioctl numbers in uapi/uinput.h
From: Gabriel Laskar @ 2015-01-10 12:43 UTC (permalink / raw)
To: linux-api-u79uwXL29TY76Z2rM5mHXA, Dmitry Torokhov, David Herrmann,
Peter Hutterer, Benjamin Tissoires
Cc: Gabriel Laskar
Ioctls numbers for UI_GET_SYSNAME and UI_GET_VERSION are incorrectly numbered,
since nr number is 8bit encoded, 300 and 301 will effectively get 44 and 45.
these two patches fixes this
Gabriel Laskar (2):
Input: uinput - fix ioctl nr overflow for UI_GET_SYSNAME
Input: uinput - fix ioctl nr overflow for UI_GET_VERSION
include/uapi/linux/uinput.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
--
2.2.1
^ permalink raw reply
* [PATCH 1/2] Input: uinput - fix ioctl nr overflow for UI_GET_SYSNAME
From: Gabriel Laskar @ 2015-01-10 12:43 UTC (permalink / raw)
To: linux-api-u79uwXL29TY76Z2rM5mHXA, Dmitry Torokhov, David Herrmann,
Peter Hutterer, Benjamin Tissoires
Cc: Gabriel Laskar
In-Reply-To: <1420893816-11620-1-git-send-email-gabriel-tU7rkvAWjlwhT4uAktR2oQ@public.gmane.org>
Request number for ioctls are encoded on 8bit. Values for are superior
to 255. The effective value is 0x2c. The effective ioctl number is still
the same one, it will not change the api in anyway.
Signed-off-by: Gabriel Laskar <gabriel-tU7rkvAWjlwhT4uAktR2oQ@public.gmane.org>
---
include/uapi/linux/uinput.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/uapi/linux/uinput.h b/include/uapi/linux/uinput.h
index baeab83..358f7d9 100644
--- a/include/uapi/linux/uinput.h
+++ b/include/uapi/linux/uinput.h
@@ -82,7 +82,7 @@ struct uinput_ff_erase {
* The complete sysfs path is then /sys/devices/virtual/input/--NAME--
* Usually, it is in the form "inputN"
*/
-#define UI_GET_SYSNAME(len) _IOC(_IOC_READ, UINPUT_IOCTL_BASE, 300, len)
+#define UI_GET_SYSNAME(len) _IOC(_IOC_READ, UINPUT_IOCTL_BASE, 0x2c, len)
/**
* UI_GET_VERSION - Return version of uinput protocol
--
2.2.1
^ permalink raw reply related
* [PATCH 2/2] Input: uinput - fix ioctl nr overflow for UI_GET_VERSION
From: Gabriel Laskar @ 2015-01-10 12:43 UTC (permalink / raw)
To: linux-api-u79uwXL29TY76Z2rM5mHXA, Dmitry Torokhov, David Herrmann,
Peter Hutterer, Benjamin Tissoires
Cc: Gabriel Laskar
In-Reply-To: <1420893816-11620-1-git-send-email-gabriel-tU7rkvAWjlwhT4uAktR2oQ@public.gmane.org>
Request number for ioctls are encoded on 8bit. Values for are superior
to 255. The effective value is 0x2d. The effective ioctl number is still
the same one, it will not change the api in anyway.
Signed-off-by: Gabriel Laskar <gabriel-tU7rkvAWjlwhT4uAktR2oQ@public.gmane.org>
---
include/uapi/linux/uinput.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/uapi/linux/uinput.h b/include/uapi/linux/uinput.h
index 358f7d9..e1daf2e 100644
--- a/include/uapi/linux/uinput.h
+++ b/include/uapi/linux/uinput.h
@@ -91,7 +91,7 @@ struct uinput_ff_erase {
* the integer pointed to by the ioctl argument. The protocol version
* is hard-coded in the kernel and is independent of the uinput device.
*/
-#define UI_GET_VERSION _IOR(UINPUT_IOCTL_BASE, 301, unsigned int)
+#define UI_GET_VERSION _IOR(UINPUT_IOCTL_BASE, 0x2d, unsigned int)
/*
* To write a force-feedback-capable driver, the upload_effect
--
2.2.1
^ permalink raw reply related
* Re: [PATCH] Documentation: Add entry for dell-laptop sysfs interface
From: Gabriele Mazzotta @ 2015-01-10 13:08 UTC (permalink / raw)
To: Darren Hart
Cc: Brian Norris, mjg59-1xO5oi07KQx4cg9Nei1l7Q,
linux-api-u79uwXL29TY76Z2rM5mHXA,
platform-driver-x86-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
libsmbios-devel-XtjxT7Vmt5ZskZv2Y/7f+AC/G2K4zDHf,
Srinivas_G_Gowda-8PEkshWhKlo, Michael_E_Brown-8PEkshWhKlo,
pali.rohar-Re5JQEeQqe8AvxtiuMwx3w,
A.Sloman-dxBOTFGcEFw2EctHIo1CcQ
In-Reply-To: <20150110065714.GB104814@vmdeb7>
On Friday 09 January 2015 22:57:14 Darren Hart wrote:
> On Wed, Dec 31, 2014 at 12:20:59PM +0100, Gabriele Mazzotta wrote:
> > On Tuesday 30 December 2014 21:31:49 Brian Norris wrote:
> > > Hi,
> > >
> > > On Wed, Dec 03, 2014 at 06:41:33PM +0100, Gabriele Mazzotta wrote:
> > > > Add the documentation for the new sysfs interface of dell-laptop
> > > > that allows to configure the keyboard illumination on Dell systems.
> > > >
> > > > Signed-off-by: Gabriele Mazzotta <gabriele.mzt-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > > > Signed-off-by: Pali Rohár <pali.rohar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > > > ---
> > > > .../ABI/testing/sysfs-platform-dell-laptop | 60 ++++++++++++++++++++++
> > > > 1 file changed, 60 insertions(+)
> > > > create mode 100644 Documentation/ABI/testing/sysfs-platform-dell-laptop
> > > >
> > > > diff --git a/Documentation/ABI/testing/sysfs-platform-dell-laptop b/Documentation/ABI/testing/sysfs-platform-dell-laptop
> > > > new file mode 100644
> > > > index 0000000..7969443
> > > > --- /dev/null
> > > > +++ b/Documentation/ABI/testing/sysfs-platform-dell-laptop
> > > > @@ -0,0 +1,60 @@
> > > > +What: /sys/class/leds/dell::kbd_backlight/als_setting
> > > > +Date: December 2014
> > > > +KernelVersion: 3.19
> > > > +Contact: Gabriele Mazzotta <gabriele.mzt-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
> > > > + Pali Rohár <pali.rohar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > > > +Description:
> > > > + This file allows to control the automatic keyboard
> > > > + illumination mode on some systems that have an ambient
> > > > + light sensor. Write 1 to this file to enable the auto
> > > > + mode, 0 to disable it.
> > > [...]
> > >
> > > This entry appears wrong, or at least incomplete. My system boots with a
> > > default value of 18, and the 'set' implementation accepts any value from
> > > 0 to 255.
> > >
> > > According to the comments in dell-laptop.c, the value actually means
> > > something different than on/off:
> > >
> > > cbArg3, byte0 Desired setting of ALS value that turns the light on or off.
> > >
> > > Admittedly, I'm not clear on what the intended interface is, as I've
> > > never had the ALS / keyboard backlight controls all working properly on
> > > my laptop in the first place, but I thought this would be the right
> > > place to ask about getting the documentation and driver in sync.
> >
> > Hi,
> >
> > You are perfectly right, the documentation is wrong and I'm sorry for
> > that. als_setting should allow you to define when the keyboard
> > backlight has to be turned on or off depending on the value reported
> > by the ambient light sensor.
> >
> > Please note that not everything that is in libsmbios [1] is implemented
> > in the kernel driver. Take a look at it, recently there have been few
> > changes related to the ALS settings. Probably they are not needed given
> > that you are able to set the threshold with the current kernel driver,
> > but you might find something interesting.
> >
> > I'll submit a patch to correct the documentation, thanks for reporting.
>
> Hi Gabriele,
>
> Have I missed a patch from you for this? I'd like to see this corrected in the
> 3.19 rc cycle.
>
> Thanks,
I'm sorry, I must have done something wrong when I sent the patch, the
CC list is not complete.
Thanks for the link Brian.
^ permalink raw reply
* Re: [PATCHv10 man-pages 5/5] execveat.2: initial man page for execveat(2)
From: Rich Felker @ 2015-01-10 13:31 UTC (permalink / raw)
To: Michael Kerrisk (man-pages)
Cc: David Drysdale, Eric W. Biederman, Andy Lutomirski,
Alexander Viro, Meredydd Luff, linux-kernel@vger.kernel.org,
Andrew Morton, David Miller, Thomas Gleixner, Stephen Rothwell,
Oleg Nesterov, Ingo Molnar, H. Peter Anvin, Kees Cook,
Arnd Bergmann, Christoph Hellwig, X86 ML, linux-arch, Linux API,
sparclinux
In-Reply-To: <54B0E282.7090203@gmail.com>
On Sat, Jan 10, 2015 at 09:27:46AM +0100, Michael Kerrisk (man-pages) wrote:
> On 01/09/2015 06:46 PM, David Drysdale wrote:
> > On Fri, Jan 9, 2015 at 4:13 PM, Rich Felker <dalias@aerifal.cx> wrote:
> >> On Fri, Jan 09, 2015 at 04:47:31PM +0100, Michael Kerrisk (man-pages) wrote:
> >>> On 11/24/2014 12:53 PM, David Drysdale wrote:
> >>>> Signed-off-by: David Drysdale <drysdale@google.com>
> >>>> ---
> >>>> man2/execveat.2 | 153 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >>>> 1 file changed, 153 insertions(+)
> >>>> create mode 100644 man2/execveat.2
> >>>
> >>> David,
> >>>
> >>> Thanks for the very nicely prepared man page. I've done
> >>> a few very light edits, and will release the version below
> >>> with the next man-pages release.
> >>>
> >>> I have one question. In the message accompanying
> >>> commit 51f39a1f0cea1cacf8c787f652f26dfee9611874 you wrote:
> >>>
> >>> The filename fed to the executed program as argv[0] (or the name of the
> >>> script fed to a script interpreter) will be of the form "/dev/fd/<fd>"
> >>> (for an empty filename) or "/dev/fd/<fd>/<filename>", effectively
> >>> reflecting how the executable was found. This does however mean that
> >>> execution of a script in a /proc-less environment won't work; also, script
> >>> execution via an O_CLOEXEC file descriptor fails (as the file will not be
> >>> accessible after exec).
> >>>
> >>> How does one produce this situation where the execed program sees
> >>> argv[0] as a /dev/fd path? (i.e., what would the execveat()
> >>> call look like?) I tried to produce this scenario, but could not.
> >>
> >> I think this is wrong. argv[0] is an arbitrary string provided by the
> >> caller and would never be derived from the fd passed.
> >
> > Yeah, I think I just wrote that wrong, it's only relevant for scripts.
> > As Rich says, for normal binaries argv[0] is just the argv[0] that
> > was passed into the execve[at] call. For a script, the code in
> > fs/binfmt_script.c will remove the original argv[0] and put the
> > interpreter name and the script filename (e.g. "/bin/sh",
> > "/dev/fd/6/script") in as 2 arguments in its place.
>
> So, on reflection, I think it's worth saying something about this, and
> I added the following text to the man page:
>
> NOTES
> When asked to execute a script file, the argv[0] that is passed
> to the script interpreter is a string of the form /dev/fd/N or
> /dev/fd/N/P, where N is the number of the file descriptor passed
> via the dirfd argument. A string of the first form occurs when
> AT_EMPTY_PATH is employed. A string of the second form occurs
> when the script is specified via both dirfd and pathname; in this
> case, P is the value given in pathname.
While I'm aware that you're simply documenting, it seems unnecessary
to me (and unnecessarily complicating of the cloexec issue) to have
the /dev/fd/N/P form. This could always be resolved by the kernel to a
single temp fd for the new process to use, and in fact it's probably
preferable to always get a "temp fd" in case the fd passed to fexecve
is NOT a throwaway one (e.g. if the original fd was stdin or
something); the program being executed should not have to use ugly and
error-prone heuristics to decide if it should close the exec fd.
On the other hand, this resolution could be done by userspace (open
with O_PATH|O_CLOEXEC prior to making the fexecveat syscall, and
always passing AT_EMPTY_PATH to the kernel) if desirable, so maybe it
doesn't make sense to have the kernel do it. In this sense the whole
"at" part of fexecveat becomes vestigial, though.
Any thoughts?
Rich
^ permalink raw reply
* Re: [PATCHv10 man-pages 5/5] execveat.2: initial man page for execveat(2)
From: Eric W. Biederman @ 2015-01-10 22:27 UTC (permalink / raw)
To: Rich Felker
Cc: Al Viro, David Drysdale, Michael Kerrisk (man-pages),
Andy Lutomirski, Meredydd Luff, linux-kernel@vger.kernel.org,
Andrew Morton, David Miller, Thomas Gleixner, Stephen Rothwell,
Oleg Nesterov, Ingo Molnar, H. Peter Anvin, Kees Cook,
Arnd Bergmann, Christoph Hellwig, X86 ML, linux-arch, Linux API,
sparclinux
In-Reply-To: <20150110055713.GE4574@brightrain.aerifal.cx>
Rich Felker <dalias@aerifal.cx> writes:
> On Sat, Jan 10, 2015 at 04:14:57AM +0000, Al Viro wrote:
>> Except that if your interpreter does stat(2) (or access(2), or getxattr(2),
>> etc.) before bothering with open(2), you'll get screwed.
>
> Yes, but I think that would be very bad interpreter design.
> stat/getxattr/access/whatever followed by open is always a TOCTOU
> race. The correct sequence of actions is always open followed by
> fstat/fgetxattr/...
Sigh. I think everyone who has looked at this has been blind.
If userspace is reasonable all we have to do is fix /proc/self/exe
for shell scripts to point at the actual script,
and then pass /proc/self/exe on the shell scripts command line.
At a practical level we have to worry about backwards compability and
chroot jails. But the existence of a clean implementation with
/proc/self/exe serves a proof of concept that it would not be too
difficult. When someone cares enough to implement it.
Eric
^ permalink raw reply
* Re: [PATCHv10 man-pages 5/5] execveat.2: initial man page for execveat(2)
From: Rich Felker @ 2015-01-11 1:15 UTC (permalink / raw)
To: Eric W. Biederman
Cc: Al Viro, David Drysdale, Michael Kerrisk (man-pages),
Andy Lutomirski, Meredydd Luff, linux-kernel@vger.kernel.org,
Andrew Morton, David Miller, Thomas Gleixner, Stephen Rothwell,
Oleg Nesterov, Ingo Molnar, H. Peter Anvin, Kees Cook,
Arnd Bergmann, Christoph Hellwig, X86 ML, linux-arch, Linux API,
sparclinux
In-Reply-To: <878uhaqnjo.fsf@x220.int.ebiederm.org>
On Sat, Jan 10, 2015 at 04:27:23PM -0600, Eric W. Biederman wrote:
> Rich Felker <dalias@aerifal.cx> writes:
>
> > On Sat, Jan 10, 2015 at 04:14:57AM +0000, Al Viro wrote:
>
> >> Except that if your interpreter does stat(2) (or access(2), or getxattr(2),
> >> etc.) before bothering with open(2), you'll get screwed.
> >
> > Yes, but I think that would be very bad interpreter design.
> > stat/getxattr/access/whatever followed by open is always a TOCTOU
> > race. The correct sequence of actions is always open followed by
> > fstat/fgetxattr/...
>
> Sigh. I think everyone who has looked at this has been blind.
>
> If userspace is reasonable all we have to do is fix /proc/self/exe
> for shell scripts to point at the actual script,
> and then pass /proc/self/exe on the shell scripts command line.
>
> At a practical level we have to worry about backwards compability and
> chroot jails. But the existence of a clean implementation with
> /proc/self/exe serves a proof of concept that it would not be too
> difficult. When someone cares enough to implement it.
Is /proc/self/exe a "magic symlink" that's bound to the inode, or just
a regular symlink? In the latter case it defeats the whole purpose of
using O_EXEC fds and fexecve rather than pathnames.
Rich
^ permalink raw reply
* Re: [PATCHv10 man-pages 5/5] execveat.2: initial man page for execveat(2)
From: Eric W. Biederman @ 2015-01-11 2:09 UTC (permalink / raw)
To: Rich Felker
Cc: Al Viro, David Drysdale, Michael Kerrisk (man-pages),
Andy Lutomirski, Meredydd Luff, linux-kernel@vger.kernel.org,
Andrew Morton, David Miller, Thomas Gleixner, Stephen Rothwell,
Oleg Nesterov, Ingo Molnar, H. Peter Anvin, Kees Cook,
Arnd Bergmann, Christoph Hellwig, X86 ML, linux-arch, Linux API,
sparclinux
In-Reply-To: <20150111011559.GG4574@brightrain.aerifal.cx>
Rich Felker <dalias@aerifal.cx> writes:
> On Sat, Jan 10, 2015 at 04:27:23PM -0600, Eric W. Biederman wrote:
>> Rich Felker <dalias@aerifal.cx> writes:
>>
>> > On Sat, Jan 10, 2015 at 04:14:57AM +0000, Al Viro wrote:
>>
>> >> Except that if your interpreter does stat(2) (or access(2), or getxattr(2),
>> >> etc.) before bothering with open(2), you'll get screwed.
>> >
>> > Yes, but I think that would be very bad interpreter design.
>> > stat/getxattr/access/whatever followed by open is always a TOCTOU
>> > race. The correct sequence of actions is always open followed by
>> > fstat/fgetxattr/...
>>
>> Sigh. I think everyone who has looked at this has been blind.
>>
>> If userspace is reasonable all we have to do is fix /proc/self/exe
>> for shell scripts to point at the actual script,
>> and then pass /proc/self/exe on the shell scripts command line.
>>
>> At a practical level we have to worry about backwards compability and
>> chroot jails. But the existence of a clean implementation with
>> /proc/self/exe serves a proof of concept that it would not be too
>> difficult. When someone cares enough to implement it.
>
> Is /proc/self/exe a "magic symlink" that's bound to the inode, or just
> a regular symlink? In the latter case it defeats the whole purpose of
> using O_EXEC fds and fexecve rather than pathnames.
In implementation /proc/self/exe is a named rather than a numbered file
descriptor. Essentially when loading an elf executable the file
descriptor is duped to the name /proc/self/exe. The implementation
otherwise is the same as /proc/self/fd/N.
The downside of course is that I expect if we were actually to change
/proc/self/exe from to point at the script instead of the shell some
piece of software somewhere would come melting down. I am totally not
ready to consider that kind of mine field today.
Eric
^ permalink raw reply
* [PATCH v9 0/2] crypto: AF_ALG: add AEAD and RNG support
From: Stephan Mueller @ 2015-01-11 3:44 UTC (permalink / raw)
To: 'Herbert Xu'
Cc: Daniel Borkmann, 'Quentin Gouchet', 'LKML',
linux-crypto, linux-api
Hi,
This patch set adds AEAD and RNG support to the AF_ALG interface
exported by the kernel crypto API. By extending AF_ALG with AEAD and RNG
support, all cipher types the kernel crypto API allows access to are
now accessible from userspace.
Both, AEAD and RNG implementations are stand-alone and do not depend
other AF_ALG interfaces (like hash or skcipher).
The AEAD implementation uses the same approach as provided with
skcipher by offering the following interfaces:
* sendmsg and recvmsg interfaces allowing multiple
invocations supporting a threaded user space. To support
multi-threaded user space, kernel-side buffering
is implemented similarly to skcipher.
* splice / vmsplice interfaces allowing a zero-copy
invocation
The RNG interface only implements the recvmsg interface as
zero-copy is not applicable.
The new AEAD and RNG interfaces are fully tested with the test application
provided at [1]. That test application exercises all newly added user space
interfaces. The testing covers:
* use of the sendmsg/recvmsg interface
* use of the splice / vmsplice interface
* invocation of all AF_ALG types (aead, rng, skcipher, hash)
* using all types of operation (encryption, decryption, keyed MD,
MD, random numbers, AEAD decryption with positive and negative
authentication verification)
* stress testing by running all tests for 30 minutes in an
endless loop
* test execution on 64 bit and 32 bit
[1] http://www.chronox.de/libkcapi.html
Changes v2:
* rebase to current cryptodev-2.6 tree
* use memzero_explicit to zeroize AEAD associated data
* use sizeof for determining length of AEAD associated data
* update algif_rng.c covering all suggestions from Daniel Borkmann
<dborkman@redhat.com>
* addition of patch 9: add digestsize interface for hashes
* addition of patch to update documentation covering the userspace interface
* change numbers of getsockopt options: separate them from sendmsg interface
definitions
Changes v3:
* remove getsockopt interface
* AEAD: associated data is set prepended to the plain/ciphertext
* AEAD: allowing arbitrary associated data lengths
* remove setkey patch as protection was already in the existing code
Changes v4:
* stand-alone implementation of AEAD
* testing of all interfaces offered by AEAD
* stress testing of AEAD and RNG
Changes v5:
* AEAD: add outer while(size) loop in aead_sendmsg to ensure all data is
copied into the kernel (reporter Herbert Xu)
* AEAD: aead_sendmsg bug fix: change size -= len; to size -= plen;
* AF_ALG / AEAD: add aead_setauthsize and associated extension to
struct af_alg_type as well as alg_setsockopt (reporter Herbert Xu)
* RNG: rng_recvmsg: use 128 byte stack variable for output of RNG instead
of ctx->result (reporter Herbert Xu)
* RNG / AF_ALG: allow user space to seed RNG via setsockopt
* RNG: rng_recvmsg bug fix: use genlen as result variable for
crypto_rng_get_bytes as previously no negative errors were obtained
* AF_ALG: alg_setop: zeroize buffer before free
Changes v6:
* AEAD/RNG: port to 3.19-rc1 with the iov_iter handling
* RNG: use the setkey interface to obtain the seed and drop the patch adding
a separate reseeding interface
* extract the zeroization patch for alg_setkey into a stand-alone patch
submission
* fix bug in aead_sufficient_data (reporter Herbert Xu)
* testing of all interfaces with test application provided with libkcapi version
0.6.2
Changes v7:
* AEAD: aead_recvmsg: change error code from ENOMEM to EINVAL
* AEAD: drop aead_readable/aead_sufficient_data and only use ctx->more to decide
whether the read side shall become active. This change requires that the
patch for crypto_aead_decrypt ensuring that the ciphertext contains the
authentication tag was added -- see https://lkml.org/lkml/2014/12/30/200.
Otherwise, user space can trigger a kernel crash.
* RNG: patch dropped as it was applied
* AEAD: port Kconfig/Makefile patch forward to current code base
Changes v8:
* Removed check for aead_assoclen in aead_sendmsg
* Fix endless loop bug in aead_sendmsg (check for sgl->cur > ALG_MAX_PAGES in
while condition removed -- this condition is checked within the loop already)
* Resurrect aead_sufficient_data and call it in aead_sendmsg, aead_sendpage to
notify caller about wrong invocation
* Re-add aead_sufficient_data to aead_recvmsg to verify user input data before
using them to ensure the kernel protects against malicious parameters
* Allow arbitrary size of AD (i.e. up to the maximum buffer size of
ALG_MAX_PAGES)
* When aead_recvmsg receives an error from decryption, release all pages if the
error is EBADMSG -- this error implies that a proper decryption was performed
but the integrity of the message is lost. This error is considered to be a
valid decryption result.
* Add test cases for sendmsg and splice interface to test large AD sizes (in
case of sendmsg, use 65504 bytes AD and 32 bytes plaintext; in case of splice
use 15 pages AD and 32 bytes in the 16th page for plaintext). See [1] for
updated test case.
Changes v9:
* if socket is not writable during sendmsg/sendpage due to insufficient memory
and a recvmsg operation is forced, inform userspace about truncated operation
via MSG_TRUNC
* use -EMSGSIZE in case insufficient data was provided in sendmsg/sendpage
* release all buffers in case insufficient data was provided in sendmsg/sendpage
* bug fix in sendmsg: when a new page is allocated, reset sg->offset to 0 --
the error is visible with the new tests in [1] when using the -d flag
with the test application
Stephan Mueller (2):
crypto: AF_ALG: add AEAD support
crypto: AF_ALG: enable AEAD interface compilation
crypto/Kconfig | 9 +
crypto/Makefile | 1 +
crypto/algif_aead.c | 679 ++++++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 689 insertions(+)
create mode 100644 crypto/algif_aead.c
--
2.1.0
^ permalink raw reply
* [PATCH v9 1/2] crypto: AF_ALG: add AEAD support
From: Stephan Mueller @ 2015-01-11 3:45 UTC (permalink / raw)
To: 'Herbert Xu'
Cc: Daniel Borkmann, 'Quentin Gouchet', 'LKML',
linux-crypto-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <41794881.9gBey4al6X-PJstQz4BMNNP20K/wil9xYQuADTiUCJX@public.gmane.org>
This patch adds the AEAD support for AF_ALG.
The implementation is based on algif_skcipher, but contains heavy
modifications to streamline the interface for AEAD uses.
To use AEAD, the user space consumer has to use the salg_type named
"aead".
The AEAD implementation includes some overhead to calculate the size of
the ciphertext, because the AEAD implementation of the kernel crypto API
makes implied assumption on the location of the authentication tag. When
performing an encryption, the tag will be added to the created
ciphertext (note, the tag is placed adjacent to the ciphertext). For
decryption, the caller must hand in the ciphertext with the tag appended
to the ciphertext. Therefore, the selection of the used memory
needs to add/subtract the tag size from the source/destination buffers
depending on the encryption type. The code is provided with comments
explaining when and how that operation is performed.
A fully working example using all aspects of AEAD is provided at
http://www.chronox.de/libkcapi.html
Signed-off-by: Stephan Mueller <smueller-T9tCv8IpfcWELgA04lAiVw@public.gmane.org>
---
crypto/algif_aead.c | 679 ++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 679 insertions(+)
create mode 100644 crypto/algif_aead.c
diff --git a/crypto/algif_aead.c b/crypto/algif_aead.c
new file mode 100644
index 0000000..ed41577
--- /dev/null
+++ b/crypto/algif_aead.c
@@ -0,0 +1,679 @@
+/*
+ * algif_aead: User-space interface for AEAD algorithms
+ *
+ * Copyright (C) 2014, Stephan Mueller <smueller-T9tCv8IpfcWELgA04lAiVw@public.gmane.org>
+ *
+ * This file provides the user-space API for AEAD ciphers.
+ *
+ * This file is derived from algif_skcipher.c.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the Free
+ * Software Foundation; either version 2 of the License, or (at your option)
+ * any later version.
+ */
+
+#include <crypto/scatterwalk.h>
+#include <crypto/if_alg.h>
+#include <linux/init.h>
+#include <linux/list.h>
+#include <linux/kernel.h>
+#include <linux/mm.h>
+#include <linux/module.h>
+#include <linux/net.h>
+#include <net/sock.h>
+
+struct aead_sg_list {
+ unsigned int cur;
+ struct scatterlist sg[ALG_MAX_PAGES];
+};
+
+struct aead_ctx {
+ struct aead_sg_list tsgl;
+ struct af_alg_sgl rsgl;
+
+ void *iv;
+
+ struct af_alg_completion completion;
+
+ unsigned long used;
+
+ unsigned int len;
+ bool more;
+ bool merge;
+ bool enc;
+ bool trunc;
+
+ size_t aead_assoclen;
+ struct aead_request aead_req;
+};
+
+static inline int aead_sndbuf(struct sock *sk)
+{
+ struct alg_sock *ask = alg_sk(sk);
+ struct aead_ctx *ctx = ask->private;
+
+ return max_t(int, max_t(int, sk->sk_sndbuf & PAGE_MASK, PAGE_SIZE) -
+ ctx->used, 0);
+}
+
+static inline bool aead_writable(struct sock *sk)
+{
+ return PAGE_SIZE <= aead_sndbuf(sk);
+}
+
+static inline bool aead_sufficient_data(struct aead_ctx *ctx)
+{
+ unsigned as = crypto_aead_authsize(crypto_aead_reqtfm(&ctx->aead_req));
+
+ return (ctx->used >= (ctx->aead_assoclen + (ctx->enc ? 0 : as)));
+}
+
+static void aead_put_sgl(struct sock *sk)
+{
+ struct alg_sock *ask = alg_sk(sk);
+ struct aead_ctx *ctx = ask->private;
+ struct aead_sg_list *sgl = &ctx->tsgl;
+ struct scatterlist *sg = sgl->sg;
+ unsigned int i;
+
+ for (i = 0; i < sgl->cur; i++) {
+ if (!sg_page(sg + i))
+ continue;
+
+ put_page(sg_page(sg + i));
+ sg_assign_page(sg + i, NULL);
+ }
+ sgl->cur = 0;
+ ctx->used = 0;
+ ctx->more = 0;
+ ctx->merge = 0;
+ ctx->trunc = 0;
+}
+
+static int aead_wait_for_wmem(struct sock *sk, unsigned flags)
+{
+ long timeout;
+ DEFINE_WAIT(wait);
+ int err = -ERESTARTSYS;
+
+ if (flags & MSG_DONTWAIT)
+ return -EAGAIN;
+
+ set_bit(SOCK_ASYNC_NOSPACE, &sk->sk_socket->flags);
+
+ for (;;) {
+ if (signal_pending(current))
+ break;
+ prepare_to_wait(sk_sleep(sk), &wait, TASK_INTERRUPTIBLE);
+ timeout = MAX_SCHEDULE_TIMEOUT;
+ if (sk_wait_event(sk, &timeout, aead_writable(sk))) {
+ err = 0;
+ break;
+ }
+ }
+ finish_wait(sk_sleep(sk), &wait);
+
+ return err;
+}
+
+static void aead_wmem_wakeup(struct sock *sk)
+{
+ struct socket_wq *wq;
+
+ if (!aead_writable(sk))
+ return;
+
+ rcu_read_lock();
+ wq = rcu_dereference(sk->sk_wq);
+ if (wq_has_sleeper(wq))
+ wake_up_interruptible_sync_poll(&wq->wait, POLLIN |
+ POLLRDNORM |
+ POLLRDBAND);
+ sk_wake_async(sk, SOCK_WAKE_WAITD, POLL_IN);
+ rcu_read_unlock();
+}
+
+static int aead_wait_for_data(struct sock *sk, unsigned flags)
+{
+ struct alg_sock *ask = alg_sk(sk);
+ struct aead_ctx *ctx = ask->private;
+ long timeout;
+ DEFINE_WAIT(wait);
+ int err = -ERESTARTSYS;
+
+ if (flags & MSG_DONTWAIT) {
+ return -EAGAIN;
+ }
+
+ set_bit(SOCK_ASYNC_WAITDATA, &sk->sk_socket->flags);
+
+ for (;;) {
+ if (signal_pending(current))
+ break;
+ prepare_to_wait(sk_sleep(sk), &wait, TASK_INTERRUPTIBLE);
+ timeout = MAX_SCHEDULE_TIMEOUT;
+ if (sk_wait_event(sk, &timeout, !ctx->more)) {
+ err = 0;
+ break;
+ }
+ }
+ finish_wait(sk_sleep(sk), &wait);
+
+ clear_bit(SOCK_ASYNC_WAITDATA, &sk->sk_socket->flags);
+
+ return err;
+}
+
+static void aead_data_wakeup(struct sock *sk)
+{
+ struct alg_sock *ask = alg_sk(sk);
+ struct aead_ctx *ctx = ask->private;
+ struct socket_wq *wq;
+
+ if (ctx->more)
+ return;
+
+ rcu_read_lock();
+ wq = rcu_dereference(sk->sk_wq);
+ if (wq_has_sleeper(wq))
+ wake_up_interruptible_sync_poll(&wq->wait, POLLOUT |
+ POLLRDNORM |
+ POLLRDBAND);
+ sk_wake_async(sk, SOCK_WAKE_SPACE, POLL_OUT);
+ rcu_read_unlock();
+}
+
+static int aead_sendmsg(struct kiocb *unused, struct socket *sock,
+ struct msghdr *msg, size_t size)
+{
+ struct sock *sk = sock->sk;
+ struct alg_sock *ask = alg_sk(sk);
+ struct aead_ctx *ctx = ask->private;
+ unsigned ivsize =
+ crypto_aead_ivsize(crypto_aead_reqtfm(&ctx->aead_req));
+ struct aead_sg_list *sgl = &ctx->tsgl;
+ struct af_alg_control con = {};
+ long copied = 0;
+ bool enc = 0;
+ bool init = 0;
+ int err = -EINVAL;
+
+ if (msg->msg_controllen) {
+ err = af_alg_cmsg_send(msg, &con);
+ if (err)
+ return err;
+
+ init = 1;
+ switch (con.op) {
+ case ALG_OP_ENCRYPT:
+ enc = 1;
+ break;
+ case ALG_OP_DECRYPT:
+ enc = 0;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ if (con.iv && con.iv->ivlen != ivsize)
+ return -EINVAL;
+ }
+
+ lock_sock(sk);
+ if (!ctx->more && ctx->used)
+ goto unlock;
+
+ if (init) {
+ ctx->enc = enc;
+ if (con.iv)
+ memcpy(ctx->iv, con.iv->iv, ivsize);
+
+ ctx->aead_assoclen = con.aead_assoclen;
+ }
+
+ while (size) {
+ unsigned long len = size;
+ struct scatterlist *sg = NULL;
+
+ /* use the existing memory in an allocated page */
+ if (ctx->merge) {
+ sg = sgl->sg + sgl->cur - 1;
+ len = min_t(unsigned long, len,
+ PAGE_SIZE - sg->offset - sg->length);
+ err = memcpy_from_msg(page_address(sg_page(sg)) +
+ sg->offset + sg->length,
+ msg, len);
+ if (err)
+ goto unlock;
+
+ sg->length += len;
+ ctx->merge = (sg->offset + sg->length) &
+ (PAGE_SIZE - 1);
+
+ ctx->used += len;
+ copied += len;
+ size -= len;
+ }
+
+ if (!aead_writable(sk)) {
+ /*
+ * If there is more data to be expected, but we cannot
+ * write more data, forcefully define that we do not
+ * expect more data to invoke the AEAD operation. This
+ * prevents a deadlock in user space.
+ */
+ ctx->more = 0;
+ ctx->trunc = 1;
+ err = aead_wait_for_wmem(sk, msg->msg_flags);
+ if (err)
+ goto unlock;
+ }
+
+ /* allocate a new page */
+ len = min_t(unsigned long, size, aead_sndbuf(sk));
+ while (len) {
+ int plen = 0;
+
+ if (sgl->cur >= ALG_MAX_PAGES) {
+ err = -E2BIG;
+ goto unlock;
+ }
+
+ sg = sgl->sg + sgl->cur;
+ plen = min_t(int, len, PAGE_SIZE);
+
+ sg_assign_page(sg, alloc_page(GFP_KERNEL));
+ err = -ENOMEM;
+ if (!sg_page(sg))
+ goto unlock;
+
+ err = memcpy_from_msg(page_address(sg_page(sg)),
+ msg, plen);
+ if (err) {
+ __free_page(sg_page(sg));
+ sg_assign_page(sg, NULL);
+ goto unlock;
+ }
+
+ sg->offset = 0;
+ sg->length = plen;
+ len -= plen;
+ ctx->used += plen;
+ copied += plen;
+ sgl->cur++;
+ size -= plen;
+ ctx->merge = plen & (PAGE_SIZE - 1);
+ }
+ }
+
+ err = 0;
+
+ ctx->more = msg->msg_flags & MSG_MORE;
+ if (!ctx->more && !aead_sufficient_data(ctx)) {
+ aead_put_sgl(sk);
+ err = -EMSGSIZE;
+ }
+
+unlock:
+ aead_data_wakeup(sk);
+ release_sock(sk);
+
+ return err ?: copied;
+}
+
+static ssize_t aead_sendpage(struct socket *sock, struct page *page,
+ int offset, size_t size, int flags)
+{
+ struct sock *sk = sock->sk;
+ struct alg_sock *ask = alg_sk(sk);
+ struct aead_ctx *ctx = ask->private;
+ struct aead_sg_list *sgl = &ctx->tsgl;
+ int err = -EINVAL;
+
+ if (flags & MSG_SENDPAGE_NOTLAST)
+ flags |= MSG_MORE;
+
+ if (sgl->cur >= ALG_MAX_PAGES)
+ return -E2BIG;
+
+ lock_sock(sk);
+ if (!ctx->more && ctx->used)
+ goto unlock;
+
+ if (!size)
+ goto done;
+
+ if (!aead_writable(sk)) {
+ /* see aead_sendmsg why more is set to 0 */
+ ctx->more = 0;
+ ctx->trunc = 1;
+ err = aead_wait_for_wmem(sk, flags);
+ if (err)
+ goto unlock;
+ }
+
+ ctx->merge = 0;
+
+ get_page(page);
+ sg_set_page(sgl->sg + sgl->cur, page, size, offset);
+ sgl->cur++;
+ ctx->used += size;
+
+ err = 0;
+
+done:
+ ctx->more = flags & MSG_MORE;
+ if (!ctx->more && !aead_sufficient_data(ctx)) {
+ aead_put_sgl(sk);
+ err = -EMSGSIZE;
+ }
+
+unlock:
+ aead_data_wakeup(sk);
+ release_sock(sk);
+
+ return err ?: size;
+}
+
+static int aead_recvmsg(struct kiocb *unused, struct socket *sock,
+ struct msghdr *msg, size_t ignored, int flags)
+{
+ struct sock *sk = sock->sk;
+ struct alg_sock *ask = alg_sk(sk);
+ struct aead_ctx *ctx = ask->private;
+ unsigned bs = crypto_aead_blocksize(crypto_aead_reqtfm(&ctx->aead_req));
+ unsigned as = crypto_aead_authsize(crypto_aead_reqtfm(&ctx->aead_req));
+ struct aead_sg_list *sgl = &ctx->tsgl;
+ struct scatterlist *sg = sgl->sg;
+ struct scatterlist assoc[ALG_MAX_PAGES];
+ size_t assoclen = 0;
+ unsigned int i = 0;
+ int err = -EINVAL;
+ unsigned long used = 0;
+ unsigned long outlen = 0;
+
+ /*
+ * Require exactly one IOV block as the AEAD operation is a one shot
+ * due to the authentication tag.
+ */
+ if (msg->msg_iter.nr_segs != 1)
+ return -ENOMSG;
+
+ lock_sock(sk);
+ /*
+ * AEAD memory structure: For encryption, the tag is appended to the
+ * ciphertext which implies that the memory allocated for the ciphertext
+ * must be increased by the tag length. For decryption, the tag
+ * is expected to be concatenated to the ciphertext. The plaintext
+ * therefore has a memory size of the ciphertext minus the tag length.
+ *
+ * The memory structure for cipher operation has the following
+ * structure:
+ * AEAD encryption input: assoc data || plaintext
+ * AEAD encryption output: cipherntext || auth tag
+ * AEAD decryption input: assoc data || ciphertext || auth tag
+ * AEAD decryption output: plaintext
+ */
+
+ if (ctx->more) {
+ err = aead_wait_for_data(sk, flags);
+ if (err)
+ goto unlock;
+ }
+
+ used = ctx->used;
+
+ /*
+ * Make sure sufficient data is present -- note, the same check is
+ * is also present in sendmsg/sendpage. The checks in sendpage/sendmsg
+ * shall provide an information to the data sender that something is
+ * wrong, but they are irrelevant to maintain the kernel integrity.
+ * We need this check here too in case user space decides to not honor
+ * the error message in sendmsg/sendpage and still call recvmsg. This
+ * check here protects the kernel integrity.
+ */
+ if (!aead_sufficient_data(ctx))
+ goto unlock;
+
+ /*
+ * The cipher operation input data is reduced by the associated data
+ * length as this data is processed separately later on.
+ */
+ used -= ctx->aead_assoclen;
+
+ if (ctx->enc) {
+ /* round up output buffer to multiple of block size */
+ outlen = ((used + bs - 1) / bs * bs);
+ /* add the size needed for the auth tag to be created */
+ outlen += as;
+ } else {
+ /* output data size is input without the authentication tag */
+ outlen = used - as;
+ /* round up output buffer to multiple of block size */
+ outlen = ((outlen + bs - 1) / bs * bs);
+ }
+
+ /* ensure output buffer is sufficiently large */
+ if (msg->msg_iter.iov->iov_len < outlen)
+ goto unlock;
+
+ outlen = af_alg_make_sg(&ctx->rsgl, msg->msg_iter.iov->iov_base,
+ outlen, 1);
+ err = outlen;
+ if (err < 0)
+ goto unlock;
+
+ err = -EINVAL;
+ sg_init_table(assoc, ALG_MAX_PAGES);
+ assoclen = ctx->aead_assoclen;
+ /*
+ * Split scatterlist into two: first part becomes AD, second part
+ * is plaintext / ciphertext. The first part is assigned to assoc
+ * scatterlist. When this loop finishes, sg points to the start of the
+ * plaintext / ciphertext.
+ */
+ for(i = 0; i < ctx->tsgl.cur; i++) {
+ sg = sgl->sg + i;
+ if (sg->length <= assoclen) {
+ /* AD is larger than one page */
+ sg_set_page(assoc + i, sg_page(sg),
+ sg->length, sg->offset);
+ assoclen -= sg->length;
+ if (i >= ctx->tsgl.cur)
+ goto unlock;
+ } else if (!assoclen) {
+ /* current page is to start of plaintext / ciphertext */
+ if (i)
+ /* AD terminates at page boundary */
+ sg_mark_end(assoc + i - 1);
+ else
+ /* AD size is zero */
+ sg_mark_end(assoc);
+ break;
+ } else {
+ /* AD does not terminate at page boundary */
+ sg_set_page(assoc + i, sg_page(sg),
+ assoclen, sg->offset);
+ sg_mark_end(assoc + i);
+ /* plaintext / ciphertext starts after AD */
+ sg->length -= assoclen;
+ sg->offset += assoclen;
+ break;
+ }
+ }
+
+ aead_request_set_assoc(&ctx->aead_req, assoc, ctx->aead_assoclen);
+ aead_request_set_crypt(&ctx->aead_req, sg, ctx->rsgl.sg, used, ctx->iv);
+
+ err = af_alg_wait_for_completion(ctx->enc ?
+ crypto_aead_encrypt(&ctx->aead_req) :
+ crypto_aead_decrypt(&ctx->aead_req),
+ &ctx->completion);
+
+ af_alg_free_sg(&ctx->rsgl);
+
+ /* indicate userspace that we processed incomplete data */
+ if (ctx->trunc)
+ msg->msg_flags |= MSG_TRUNC;
+
+ if (err) {
+ /* EBADMSG implies a valid cipher operation took place */
+ if (err == -EBADMSG)
+ aead_put_sgl(sk);
+ goto unlock;
+ }
+
+ aead_put_sgl(sk);
+
+ err = 0;
+
+unlock:
+ aead_wmem_wakeup(sk);
+ release_sock(sk);
+
+ return err ? err : outlen;
+}
+
+static unsigned int aead_poll(struct file *file, struct socket *sock,
+ poll_table *wait)
+{
+ struct sock *sk = sock->sk;
+ struct alg_sock *ask = alg_sk(sk);
+ struct aead_ctx *ctx = ask->private;
+ unsigned int mask;
+
+ sock_poll_wait(file, sk_sleep(sk), wait);
+ mask = 0;
+
+ if (!ctx->more)
+ mask |= POLLIN | POLLRDNORM;
+
+ if (aead_writable(sk))
+ mask |= POLLOUT | POLLWRNORM | POLLWRBAND;
+
+ return mask;
+}
+
+static struct proto_ops algif_aead_ops = {
+ .family = PF_ALG,
+
+ .connect = sock_no_connect,
+ .socketpair = sock_no_socketpair,
+ .getname = sock_no_getname,
+ .ioctl = sock_no_ioctl,
+ .listen = sock_no_listen,
+ .shutdown = sock_no_shutdown,
+ .getsockopt = sock_no_getsockopt,
+ .mmap = sock_no_mmap,
+ .bind = sock_no_bind,
+ .accept = sock_no_accept,
+ .setsockopt = sock_no_setsockopt,
+
+ .release = af_alg_release,
+ .sendmsg = aead_sendmsg,
+ .sendpage = aead_sendpage,
+ .recvmsg = aead_recvmsg,
+ .poll = aead_poll,
+};
+
+static void *aead_bind(const char *name, u32 type, u32 mask)
+{
+ return crypto_alloc_aead(name, type, mask);
+}
+
+static void aead_release(void *private)
+{
+ crypto_free_aead(private);
+}
+
+static int aead_setauthsize(void *private, unsigned int authsize)
+{
+ return crypto_aead_setauthsize(private, authsize);
+}
+
+static int aead_setkey(void *private, const u8 *key, unsigned int keylen)
+{
+ return crypto_aead_setkey(private, key, keylen);
+}
+
+static void aead_sock_destruct(struct sock *sk)
+{
+ struct alg_sock *ask = alg_sk(sk);
+ struct aead_ctx *ctx = ask->private;
+ unsigned int ivlen = crypto_aead_ivsize(
+ crypto_aead_reqtfm(&ctx->aead_req));
+
+ aead_put_sgl(sk);
+ sock_kzfree_s(sk, ctx->iv, ivlen);
+ sock_kfree_s(sk, ctx, ctx->len);
+ af_alg_release_parent(sk);
+}
+
+static int aead_accept_parent(void *private, struct sock *sk)
+{
+ struct aead_ctx *ctx;
+ struct alg_sock *ask = alg_sk(sk);
+ unsigned int len = sizeof(*ctx) + crypto_aead_reqsize(private);
+ unsigned int ivlen = crypto_aead_ivsize(private);
+
+ ctx = sock_kmalloc(sk, len, GFP_KERNEL);
+ if (!ctx)
+ return -ENOMEM;
+ memset(ctx, 0, len);
+
+ ctx->iv = sock_kmalloc(sk, ivlen, GFP_KERNEL);
+ if (!ctx->iv) {
+ sock_kfree_s(sk, ctx, len);
+ return -ENOMEM;
+ }
+ memset(ctx->iv, 0, ivlen);
+
+ ctx->len = len;
+ ctx->used = 0;
+ ctx->more = 0;
+ ctx->merge = 0;
+ ctx->enc = 0;
+ ctx->tsgl.cur = 0;
+ ctx->aead_assoclen = 0;
+ af_alg_init_completion(&ctx->completion);
+ sg_init_table(ctx->tsgl.sg, ALG_MAX_PAGES);
+
+ ask->private = ctx;
+
+ aead_request_set_tfm(&ctx->aead_req, private);
+ aead_request_set_callback(&ctx->aead_req, CRYPTO_TFM_REQ_MAY_BACKLOG,
+ af_alg_complete, &ctx->completion);
+
+ sk->sk_destruct = aead_sock_destruct;
+
+ return 0;
+}
+
+static const struct af_alg_type algif_type_aead = {
+ .bind = aead_bind,
+ .release = aead_release,
+ .setkey = aead_setkey,
+ .setauthsize = aead_setauthsize,
+ .accept = aead_accept_parent,
+ .ops = &algif_aead_ops,
+ .name = "aead",
+ .owner = THIS_MODULE
+};
+
+static int __init algif_aead_init(void)
+{
+ return af_alg_register_type(&algif_type_aead);
+}
+
+static void __exit algif_aead_exit(void)
+{
+ int err = af_alg_unregister_type(&algif_type_aead);
+ BUG_ON(err);
+}
+
+module_init(algif_aead_init);
+module_exit(algif_aead_exit);
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Stephan Mueller <smueller-T9tCv8IpfcWELgA04lAiVw@public.gmane.org>");
+MODULE_DESCRIPTION("AEAD kernel crypto API user space interface");
--
2.1.0
^ permalink raw reply related
* [PATCH v9 2/2] crypto: AF_ALG: enable AEAD interface compilation
From: Stephan Mueller @ 2015-01-11 3:46 UTC (permalink / raw)
To: 'Herbert Xu'
Cc: Daniel Borkmann, 'Quentin Gouchet', 'LKML',
linux-crypto-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <41794881.9gBey4al6X-PJstQz4BMNNP20K/wil9xYQuADTiUCJX@public.gmane.org>
Enable compilation of the AEAD AF_ALG support and provide a Kconfig
option to compile the AEAD AF_ALG support.
Signed-off-by: Stephan Mueller <smueller-T9tCv8IpfcWELgA04lAiVw@public.gmane.org>
---
crypto/Kconfig | 9 +++++++++
crypto/Makefile | 1 +
2 files changed, 10 insertions(+)
diff --git a/crypto/Kconfig b/crypto/Kconfig
index 50f4da4..41a3fc5 100644
--- a/crypto/Kconfig
+++ b/crypto/Kconfig
@@ -1523,6 +1523,15 @@ config CRYPTO_USER_API_RNG
This option enables the user-spaces interface for random
number generator algorithms.
+config CRYPTO_USER_API_AEAD
+ tristate "User-space interface for AEAD cipher algorithms"
+ depends on NET
+ select CRYPTO_AEAD
+ select CRYPTO_USER_API
+ help
+ This option enables the user-spaces interface for AEAD
+ cipher algorithms.
+
config CRYPTO_HASH_INFO
bool
diff --git a/crypto/Makefile b/crypto/Makefile
index ba19465..97b7d3a 100644
--- a/crypto/Makefile
+++ b/crypto/Makefile
@@ -100,6 +100,7 @@ obj-$(CONFIG_CRYPTO_USER_API) += af_alg.o
obj-$(CONFIG_CRYPTO_USER_API_HASH) += algif_hash.o
obj-$(CONFIG_CRYPTO_USER_API_SKCIPHER) += algif_skcipher.o
obj-$(CONFIG_CRYPTO_USER_API_RNG) += algif_rng.o
+obj-$(CONFIG_CRYPTO_USER_API_AEAD) += algif_aead.o
#
# generic algorithms and the async_tx api
--
2.1.0
^ permalink raw reply related
* Re: [PATCHv10 man-pages 5/5] execveat.2: initial man page for execveat(2)
From: Christoph Hellwig @ 2015-01-11 11:02 UTC (permalink / raw)
To: Eric W. Biederman
Cc: Rich Felker, Al Viro, David Drysdale, Michael Kerrisk (man-pages),
Andy Lutomirski, Meredydd Luff,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Andrew Morton, David Miller, Thomas Gleixner, Stephen Rothwell,
Oleg Nesterov, Ingo Molnar, H. Peter Anvin, Kees Cook,
Arnd Bergmann, Christoph Hellwig, X86 ML, linux-arch, Linux API,
sparclinux-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <87oaq6oypl.fsf-JOvCrm2gF+uungPnsOpG7nhyD016LWXt@public.gmane.org>
On Sat, Jan 10, 2015 at 08:09:10PM -0600, Eric W. Biederman wrote:
> In implementation /proc/self/exe is a named rather than a numbered file
> descriptor. Essentially when loading an elf executable the file
> descriptor is duped to the name /proc/self/exe. The implementation
> otherwise is the same as /proc/self/fd/N.
>
> The downside of course is that I expect if we were actually to change
> /proc/self/exe from to point at the script instead of the shell some
> piece of software somewhere would come melting down. I am totally not
> ready to consider that kind of mine field today.
We could add a /proc/self/script that points to the script, and either
is not available or still points to the executable if we are not running
a script.
^ permalink raw reply
* Re: [PATCHv2 1/9] media: Fix DVB representation at media controller API
From: Laurent Pinchart @ 2015-01-11 13:50 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Linux Media Mailing List, Mauro Carvalho Chehab,
linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <ea1dd8e443b34e2047468866ec423d4334f54eba.1420294938.git.mchehab-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
Hi Mauro,
Thank you for the patch.
On Saturday 03 January 2015 12:49:03 Mauro Carvalho Chehab wrote:
> The DVB devices are identified via a (major, minor) tuple,
> and not by a random id. Fix it, before we start using it.
>
> Signed-off-by: Mauro Carvalho Chehab <mchehab-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
>
> diff --git a/include/media/media-entity.h b/include/media/media-entity.h
> index e00459185d20..de333cc8261b 100644
> --- a/include/media/media-entity.h
> +++ b/include/media/media-entity.h
> @@ -97,7 +97,10 @@ struct media_entity {
> u32 device;
> u32 subdevice;
> } alsa;
> - int dvb;
> + struct {
> + u32 major;
> + u32 minor;
> + } dvb;
>
> /* Sub-device specifications */
> /* Nothing needed yet */
> diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
> index d847c760e8f0..7902e800f019 100644
> --- a/include/uapi/linux/media.h
> +++ b/include/uapi/linux/media.h
> @@ -27,7 +27,7 @@
> #include <linux/types.h>
> #include <linux/version.h>
>
> -#define MEDIA_API_VERSION KERNEL_VERSION(0, 1, 0)
> +#define MEDIA_API_VERSION KERNEL_VERSION(0, 1, 1)
>
> struct media_device_info {
> char driver[16];
> @@ -88,7 +88,10 @@ struct media_entity_desc {
> __u32 device;
> __u32 subdevice;
> } alsa;
> - int dvb;
> + struct {
> + __u32 major;
> + __u32 minor;
> + } dvb;
Won't this break compilation of existing userspace code ? As DVB is not
properly supported in MC at the moment we could consider that only mediactl
will be affected, so it shouldn't be a big issue.
>
> /* Sub-device specifications */
> /* Nothing needed yet */
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCHv3 01/20] media: add new types for DVB devnodes
From: Laurent Pinchart @ 2015-01-11 13:54 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Sakari Ailus, Linux Media Mailing List, Mauro Carvalho Chehab,
linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20150108154450.654d04d6-iErKd7e5oVp+urZeOPWqwQ@public.gmane.org>
Hi Mauro,
On Thursday 08 January 2015 15:44:50 Mauro Carvalho Chehab wrote:
> Em Thu, 08 Jan 2015 18:10:13 +0200 Laurent Pinchart escreveu:
> > On Wednesday 07 January 2015 12:22:39 Mauro Carvalho Chehab wrote:
> >> Em Wed, 07 Jan 2015 16:09:04 +0200 Sakari Ailus escreveu:
> >>> Mauro Carvalho Chehab wrote:
> >>>> Most of the DVB subdevs have already their own devnode.
> >>>>
> >>>> Add support for them at the media controller API.
> >>>>
> >>>> Signed-off-by: Mauro Carvalho Chehab <mchehab-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
> >>>>
> >>>> diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
> >>>> index 7902e800f019..707db275f92b 100644
> >>>> --- a/include/uapi/linux/media.h
> >>>> +++ b/include/uapi/linux/media.h
> >>>> @@ -50,7 +50,14 @@ struct media_device_info {
> >>>>
> >>>> #define MEDIA_ENT_T_DEVNODE_V4L (MEDIA_ENT_T_DEVNODE + 1)
> >>>> #define MEDIA_ENT_T_DEVNODE_FB (MEDIA_ENT_T_DEVNODE + 2)
> >>>> #define MEDIA_ENT_T_DEVNODE_ALSA (MEDIA_ENT_T_DEVNODE + 3)
> >>>>
> >>>> -#define MEDIA_ENT_T_DEVNODE_DVB (MEDIA_ENT_T_DEVNODE + 4)
> >>>> +#define MEDIA_ENT_T_DEVNODE_DVB_FE (MEDIA_ENT_T_DEVNODE + 4)
> >>>> +#define MEDIA_ENT_T_DEVNODE_DVB_DEMUX (MEDIA_ENT_T_DEVNODE + 5)
> >>>> +#define MEDIA_ENT_T_DEVNODE_DVB_DVR (MEDIA_ENT_T_DEVNODE + 6)
> >>>> +#define MEDIA_ENT_T_DEVNODE_DVB_CA (MEDIA_ENT_T_DEVNODE + 7)
> >>>> +#define MEDIA_ENT_T_DEVNODE_DVB_NET (MEDIA_ENT_T_DEVNODE + 8)
> >>>
> >>> I'd create another type for the DVB sub-type devices, as there is for
> >>> V4L2 sub-devices. I wonder what Laurent thinks.
> >>
> >> I discussed this quickly with Laurent on IRC.
> >>
> >> There are some concept differences between V4L2 and DVB.
> >>
> >> At v4l2:
> >> - the spec is one monolitic header (videodev2.h);
> >> - one devnode is used to control everyhing (/dev/video?)
> >> - there is one v4l core for all types of devices
> >>
> >> At DVB:
> >> - each different DVB API has its own header;
> >> - each DVB device type has its own core (ok, they're
> >> linked into one module, but internally they're almost independent);
> >> - each different DVB API has its own devnode.
> >>
> >> So, using "SUBDEV" for DVB (or at least for the devnodes) don't
> >> make much sense.
> >>
> >> Ok, there are still some things at DVB side that could be mapped as
> >> subdev. The clear example is the tuner. However, in this case, the
> >> same tuner can be either V4L, DVB or both. So, we need to define just
> >> one subdev type for the tuner.
> >>
> >> Also, each DVB device can be identified via major/minor pairs.
> >>
> >> I wrote already (and submitted upstream) the patches for media-ctl to
> >>
> >> recognize them. They're also on my experimental v4l-utils tree:
> >> http://git.linuxtv.org/cgit.cgi/mchehab/experimental-v4l-> >> utils.git/log/?h=dvb-media-ctl>
> >
> > As I've mentioned in a previous discussion, the media_entity type field is
> > too restrictive. Not only does this use case show that we need a type,
> > sub-type and sub-sub-type, there are also entities that implement several
> > distinct types. I thus believe we need a new ioctl is needed to expose
> > detailed information about entities. This topic has been discussed
> > numerous times in the past, it "just" requires someone to implement it.
> >
> > I'm not opposed to a short-term solution like the one proposed here, but
> > maybe we should instead decide it's time to implement the new ioctl
> > instead.
>
> Ok, so let's stick with it for DVB. At DVB side, I don't see a need for
> sub-sub-type, especially since DVB has no subdevs (except for the shared
> tuner between DVB and V4L). Everything there are devnodes, with their
> functionality strictly following the documentation, as the API is fully
> handled inside the DVB core[1].
At the moment the MC API has a devnode type with a DVB devnode subtype.
Splitting DVB devnodes into different categories effectively create sub-
subtypes. That's what bothers mode, the type field is becoming a ragbag.
> Also, I don't want to mix adding DVB media controller support with the
> addition of a new ioctl.
*If* we conclude that a new ioctl is needed to support DVB in a clean way, I
don't see why that new ioctl shouldn't be considered as a prerequisite.
> [1] For the non-deprecated DVB devnodes. DVB have 3 devnode types that
> are deprecated because they implement functionality found elsewhere
> (video, audio and OSD dvb APIs). Only one legacy driver implements it,
> and there's no plan to ever add media controller or expand/accept
> new drivers using those legacy APIs.
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCHv2 1/9] media: Fix DVB representation at media controller API
From: Mauro Carvalho Chehab @ 2015-01-11 13:58 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Linux Media Mailing List, Mauro Carvalho Chehab,
linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <2274549.Xm2J2SkQ3y@avalon>
Em Sun, 11 Jan 2015 15:50:04 +0200
Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org> escreveu:
> Hi Mauro,
>
> Thank you for the patch.
>
> On Saturday 03 January 2015 12:49:03 Mauro Carvalho Chehab wrote:
> > The DVB devices are identified via a (major, minor) tuple,
> > and not by a random id. Fix it, before we start using it.
> >
> > Signed-off-by: Mauro Carvalho Chehab <mchehab-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
> >
> > diff --git a/include/media/media-entity.h b/include/media/media-entity.h
> > index e00459185d20..de333cc8261b 100644
> > --- a/include/media/media-entity.h
> > +++ b/include/media/media-entity.h
> > @@ -97,7 +97,10 @@ struct media_entity {
> > u32 device;
> > u32 subdevice;
> > } alsa;
> > - int dvb;
> > + struct {
> > + u32 major;
> > + u32 minor;
> > + } dvb;
> >
> > /* Sub-device specifications */
> > /* Nothing needed yet */
> > diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
> > index d847c760e8f0..7902e800f019 100644
> > --- a/include/uapi/linux/media.h
> > +++ b/include/uapi/linux/media.h
> > @@ -27,7 +27,7 @@
> > #include <linux/types.h>
> > #include <linux/version.h>
> >
> > -#define MEDIA_API_VERSION KERNEL_VERSION(0, 1, 0)
> > +#define MEDIA_API_VERSION KERNEL_VERSION(0, 1, 1)
> >
> > struct media_device_info {
> > char driver[16];
> > @@ -88,7 +88,10 @@ struct media_entity_desc {
> > __u32 device;
> > __u32 subdevice;
> > } alsa;
> > - int dvb;
> > + struct {
> > + __u32 major;
> > + __u32 minor;
> > + } dvb;
>
> Won't this break compilation of existing userspace code ? As DVB is not
> properly supported in MC at the moment we could consider that only mediactl
> will be affected, so it shouldn't be a big issue.
Well, media-ctl uses a local copy of the videodev2.h header, so it won't
break.
I'm not aware of any other application using MC for DVB.
Yet, imagining that such application exists, then, IMHO, it is better
to break compilation for it, as probably such application was written for
some OOT driver that might be using its own version of the media
controller implementation.
Regards,
Mauro
^ permalink raw reply
* Re: [PATCH 1/7] tuner-core: properly initialize media controller subdev
From: Laurent Pinchart @ 2015-01-11 14:02 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Linux Media Mailing List, Mauro Carvalho Chehab, Hans Verkuil,
Prabhakar Lad, Sakari Ailus, linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <4ff2de5fce002a6f6f87993440f45e0f198c57cb.1420315245.git.mchehab-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
Hi Mauro,
Thank you for the patch.
On Saturday 03 January 2015 18:09:33 Mauro Carvalho Chehab wrote:
> Properly initialize tuner core subdev at the media controller.
>
> That requires a new subtype at the media controller API.
>
> Signed-off-by: Mauro Carvalho Chehab <mchehab-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
>
> diff --git a/drivers/media/v4l2-core/tuner-core.c
> b/drivers/media/v4l2-core/tuner-core.c index 559f8372e2eb..114715ed0110
> 100644
> --- a/drivers/media/v4l2-core/tuner-core.c
> +++ b/drivers/media/v4l2-core/tuner-core.c
> @@ -134,6 +134,9 @@ struct tuner {
> unsigned int type; /* chip type id */
> void *config;
> const char *name;
> +#if defined(CONFIG_MEDIA_CONTROLLER)
> + struct media_pad pad;
> +#endif
I'm not too familiar with tuners, do they all have a single output only and no
input ?
> };
>
> /*
> @@ -434,6 +437,8 @@ static void set_type(struct i2c_client *c, unsigned int
> type, t->name = analog_ops->info.name;
> }
>
> + t->sd.entity.name = t->name;
> +
Entity information is not supposed to change at runtime, I'm not sure to be
comfortable with this change.
set_type() is called at probe time and in tuner_s_type_addr(). The former just
duplicates the name initialization in tuner_probe(), so isn't really needed.
The later bothers me.
> tuner_dbg("type set to %s\n", t->name);
>
> t->mode_mask = new_mode_mask;
> @@ -592,6 +597,7 @@ static int tuner_probe(struct i2c_client *client,
> struct tuner *t;
> struct tuner *radio;
> struct tuner *tv;
> + int ret;
>
> t = kzalloc(sizeof(struct tuner), GFP_KERNEL);
> if (NULL == t)
> @@ -696,6 +702,15 @@ register_client:
> t->type,
> t->mode_mask & T_RADIO ? " Radio" : "",
> t->mode_mask & T_ANALOG_TV ? " TV" : "");
> +#if defined(CONFIG_MEDIA_CONTROLLER)
> + t->pad.flags = MEDIA_PAD_FL_SOURCE;
> + t->sd.entity.type = MEDIA_ENT_T_V4L2_SUBDEV_TUNER;
> + t->sd.entity.name = t->name;
v4l2_subdev_init(), called by v4l2_i2c_subdev_init(), sets sd->entity.name to
point to sd->name. Is there a reason why the subdev name can't be used as the
entity name ?
> +
> + ret = media_entity_init(&t->sd.entity, 1, &t->pad, 0);
> + if (ret < 0)
> + tuner_err("failed to initialize media entity!\n");
> +#endif
> return 0;
> }
>
> diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
> index 707db275f92b..5ffde035789b 100644
> --- a/include/uapi/linux/media.h
> +++ b/include/uapi/linux/media.h
> @@ -66,6 +66,8 @@ struct media_device_info {
> /* A converter of analogue video to its digital representation. */
> #define MEDIA_ENT_T_V4L2_SUBDEV_DECODER (MEDIA_ENT_T_V4L2_SUBDEV + 4)
>
> +#define MEDIA_ENT_T_V4L2_SUBDEV_TUNER (MEDIA_ENT_T_V4L2_SUBDEV + 5)
> +
> #define MEDIA_ENT_FL_DEFAULT (1 << 0)
>
> struct media_entity_desc {
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCHv2 1/9] media: Fix DVB representation at media controller API
From: Laurent Pinchart @ 2015-01-11 14:05 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Linux Media Mailing List, Mauro Carvalho Chehab,
linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20150111115824.0e4acdf0-+RedX5hVuTR+urZeOPWqwQ@public.gmane.org>
Hi Mauro,
On Sunday 11 January 2015 11:58:24 Mauro Carvalho Chehab wrote:
> Em Sun, 11 Jan 2015 15:50:04 +0200 Laurent Pinchart escreveu:
> > On Saturday 03 January 2015 12:49:03 Mauro Carvalho Chehab wrote:
> >> The DVB devices are identified via a (major, minor) tuple,
> >> and not by a random id. Fix it, before we start using it.
> >>
> >> Signed-off-by: Mauro Carvalho Chehab <mchehab-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
> >>
> >> diff --git a/include/media/media-entity.h b/include/media/media-entity.h
> >> index e00459185d20..de333cc8261b 100644
> >> --- a/include/media/media-entity.h
> >> +++ b/include/media/media-entity.h
> >> @@ -97,7 +97,10 @@ struct media_entity {
> >> u32 device;
> >> u32 subdevice;
> >> } alsa;
> >> - int dvb;
> >> + struct {
> >> + u32 major;
> >> + u32 minor;
> >> + } dvb;
> >>
> >> /* Sub-device specifications */
> >> /* Nothing needed yet */
> >> diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
> >> index d847c760e8f0..7902e800f019 100644
> >> --- a/include/uapi/linux/media.h
> >> +++ b/include/uapi/linux/media.h
> >> @@ -27,7 +27,7 @@
> >> #include <linux/types.h>
> >> #include <linux/version.h>
> >>
> >> -#define MEDIA_API_VERSION KERNEL_VERSION(0, 1, 0)
> >> +#define MEDIA_API_VERSION KERNEL_VERSION(0, 1, 1)
> >>
> >> struct media_device_info {
> >> char driver[16];
> >> @@ -88,7 +88,10 @@ struct media_entity_desc {
> >> __u32 device;
> >> __u32 subdevice;
> >> } alsa;
> >> - int dvb;
> >> + struct {
> >> + __u32 major;
> >> + __u32 minor;
> >> + } dvb;
> >
> > Won't this break compilation of existing userspace code ? As DVB is not
> > properly supported in MC at the moment we could consider that only
> > mediactl will be affected, so it shouldn't be a big issue.
>
> Well, media-ctl uses a local copy of the videodev2.h header, so it won't
> break.
It's media.h, but you're correct here.
> I'm not aware of any other application using MC for DVB.
>
> Yet, imagining that such application exists, then, IMHO, it is better
> to break compilation for it, as probably such application was written for
> some OOT driver that might be using its own version of the media
> controller implementation.
OK. I'll remember that argument the next time I want to break a kernel API
though ;-)
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCHv3 01/20] media: add new types for DVB devnodes
From: Mauro Carvalho Chehab @ 2015-01-11 14:09 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Sakari Ailus, Linux Media Mailing List, Mauro Carvalho Chehab,
linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <5047178.zZItJZYJNH@avalon>
Em Sun, 11 Jan 2015 15:54:14 +0200
Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org> escreveu:
> Hi Mauro,
>
> On Thursday 08 January 2015 15:44:50 Mauro Carvalho Chehab wrote:
> > Em Thu, 08 Jan 2015 18:10:13 +0200 Laurent Pinchart escreveu:
> > > On Wednesday 07 January 2015 12:22:39 Mauro Carvalho Chehab wrote:
> > >> Em Wed, 07 Jan 2015 16:09:04 +0200 Sakari Ailus escreveu:
> > >>> Mauro Carvalho Chehab wrote:
> > >>>> Most of the DVB subdevs have already their own devnode.
> > >>>>
> > >>>> Add support for them at the media controller API.
> > >>>>
> > >>>> Signed-off-by: Mauro Carvalho Chehab <mchehab-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
> > >>>>
> > >>>> diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
> > >>>> index 7902e800f019..707db275f92b 100644
> > >>>> --- a/include/uapi/linux/media.h
> > >>>> +++ b/include/uapi/linux/media.h
> > >>>> @@ -50,7 +50,14 @@ struct media_device_info {
> > >>>>
> > >>>> #define MEDIA_ENT_T_DEVNODE_V4L (MEDIA_ENT_T_DEVNODE + 1)
> > >>>> #define MEDIA_ENT_T_DEVNODE_FB (MEDIA_ENT_T_DEVNODE + 2)
> > >>>> #define MEDIA_ENT_T_DEVNODE_ALSA (MEDIA_ENT_T_DEVNODE + 3)
> > >>>>
> > >>>> -#define MEDIA_ENT_T_DEVNODE_DVB (MEDIA_ENT_T_DEVNODE + 4)
> > >>>> +#define MEDIA_ENT_T_DEVNODE_DVB_FE (MEDIA_ENT_T_DEVNODE + 4)
> > >>>> +#define MEDIA_ENT_T_DEVNODE_DVB_DEMUX (MEDIA_ENT_T_DEVNODE + 5)
> > >>>> +#define MEDIA_ENT_T_DEVNODE_DVB_DVR (MEDIA_ENT_T_DEVNODE + 6)
> > >>>> +#define MEDIA_ENT_T_DEVNODE_DVB_CA (MEDIA_ENT_T_DEVNODE + 7)
> > >>>> +#define MEDIA_ENT_T_DEVNODE_DVB_NET (MEDIA_ENT_T_DEVNODE + 8)
> > >>>
> > >>> I'd create another type for the DVB sub-type devices, as there is for
> > >>> V4L2 sub-devices. I wonder what Laurent thinks.
> > >>
> > >> I discussed this quickly with Laurent on IRC.
> > >>
> > >> There are some concept differences between V4L2 and DVB.
> > >>
> > >> At v4l2:
> > >> - the spec is one monolitic header (videodev2.h);
> > >> - one devnode is used to control everyhing (/dev/video?)
> > >> - there is one v4l core for all types of devices
> > >>
> > >> At DVB:
> > >> - each different DVB API has its own header;
> > >> - each DVB device type has its own core (ok, they're
> > >> linked into one module, but internally they're almost independent);
> > >> - each different DVB API has its own devnode.
> > >>
> > >> So, using "SUBDEV" for DVB (or at least for the devnodes) don't
> > >> make much sense.
> > >>
> > >> Ok, there are still some things at DVB side that could be mapped as
> > >> subdev. The clear example is the tuner. However, in this case, the
> > >> same tuner can be either V4L, DVB or both. So, we need to define just
> > >> one subdev type for the tuner.
> > >>
> > >> Also, each DVB device can be identified via major/minor pairs.
> > >>
> > >> I wrote already (and submitted upstream) the patches for media-ctl to
> > >>
> > >> recognize them. They're also on my experimental v4l-utils tree:
> > >> http://git.linuxtv.org/cgit.cgi/mchehab/experimental-v4l-> >> utils.git/log/?h=dvb-media-ctl>
> > >
> > > As I've mentioned in a previous discussion, the media_entity type field is
> > > too restrictive. Not only does this use case show that we need a type,
> > > sub-type and sub-sub-type, there are also entities that implement several
> > > distinct types. I thus believe we need a new ioctl is needed to expose
> > > detailed information about entities. This topic has been discussed
> > > numerous times in the past, it "just" requires someone to implement it.
> > >
> > > I'm not opposed to a short-term solution like the one proposed here, but
> > > maybe we should instead decide it's time to implement the new ioctl
> > > instead.
> >
> > Ok, so let's stick with it for DVB. At DVB side, I don't see a need for
> > sub-sub-type, especially since DVB has no subdevs (except for the shared
> > tuner between DVB and V4L). Everything there are devnodes, with their
> > functionality strictly following the documentation, as the API is fully
> > handled inside the DVB core[1].
>
> At the moment the MC API has a devnode type with a DVB devnode subtype.
> Splitting DVB devnodes into different categories effectively create sub-
> subtypes. That's what bothers mode, the type field is becoming a ragbag.
As I explained before, and more detailed at:
http://www.spinics.net/lists/linux-media/msg85229.html
DVB is actually 3 different APIs (demux, frontend and CA). Those APIs
are independent, and each have an independent devnode. The dvr is a
special case: it is a devnode that doesn't accept ioctl's. It is just
an output devnode, controlled vis the demux API.
The only thing in common to those 3 APIs is that they belong to the same
device type. But some may argue that, on an hybrid device, the input,
remote controller and even V4L2 are also sub-types.
In any case, they're all devnodes, so MEDIA_ENT_T_DEVNODE_foo fits well.
>
> > Also, I don't want to mix adding DVB media controller support with the
> > addition of a new ioctl.
>
> *If* we conclude that a new ioctl is needed to support DVB in a clean way, I
> don't see why that new ioctl shouldn't be considered as a prerequisite.
I don't see the need of a new ioctl for DVB. DVB is even simpler than V4L2,
as it doesn't have subdevs (except for tuner).
>
> > [1] For the non-deprecated DVB devnodes. DVB have 3 devnode types that
> > are deprecated because they implement functionality found elsewhere
> > (video, audio and OSD dvb APIs). Only one legacy driver implements it,
> > and there's no plan to ever add media controller or expand/accept
> > new drivers using those legacy APIs.
>
^ permalink raw reply
* Re: [PATCH 1/7] tuner-core: properly initialize media controller subdev
From: Mauro Carvalho Chehab @ 2015-01-11 14:25 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Linux Media Mailing List, Mauro Carvalho Chehab, Hans Verkuil,
Prabhakar Lad, Sakari Ailus, linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <3223125.oELRrvBOf4@avalon>
Em Sun, 11 Jan 2015 16:02:41 +0200
Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org> escreveu:
> Hi Mauro,
>
> Thank you for the patch.
>
> On Saturday 03 January 2015 18:09:33 Mauro Carvalho Chehab wrote:
> > Properly initialize tuner core subdev at the media controller.
> >
> > That requires a new subtype at the media controller API.
> >
> > Signed-off-by: Mauro Carvalho Chehab <mchehab-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
> >
> > diff --git a/drivers/media/v4l2-core/tuner-core.c
> > b/drivers/media/v4l2-core/tuner-core.c index 559f8372e2eb..114715ed0110
> > 100644
> > --- a/drivers/media/v4l2-core/tuner-core.c
> > +++ b/drivers/media/v4l2-core/tuner-core.c
> > @@ -134,6 +134,9 @@ struct tuner {
> > unsigned int type; /* chip type id */
> > void *config;
> > const char *name;
> > +#if defined(CONFIG_MEDIA_CONTROLLER)
> > + struct media_pad pad;
> > +#endif
>
> I'm not too familiar with tuners, do they all have a single output only and no
> input ?
They have an input: the antenna connector. However, I don't see any need
to map it for most tuners, as there's generally just one input, hardwired
into the tuner chip.
There are some hardware with 2 antenna connectors, but for different
functions (FM and TV). They're selected automatically when the V4L2
driver switches between FM and TV.
In any case, the tuner-core doesn't provide any way to select the
antenna input.
So, if a driver would need to select the input, it would either need
to not use tuner-core or some patch will be required to add such
functionality inside tuner-core.
> > };
> >
> > /*
> > @@ -434,6 +437,8 @@ static void set_type(struct i2c_client *c, unsigned int
> > type, t->name = analog_ops->info.name;
> > }
> >
> > + t->sd.entity.name = t->name;
> > +
>
> Entity information is not supposed to change at runtime, I'm not sure to be
> comfortable with this change.
>
> set_type() is called at probe time and in tuner_s_type_addr(). The former just
> duplicates the name initialization in tuner_probe(), so isn't really needed.
> The later bothers me.
The tuner-core driver is a "core subdev" implementation. It handles
the ioctl logic, but the actual driver is a different one. It also
have internally a probe logic that will load the correct tuner subdev.
The tuner_s_type_addr() callback, used only at bridge probing time,
is a way for the bridge driver to provide the name of the tuner driver
that should be loaded, plus its I2C address.
So, once the board is probed, the name shouldn't change.
>
> > tuner_dbg("type set to %s\n", t->name);
> >
> > t->mode_mask = new_mode_mask;
> > @@ -592,6 +597,7 @@ static int tuner_probe(struct i2c_client *client,
> > struct tuner *t;
> > struct tuner *radio;
> > struct tuner *tv;
> > + int ret;
> >
> > t = kzalloc(sizeof(struct tuner), GFP_KERNEL);
> > if (NULL == t)
> > @@ -696,6 +702,15 @@ register_client:
> > t->type,
> > t->mode_mask & T_RADIO ? " Radio" : "",
> > t->mode_mask & T_ANALOG_TV ? " TV" : "");
> > +#if defined(CONFIG_MEDIA_CONTROLLER)
> > + t->pad.flags = MEDIA_PAD_FL_SOURCE;
> > + t->sd.entity.type = MEDIA_ENT_T_V4L2_SUBDEV_TUNER;
> > + t->sd.entity.name = t->name;
>
> v4l2_subdev_init(), called by v4l2_i2c_subdev_init(), sets sd->entity.name to
> point to sd->name. Is there a reason why the subdev name can't be used as the
> entity name ?
If we don't set entity.name to t->name, the sd->name will be "tuner-core",
instead of the name of the real subdev.
> > +
> > + ret = media_entity_init(&t->sd.entity, 1, &t->pad, 0);
> > + if (ret < 0)
> > + tuner_err("failed to initialize media entity!\n");
> > +#endif
> > return 0;
> > }
> >
> > diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
> > index 707db275f92b..5ffde035789b 100644
> > --- a/include/uapi/linux/media.h
> > +++ b/include/uapi/linux/media.h
> > @@ -66,6 +66,8 @@ struct media_device_info {
> > /* A converter of analogue video to its digital representation. */
> > #define MEDIA_ENT_T_V4L2_SUBDEV_DECODER (MEDIA_ENT_T_V4L2_SUBDEV + 4)
> >
> > +#define MEDIA_ENT_T_V4L2_SUBDEV_TUNER (MEDIA_ENT_T_V4L2_SUBDEV + 5)
> > +
> > #define MEDIA_ENT_FL_DEFAULT (1 << 0)
> >
> > struct media_entity_desc {
>
^ permalink raw reply
* Re: [PATCHv2 1/9] media: Fix DVB representation at media controller API
From: Mauro Carvalho Chehab @ 2015-01-11 14:34 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Linux Media Mailing List, Mauro Carvalho Chehab,
linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <10692325.J7AeJnuN2d@avalon>
Em Sun, 11 Jan 2015 16:05:32 +0200
Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org> escreveu:
> Hi Mauro,
>
> On Sunday 11 January 2015 11:58:24 Mauro Carvalho Chehab wrote:
> > Em Sun, 11 Jan 2015 15:50:04 +0200 Laurent Pinchart escreveu:
> > > On Saturday 03 January 2015 12:49:03 Mauro Carvalho Chehab wrote:
> > >> The DVB devices are identified via a (major, minor) tuple,
> > >> and not by a random id. Fix it, before we start using it.
> > >>
> > >> Signed-off-by: Mauro Carvalho Chehab <mchehab-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
> > >>
> > >> diff --git a/include/media/media-entity.h b/include/media/media-entity.h
> > >> index e00459185d20..de333cc8261b 100644
> > >> --- a/include/media/media-entity.h
> > >> +++ b/include/media/media-entity.h
> > >> @@ -97,7 +97,10 @@ struct media_entity {
> > >> u32 device;
> > >> u32 subdevice;
> > >> } alsa;
> > >> - int dvb;
> > >> + struct {
> > >> + u32 major;
> > >> + u32 minor;
> > >> + } dvb;
> > >>
> > >> /* Sub-device specifications */
> > >> /* Nothing needed yet */
> > >> diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
> > >> index d847c760e8f0..7902e800f019 100644
> > >> --- a/include/uapi/linux/media.h
> > >> +++ b/include/uapi/linux/media.h
> > >> @@ -27,7 +27,7 @@
> > >> #include <linux/types.h>
> > >> #include <linux/version.h>
> > >>
> > >> -#define MEDIA_API_VERSION KERNEL_VERSION(0, 1, 0)
> > >> +#define MEDIA_API_VERSION KERNEL_VERSION(0, 1, 1)
> > >>
> > >> struct media_device_info {
> > >> char driver[16];
> > >> @@ -88,7 +88,10 @@ struct media_entity_desc {
> > >> __u32 device;
> > >> __u32 subdevice;
> > >> } alsa;
> > >> - int dvb;
> > >> + struct {
> > >> + __u32 major;
> > >> + __u32 minor;
> > >> + } dvb;
> > >
> > > Won't this break compilation of existing userspace code ? As DVB is not
> > > properly supported in MC at the moment we could consider that only
> > > mediactl will be affected, so it shouldn't be a big issue.
> >
> > Well, media-ctl uses a local copy of the videodev2.h header, so it won't
> > break.
>
> It's media.h, but you're correct here.
Ah, yes, that's what I meant ;)
Btw, I have also the patches adding support for DVB at v4l-utils:
http://git.linuxtv.org/cgit.cgi/mchehab/experimental-v4l-utils.git/log/?h=dvb-media-ctl
>
> > I'm not aware of any other application using MC for DVB.
> >
> > Yet, imagining that such application exists, then, IMHO, it is better
> > to break compilation for it, as probably such application was written for
> > some OOT driver that might be using its own version of the media
> > controller implementation.
>
> OK. I'll remember that argument the next time I want to break a kernel API
> though ;-)
:)
Actually, we're not breaking the Kernel API here, as DVB support
inside the media controller were never added.
Next time, we should be sure to not add provision for an API at
the Kernel without actually implementing it ;)
Btw, eventually we'll end facing the very same issue when we
merge support for ALSA. IMHO, it is just easier to use major,minor
for all devnodes than to use anything else.
Yet, you're right: maybe we should do, instead:
union {
struct {
u32 major;
u32 minor;
} dev;
/* DEPRECATED: old node specifications */
struct {
u32 major;
u32 minor;
} v4l;
struct {
u32 major;
u32 minor;
} fb;
struct {
u32 card;
u32 device;
u32 subdevice;
} alsa;
int dvb;
/* Sub-device specifications */
/* Nothing needed yet */
} info;
And change media-ctl to use info.dev for all devnodes. This will
provide a fix when we add support for alsa devnodes too.
Regards,
Mauro
^ permalink raw reply
* Re: [PATCH v9 1/2] crypto: AF_ALG: add AEAD support
From: Stephan Mueller @ 2015-01-11 16:16 UTC (permalink / raw)
To: 'Herbert Xu'
Cc: Daniel Borkmann, 'Quentin Gouchet', 'LKML',
linux-crypto-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1518783.PStT3Q01B6-PJstQz4BMNNP20K/wil9xYQuADTiUCJX@public.gmane.org>
Am Sonntag, 11. Januar 2015, 04:45:53 schrieb Stephan Mueller:
Hi Herbert,
> +static int aead_accept_parent(void *private, struct sock *sk)
> +{
> + struct aead_ctx *ctx;
> + struct alg_sock *ask = alg_sk(sk);
> + unsigned int len = sizeof(*ctx) + crypto_aead_reqsize(private);
> + unsigned int ivlen = crypto_aead_ivsize(private);
> +
> + ctx = sock_kmalloc(sk, len, GFP_KERNEL);
> + if (!ctx)
> + return -ENOMEM;
> + memset(ctx, 0, len);
> +
> + ctx->iv = sock_kmalloc(sk, ivlen, GFP_KERNEL);
> + if (!ctx->iv) {
> + sock_kfree_s(sk, ctx, len);
> + return -ENOMEM;
> + }
> + memset(ctx->iv, 0, ivlen);
> +
> + ctx->len = len;
> + ctx->used = 0;
> + ctx->more = 0;
> + ctx->merge = 0;
> + ctx->enc = 0;
> + ctx->tsgl.cur = 0;
ctx->trunc = 0;
is missing here.
I would wait with a new patch once you had the chance to review the updates
and provide comments.
Thanks
--
Ciao
Stephan
^ permalink raw reply
* Re: [PATCH net-next v2 0/8] net: extend ethtool link mode bitmaps to 48 bits
From: David Decotigny @ 2015-01-11 22:49 UTC (permalink / raw)
To: Amir Vadai
Cc: Florian Fainelli, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-api@vger.kernel.org,
Saeed Mahameed, David S. Miller, Jason Wang, Michael S. Tsirkin,
Herbert Xu, Al Viro, Ben Hutchings, Masatake YAMATO, Xi Wang,
Neil Horman, WANG Cong, Flavio Leitner, Tom Gundersen, Jiri Pirko,
Vlad Yasevich, Eric W. Biederman, Venkata Duvvuru,
Govindarajulu Varadarajan
In-Reply-To: <54AE4282.20009@mellanox.com>
Thanks for the input. Please ignore this patch series: I'm preparing a
new version: new commands, bitmap-based that should allow us to live
happily ever after, should take your feedback into account. Will send
an RFC patch series in the next hours/days.
On Thu, Jan 8, 2015 at 12:40 AM, Amir Vadai <amirv@mellanox.com> wrote:
> On 1/6/2015 7:36 PM, David Decotigny wrote:
>> Interesting. It seems that the band-aid I was proposing is already
>> obsolete. We could still use the remaining reserved 16 bits to encode
>> 5 more bits per mask (that is: 53 bits / mask total). But if I
>> understand you, it would allow us to survive only a few months longer,
>> as opposed to a few weeks.
>>
>> One short-term alternative solution I can imagine is the following:
>> /* For example bitmap-based for variable length: */
>> struct ethtool_link_mode {
>> __u32 cmd;
>> __u8 autoneg :1;
>> __u8 duplex :2;
>> __u16 supported_nbits;
>> __u16 advertising_nbits;
>> __u16 lp_advertising_nbits;
>> __u32 reserved[4];
>> __u8 masks[0];
>> };
>> /* Or simpler, statically limited to 64b / mask, but easier to migrate
>> to for driver authors: */
> I think the first options is better. A driver will have to do changes in
> order to support >32 link modes, so better change it once now, without
> having to change it again for >64 link modes.
>
>> struct ethtool_link_mode {
>> __u32 cmd;
>> __u8 autoneg :1;
>> __u8 duplex :2;
>> __u64 supported;
>> __u64 advertising;
>> __u64 lp_advertising;
>> __u32 reserved[4];
>> };
>> #define ETHTOOL_GLINK_MODE 0x0000004a
>> #define ETHTOOL_SLINK_MODE 0x0000004b
>> struct ethtool_ops {
>> ...
>> int (*get_link_mode)(struct net_device *, struct ethtool_link_mode *);
>> int (*set_link_mode)(struct net_device *, struct ethtool_link_mode *);
>> };
>>
>> The same thing required for EEE.
> Yeh :(
>
>>
>> I am not sure about moving the autoneg and duplex fields into the new
>> struct. Especially the "duplex" field.
> I think so too. ethtool user space will call ETHTOOL_[GS]SET and after
> that ETHTOOL_[GS]LINK_MODE (if supported). No need to get the
> duplex/autoneg fields again.
>
>>
>> Then the idea would be to update the ethtool user-space tool to try
>> get/set_link mode when reporting/changing the autoneg/advertising
>> settings.
>>
>> Both will require significant effort from the driver authors.
>> Especially if the variable-length bitmap approach is preferred:
>> - most drivers currently use simple bitwise arithmetic in their code,
>> and that goes far beyond get/set_settings, it is sometimes part of the
>> core driver logic. They will have to migrate to the bitmap API if they
>> want to use the larger bitmaps (note: no change needed if they are
>> happy with <= 32b / mask)
> As I said above, it will save as doing this work again in the future,
> and more problematic, save another version to backport in the future. In
> addition, not all drivers will have to do it, only if >32 link speeds is
> needed - this work will be required.
>
>> - we would have to progressively deprecate the use of #define
>> ADVERTISED_1000baseT_Full in favor of an enum of the bit indices.
> Maybe we could use some macro juggling to define the legacy macro's
> using enum for the first 32 bits, and fail the compilation if used on
>>32. For example, calling this:
> DEFINE_LINK_MODE(ADVERTISED_1000baseT_Full, 5)
>
> Will add the following:
> ADVERTISED_1000baseT_Full_SHIFT = 5
> ADVERTISED_1000baseT_Full = (1<<5)
>
> DEFINE_LINK_MODE(ADVERTISED_100000baseKR5_Full, 50) will add:
> ADVERTISED_100000baseKR5_Full_SHIFT = 50
> ADVERTISED_100000baseKR5_Full = #error new link speeds must be defined
> using [gs]et_link_speed
>
> This will break compilation if ADVERTISED_100000baseKR5_Full is used in
> [gs]et_settings (I know the '#error' will not print something very
> pretty - I used it only to explain what I meant)
>
>>
>> Any feedback welcome. In the meantime, I am going to propose a v3 of
>> current option with 53 bits / mask. I can also propose a prototype of
>> the scheme described above, please let me know.
> I think that it is better to do it once, and skip the 53 bits / mask
> version.
> I'll be happy to assist.
>
> Amir
^ permalink raw reply
* Re: [PATCH 0/2] Input: uinput - fix ioctl numbers in uapi/uinput.h
From: Dmitry Torokhov @ 2015-01-12 0:29 UTC (permalink / raw)
To: Gabriel Laskar
Cc: linux-api-u79uwXL29TY76Z2rM5mHXA, David Herrmann, Peter Hutterer,
Benjamin Tissoires
In-Reply-To: <1420893816-11620-1-git-send-email-gabriel-tU7rkvAWjlwhT4uAktR2oQ@public.gmane.org>
Hi Gabriel,
On Sat, Jan 10, 2015 at 01:43:34PM +0100, Gabriel Laskar wrote:
> Ioctls numbers for UI_GET_SYSNAME and UI_GET_VERSION are incorrectly numbered,
> since nr number is 8bit encoded, 300 and 301 will effectively get 44 and 45.
> these two patches fixes this
>
Nice catch, thank you! I folded the patches together (as they are fixing
essentially the same thing) , changed hex to dec (because the rest of
ioctls in uinput use decimal) and applied.
I wonder if we need to put a BUILD_BUG_ON in one if _IO* defines to
catch such errors early on.
Thanks.
--
Dmitry
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox