* Re: [PATCHv5 man-pages 3/3] execveat.2: initial man page for execveat(2)
From: Eric W. Biederman @ 2014-10-22 17:55 UTC (permalink / raw)
To: David Drysdale
Cc: Andy Lutomirski, Alexander Viro, Meredydd Luff, linux-kernel,
Thomas Gleixner, Ingo Molnar, H. Peter Anvin, Andrew Morton,
Kees Cook, Arnd Bergmann, x86, linux-arch, linux-api
In-Reply-To: <1413978269-17274-4-git-send-email-drysdale@google.com>
David Drysdale <drysdale@google.com> writes:
> Signed-off-by: David Drysdale <drysdale@google.com>
> ---
> man2/execveat.2 | 144 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 144 insertions(+)
> create mode 100644 man2/execveat.2
>
> diff --git a/man2/execveat.2 b/man2/execveat.2
> new file mode 100644
> index 000000000000..d19571a3eb9d
> --- /dev/null
> +++ b/man2/execveat.2
> @@ -0,0 +1,144 @@
> +.\" Copyright (c) 2014 Google, Inc.
> +.\"
> +.\" %%%LICENSE_START(VERBATIM)
> +.\" Permission is granted to make and distribute verbatim copies of this
> +.\" manual provided the copyright notice and this permission notice are
> +.\" preserved on all copies.
> +.\"
> +.\" Permission is granted to copy and distribute modified versions of this
> +.\" manual under the conditions for verbatim copying, provided that the
> +.\" entire resulting derived work is distributed under the terms of a
> +.\" permission notice identical to this one.
> +.\"
> +.\" Since the Linux kernel and libraries are constantly changing, this
> +.\" manual page may be incorrect or out-of-date. The author(s) assume no
> +.\" responsibility for errors or omissions, or for damages resulting from
> +.\" the use of the information contained herein. The author(s) may not
> +.\" have taken the same level of care in the production of this manual,
> +.\" which is licensed free of charge, as they might when working
> +.\" professionally.
> +.\"
> +.\" Formatted or processed versions of this manual, if unaccompanied by
> +.\" the source, must acknowledge the copyright and authors of this work.
> +.\" %%%LICENSE_END
> +.\"
> +.TH EXECVEAT 2 2014-04-02 "Linux" "Linux Programmer's Manual"
> +.SH NAME
> +execveat \- execute program relative to a directory file descriptor
> +.SH SYNOPSIS
> +.B #include <unistd.h>
> +.sp
> +.BI "int execve(int " fd ", const char *" pathname ","
^^^^^^ That needs to be execveat
> +.br
> +.BI " char *const " argv "[], char *const " envp "[],"
> +.br
> +.BI " int " flags);
> +.SH DESCRIPTION
> +The
> +.BR execveat ()
> +system call executes the program pointed to by the combination of \fIfd\fP and \fIpathname\fP.
> +The
> +.BR execveat ()
> +system call operates in exactly the same way as
> +.BR execve (2),
> +except for the differences described in this manual page.
> +
> +If the pathname given in
> +.I pathname
> +is relative, then it is interpreted relative to the directory
> +referred to by the file descriptor
> +.I fd
> +(rather than relative to the current working directory of
> +the calling process, as is done by
> +.BR execve (2)
> +for a relative pathname).
> +
> +If
> +.I pathname
> +is relative and
> +.I fd
> +is the special value
> +.BR AT_FDCWD ,
> +then
> +.I pathname
> +is interpreted relative to the current working
> +directory of the calling process (like
> +.BR execve (2)).
> +
> +If
> +.I pathname
> +is absolute, then
> +.I fd
> +is ignored.
> +
> +If
> +.I pathname
> +is an empty string and the
> +.BR AT_EMPTY_PATH
> +flag is specified, then the file descriptor
> +.I fd
> +specifies the file to be executed.
> +
> +.I flags
> +can either be 0, or include the following flags:
> +.TP
> +.BR AT_EMPTY_PATH
> +If
> +.I pathname
> +is an empty string, operate on the file referred to by
> +.IR fd
> +(which may have been obtained using the
> +.BR open (2)
> +.B O_PATH
> +flag).
> +.TP
> +.B AT_SYMLINK_NOFOLLOW
> +If the file identified by
> +.I fd
> +and a non-NULL
> +.I pathname
> +is a symbolic link, then the call fails with the error
> +.BR EINVAL .
> +.SH "RETURN VALUE"
> +On success,
> +.BR execveat ()
> +does not return. On error \-1 is returned, and
> +.I errno
> +is set appropriately.
> +.SH ERRORS
> +The same errors that occur for
> +.BR execve (2)
> +can also occur for
> +.BR execveat ().
> +The following additional errors can occur for
> +.BR execveat ():
> +.TP
> +.B EBADF
> +.I fd
> +is not a valid file descriptor.
> +.TP
> +.B EINVAL
> +Invalid flag specified in
> +.IR flags .
> +.TP
> +.B ENOTDIR
> +.I pathname
> +is relative and
> +.I fd
> +is a file descriptor referring to a file other than a directory.
> +.SH VERSIONS
> +.BR execveat ()
> +was added to Linux in kernel 3.???.
> +.SH NOTES
> +In addition to the reasons explained in
> +.BR openat (2),
> +the
> +.BR execveat ()
> +system call is also needed to allow
> +.BR fexecve (3)
> +to be implemented on systems that do not have the
> +.I /proc
> +filesystem mounted.
> +.SH SEE ALSO
> +.BR execve (2),
> +.BR fexecve (3)
^ permalink raw reply
* Re: [PATCH RFC v2 01/16] virtio: memory access APIs
From: Christopher Covington @ 2014-10-22 17:45 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Bjarke Istrup Pedersen, Anup Patel, Greg Kroah-Hartman,
linux-kernel, David Drysdale, Lad, Prabhakar, Geert Uytterhoeven,
linux-api, Alexei Starovoitov, virtualization, David S. Miller,
Piotr Król, Mauro Carvalho Chehab
In-Reply-To: <1413992894-22976-2-git-send-email-mst@redhat.com>
Hi Michael,
On 10/22/2014 11:50 AM, Michael S. Tsirkin wrote:
> virtio 1.0 makes all memory structures LE, so
> we need APIs to conditionally do a byteswap on BE
> architectures.
>
> To make it easier to check code statically,
> add virtio specific types for multi-byte integers
> in memory.
>
> Add low level wrappers that do a byteswap conditionally, these will be
> useful e.g. for vhost. Add high level wrappers that will (in the
> future) query device endian-ness and act accordingly.
>
> At the moment, stub them out and assume native endian-ness everywhere.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> include/linux/virtio_config.h | 16 +++++++++++++
> include/uapi/linux/virtio_ring.h | 49 ++++++++++++++++++++--------------------
> include/uapi/linux/Kbuild | 1 +
> 3 files changed, 42 insertions(+), 24 deletions(-)
>
> diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
> index 7f4ef66..d38d3c2 100644
> --- a/include/linux/virtio_config.h
> +++ b/include/linux/virtio_config.h
> @@ -4,6 +4,7 @@
> #include <linux/err.h>
> #include <linux/bug.h>
> #include <linux/virtio.h>
> +#include <linux/virtio_byteorder.h>
What patch creates this file?
> #include <uapi/linux/virtio_config.h>
>
> /**
> @@ -152,6 +153,21 @@ int virtqueue_set_affinity(struct virtqueue *vq, int cpu)
> return 0;
> }
>
> +/* Memory accessors */
> +#define DEFINE_VIRTIO_XX_TO_CPU(bits) \
> +static inline u##bits virtio##bits##_to_cpu(struct virtio_device *vdev, __virtio##bits val) \
> +{ \
> + return __virtio##bits##_to_cpu(false, val); \
> +} \
> +static inline __virtio##bits cpu_to_virtio##bits(struct virtio_device *vdev, u##bits val) \
> +{ \
> + return __cpu_to_virtio##bits(false, val); \
> +}
> +
> +DEFINE_VIRTIO_XX_TO_CPU(16)
> +DEFINE_VIRTIO_XX_TO_CPU(32)
> +DEFINE_VIRTIO_XX_TO_CPU(64)
> +
> /* Config space accessors. */
> #define virtio_cread(vdev, structname, member, ptr) \
> do { \
> diff --git a/include/uapi/linux/virtio_ring.h b/include/uapi/linux/virtio_ring.h
> index a99f9b7..6c00632 100644
> --- a/include/uapi/linux/virtio_ring.h
> +++ b/include/uapi/linux/virtio_ring.h
> @@ -32,6 +32,7 @@
> *
> * Copyright Rusty Russell IBM Corporation 2007. */
> #include <linux/types.h>
> +#include <linux/virtio_types.h>
>
> /* This marks a buffer as continuing via the next field. */
> #define VRING_DESC_F_NEXT 1
> @@ -61,32 +62,32 @@
> /* Virtio ring descriptors: 16 bytes. These can chain together via "next". */
> struct vring_desc {
> /* Address (guest-physical). */
> - __u64 addr;
> + __virtio64 addr;
> /* Length. */
> - __u32 len;
> + __virtio32 len;
> /* The flags as indicated above. */
> - __u16 flags;
> + __virtio16 flags;
> /* We chain unused descriptors via this, too */
> - __u16 next;
> + __virtio16 next;
> };
How does __virtio64 differ from __le64?
Thanks,
Chris
--
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* Re: [PATCHv4 RESEND 0/3] syscalls,x86: Add execveat() system call
From: Eric W. Biederman @ 2014-10-22 17:40 UTC (permalink / raw)
To: David Drysdale
Cc: Andy Lutomirski, Alexander Viro, Meredydd Luff,
linux-kernel@vger.kernel.org, Thomas Gleixner, Ingo Molnar,
H. Peter Anvin, Andrew Morton, Kees Cook, Arnd Bergmann, X86 ML,
linux-arch, Linux API
In-Reply-To: <CAHse=S_dpm8Do-H01Bo49a9TdYw+pSDzYomLOrBsYEcdO=Pytw@mail.gmail.com>
David Drysdale <drysdale@google.com> writes:
> On Tue, Oct 21, 2014 at 5:29 AM, Eric W. Biederman
> <ebiederm@xmission.com> wrote:
>> Andy Lutomirski <luto@amacapital.net> writes:
>>
>>> On Mon, Oct 20, 2014 at 6:48 AM, David Drysdale <drysdale@google.com> wrote:
>>>> On Sun, Oct 19, 2014 at 1:20 AM, Eric W. Biederman
>>>> <ebiederm@xmission.com> wrote:
>>>>> Andy Lutomirski <luto@amacapital.net> writes:
>>>>>
>>>>>> [Added Eric Biederman, since I think your tree might be a reasonable
>>>>>> route forward for these patches.]
>>>>>>
>>>>>> On Thu, Jun 5, 2014 at 6:40 AM, David Drysdale <drysdale@google.com> wrote:
>>>>>>> Resending, adding cc:linux-api.
>>>>>>>
>>>>>>> Also, it may help to add a little more background -- this patch is
>>>>>>> needed as a (small) part of implementing Capsicum in the Linux kernel.
>>>>>>>
>>>>>>> Capsicum is a security framework that has been present in FreeBSD since
>>>>>>> version 9.0 (Jan 2012), and is based on concepts from object-capability
>>>>>>> security [1].
>>>>>>>
>>>>>>> One of the features of Capsicum is capability mode, which locks down
>>>>>>> access to global namespaces such as the filesystem hierarchy. In
>>>>>>> capability mode, /proc is thus inaccessible and so fexecve(3) doesn't
>>>>>>> work -- hence the need for a kernel-space
>>>>>>
>>>>>> I just found myself wanting this syscall for another reason: injecting
>>>>>> programs into sandboxes or otherwise heavily locked-down namespaces.
>>>>>>
>>>>>> For example, I want to be able to reliably do something like nsenter
>>>>>> --namespace-flags-here toybox sh. Toybox's shell is unusual in that
>>>>>> it is more or less fully functional, so this should Just Work (tm),
>>>>>> except that the toybox binary might not exist in the namespace being
>>>>>> entered. If execveat were available, I could rig nsenter or a similar
>>>>>> tool to open it with O_CLOEXEC, enter the namespace, and then call
>>>>>> execveat.
>>>>>>
>>>>>> Is there any reason that these patches can't be merged more or less as
>>>>>> is for 3.19?
>>>>>
>>>>> Yes. There is a silliness in how it implements fexecve. The fexecve
>>>>> case should be use the empty string "" not a NULL pointer to indication
>>>>> that. That change will then harmonize execveat with the other ...at
>>>>> system calls and simplify the code and remove a special case. I believe
>>>>> using the empty string "" requires implementing the AT_EMPTY_PATH flag.
>>>>
>>>> Good point -- I'll shift to "" + AT_EMPTY_PATH.
>>>
>>> Pending a better idea, I would also see if the patches can be changed
>>> to return an error if d_path ends up with an "(unreachable)" thing
>>> rather than failing inexplicably later on.
>>
>> For my reference we are talking about
>>
>>> @@ -1489,7 +1524,21 @@ static int do_execve_common(struct filename *filename,
>>> sched_exec();
>>>
>>> bprm->file = file;
>>> - bprm->filename = bprm->interp = filename->name;
>>> + if (filename && fd == AT_FDCWD) {
>>> + bprm->filename = filename->name;
>>> + } else {
>>> + pathbuf = kmalloc(PATH_MAX, GFP_TEMPORARY);
>>> + if (!pathbuf) {
>>> + retval = -ENOMEM;
>>> + goto out_unmark;
>>> + }
>>> + bprm->filename = d_path(&file->f_path, pathbuf, PATH_MAX);
>>> + if (IS_ERR(bprm->filename)) {
>>> + retval = PTR_ERR(bprm->filename);
>>> + goto out_unmark;
>>> + }
>>> + }
>>> + bprm->interp = bprm->filename;
>>>
>>> retval = bprm_mm_init(bprm);
>>> if (retval)
>>
>> The interesting case for fexecve is when we either don't know what files
>> are present or we don't want to depend on which files are present.
>>
>> As Al pointed out d_path really isn't the right solution. It fails when
>> printing /proc/self/fd/${fd}/${filename->name} would work, and the
>> "(deleted)" or "(unreachable)" strings are wrong.
>>
>> The test for today's cases should be:
>> if ((filename->name[0] == '/') || fd == AT_FDCWD) {
>> bprm->filename = filename->name;
>> }
>>
>> To handle the case where the file descriptor is relevant.
> (s/relevant/irrelevant)
>
> Yep, good spot.
>
>> For the case where the file descriptor is relevant let me suggest
>> setting bprm->filename and bprm->interp to:
>>
>> /dev/fd/${fd}/${filename->name}
>
> I'll send out an updated patchset with this approach, but I have a slight
> reservation. Given that /dev/fd is a symlink to /proc/self/fd, this approach
> means that script invocations will always fail on a /proc-less system,
> where the previous iteration might have worked.
>
> (As it happens, this isn't a restriction that affects the things I'm
> working on, as Capsicum wouldn't allow script invocation anyway.
> However, scenarios without /proc were nominally one of the motivating
> factors for execveat in the first place...)
Which is where's Al Viro's and Peter Anvin's conversation about a
minimal filesystem that can serve the needs of /proc/self/fd comes in.
There are uses for execveat with static executables, so I think execveat
is justified. But having a dupfs that we could potentially mount on
/dev/fd would be interesting. As it is much less of a security
concern than /proc with all of the interfaces it provides.
>> It is more a description of what we have done but as a magic string it
>> is descriptive. Documetation/devices.txt documents that /dev/fd/ should
>> exist, making it an unambiguous path. Further these days the kernel
>> sets the device naming policy in dev, so I think we are strongly safe in
>> using that path in any event.
>>
>> I think execveat is interesting in the kernel because the motivating
>> cases are the cases where anything except a static executable is
>> uninteresting.
>
> FYI, there is potential in the future for something other than static
> executables -- the FreeBSD Capsicum implementation includes changes
> to the dynamic linker to get its search path as a list of pre-opened dfds
> (in LD_LIBRARY_PATH_FDS) rather than paths.
Which still leaves open the question how do you find the dynamic
linker. Is that also a pre-opened dfd?
Using /dev/fd/$N is also the kind of thing that a shell or a script
interpret could special case instead relying on a filesystem node
to exist.
Eric
^ permalink raw reply
* Re: [PATCH v1 3/3] tpm: fix multiple race conditions in tpm_ppi.c
From: Jason Gunthorpe @ 2014-10-22 17:26 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: Peter Huewe, Ashley Lai, Marcel Selhorst,
tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA,
josh.triplett-ral2JQCrhuEAvxtiuMwx3w,
christophe.ricard-Re5JQEeQqe8AvxtiuMwx3w,
jason.gunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/
In-Reply-To: <1413995036-22497-4-git-send-email-jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
On Wed, Oct 22, 2014 at 07:23:56PM +0300, Jarkko Sakkinen wrote:
> Traversal of the ACPI device tree was not done right. It should lookup
> PPI only under the ACPI device that it is associated. Otherwise, it could
> match to a wrong PPI interface if there are two TPM devices in the device
> tree.
>
> Removed global ACPI handle and version string from tpm_ppi.c as this
> is racy. Instead they should be associated with the chip.
>
> Moved code just a tiny bit towards two-phase allocation to implement
> fix for the PPI race conditions.
Not this version..
> Added missing copyright platter to tpm_ppi.c.
>
> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Reviewed-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
I like this one the most of the three I've seen :)
Did you also look in tpm_acpi.c to see if it needs to use
acpi_dev_handle somehow too?
> + union acpi_object *obj;
> + struct kobject *parent = &chip->dev->kobj;
Nit, this variable is only used once, it would be clearer to inline
> + /* Cache PPI version string. */
> + obj = acpi_evaluate_dsm_typed(chip->acpi_dev_handle, tpm_ppi_uuid,
> + TPM_PPI_REVISION_ID, TPM_PPI_FN_VERSION,
> + NULL, ACPI_TYPE_STRING);
> + if (obj) {
> + strlcpy(chip->ppi_version, obj->string.pointer,
> + PPI_VERSION_LEN + 1);
> + ACPI_FREE(obj);
> + } else
> + return -ENOMEM;
> +
> + return chip->acpi_dev_handle ?
> + sysfs_create_group(parent, &ppi_attr_grp) : 0;
The above sequence can just be:
if (!obj)
return -ENOMEM;
strlcpy(chip->ppi_version, obj->string.pointer, sizeof(chip->ppi_version));
ACPI_FREE(obj);
return sysfs_create_group(&chip->dev->kobj, &ppi_attr_grp);
Which is more idiomatic. Also remove TPM_PPI_VERSION_LEN, sizeof is better.
I know nothing about acpi, but is ENOMEM the right code? I would think
acpi_evalute_dsm_typed would also fail if tpm_ppi_uuid is not found??
> + return chip->acpi_dev_handle ?
> + sysfs_create_group(parent, &ppi_attr_grp) : 0;
dev_handle is already checked to be non 0
> +void tpm_remove_ppi(struct tpm_chip *chip)
> + struct kobject *parent = &chip->dev->kobj;
Also used only once
Jason
^ permalink raw reply
* Re: [PATCH v1 2/3] tpm: two-phase chip management functions
From: Jason Gunthorpe @ 2014-10-22 17:16 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: Peter Huewe, Ashley Lai, Marcel Selhorst,
tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA,
josh.triplett-ral2JQCrhuEAvxtiuMwx3w,
christophe.ricard-Re5JQEeQqe8AvxtiuMwx3w,
jason.gunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/
In-Reply-To: <1413995036-22497-3-git-send-email-jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
On Wed, Oct 22, 2014 at 07:23:55PM +0300, Jarkko Sakkinen wrote:
> tpm_register_hardware() and tpm_remove_hardware() are called often
> before initializing the device. This is wrong order since it could
> be that main TPM driver needs a fully initialized chip to be able to
> do its job. For example, now it is impossible to move common startup
> functions such as tpm_do_selftest() to tpm_register_hardware().
> Added tpm_chip_alloc() and tpm_chip_register() where tpm_chip_alloc()
It is called tpmm_chip_alloc() in this version..
> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Reviewed-By: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
Looks fine, if you have to spin this again you can incorporate the
nits.
> +
> +static DECLARE_BITMAP(dev_mask, TPM_NUM_DEVICES);
Not for this patch, but while this area of code is being looked at
this should probably be an IDR/IDA like other subsytems?
> + if (try_module_get(pos->dev->driver->owner)) {
> + chip = pos;
> + break;
> + }
Not for this patch, this module stuff should be wiped in favor of chip->ops
locking.
> +static void tpmm_chip_remove(void *data)
> +{
> + struct tpm_chip *chip = (struct tpm_chip *) data;
> + dev_dbg(chip->dev, "%s\n", __func__);
This print is silent in the default compile, right?
> + chip->dev_num = find_first_zero_bit(dev_mask, TPM_NUM_DEVICES);
[..]
> + set_bit(chip->dev_num, dev_mask);
Not for this patch but somehow there is no locking for dev_mask
here. I guess it should use the driver_lock spinlock?
> + chip->bios_dir = tpm_bios_log_setup(chip->devname);
Not for this patch, but tpm_bios_log_setup can return NULL if
securityfs setup fails.
> diff --git a/drivers/char/tpm/tpm_atmel.c b/drivers/char/tpm/tpm_atmel.c
> index 6069d13..8e2576a 100644
> +++ b/drivers/char/tpm/tpm_atmel.c
> @@ -138,11 +138,11 @@ static void atml_plat_remove(void)
> struct tpm_chip *chip = dev_get_drvdata(&pdev->dev);
>
> if (chip) {
> + tpm_chip_unregister(chip);
> if (chip->vendor.have_region)
> atmel_release_region(chip->vendor.base,
> chip->vendor.region_size);
> atmel_put_base_addr(chip->vendor.iobase);
> - tpm_remove_hardware(chip->dev);
> platform_device_unregister(pdev);
Missed this before, the Atmel driver is the same as the TIS driver in
force mode, ie it isn't going through the driver APIs, but instead
force creating platform devices. Same comment as for TIS - I'm not
sure devm works properly like this.
I guess just add the same comment as for TIS?
> diff --git a/drivers/char/tpm/tpm_i2c_infineon.c b/drivers/char/tpm/tpm_i2c_infineon.c
> index 472af4b..6f00bc3 100644
> +++ b/drivers/char/tpm/tpm_i2c_infineon.c
> @@ -581,10 +581,9 @@ static int tpm_tis_i2c_init(struct device *dev)
> int rc = 0;
> struct tpm_chip *chip;
>
> - chip = tpm_register_hardware(dev, &tpm_tis_i2c);
> - if (!chip) {
> - dev_err(dev, "could not register hardware\n");
> - rc = -ENODEV;
> + chip = tpmm_chip_alloc(dev, &tpm_tis_i2c);
> + if (IS_ERR(chip)) {
> + rc = PTR_ERR(chip);
> goto out_err;
Nit: out_err is synonymous with 'return rc;', so all the goto out_err
in this driver can just return;
> + rc = tpm_chip_register(chip);
> + if (rc)
> + return rc;
> + return 0;
Nit: could just be return tpm_chip_register(chip);
> @@ -619,10 +619,9 @@ tpm_st33_i2c_probe(struct i2c_client *client, const struct i2c_device_id *id)
> goto end;
> }
>
> - chip = tpm_register_hardware(&client->dev, &st_i2c_tpm);
> - if (!chip) {
> - dev_info(&client->dev, "fail chip\n");
> - err = -ENODEV;
> + chip = tpmm_chip_alloc(&client->dev, &st_i2c_tpm);
> + if (IS_ERR(chip)) {
> + err = PTR_ERR(chip);
> goto end;
> }
Nit: Same comment, 'goto end' can just be 'return rc'
> - dev_info(chip->dev, "TPM I2C Initialized\n");
> + err = tpm_chip_register(chip);
> + if (err)
> + goto _irq_set;
> +
Nit: Same comment
> end:
> pr_info("TPM I2C initialisation fail\n");
This pr_info should be deleted
> @@ -573,6 +580,8 @@ static void tpm_inf_pnp_remove(struct pnp_dev *dev)
> struct tpm_chip *chip = pnp_get_drvdata(dev);
>
> if (chip) {
Nit: This if in the remove callback is an anti-pattern and should be
globally removed. If remove is being called then probe succeeded,
there is no way for probe to succed and drvdata to be unset.
> static void tpm_nsc_remove(struct device *dev)
> {
> struct tpm_chip *chip = dev_get_drvdata(dev);
> - if ( chip ) {
> + if (chip) {
Same comment
> @@ -836,7 +831,7 @@ MODULE_PARM_DESC(hid, "Set additional specific HID for this driver to probe");
>
> static struct platform_driver tis_drv = {
> .driver = {
> - .name = "tpm_tis",
> + .name = "tpm_tis",
> .owner = THIS_MODULE,
> .pm = &tpm_tis_pm,
> },
There is no remove method here in this platform_driver, this ties into
the question if force works or not. The tpm_tis_remove call in the
cleanup_hardware should be done through the .remove method of this
driver structure..
Jason
^ permalink raw reply
* Re: [PATCH v1 1/3] tpm: merge duplicate transmit_cmd() functions
From: Jason Gunthorpe @ 2014-10-22 16:37 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: Peter Huewe, Ashley Lai, Marcel Selhorst,
tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA,
josh.triplett-ral2JQCrhuEAvxtiuMwx3w,
christophe.ricard-Re5JQEeQqe8AvxtiuMwx3w,
jason.gunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/
In-Reply-To: <1413995036-22497-2-git-send-email-jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
On Wed, Oct 22, 2014 at 07:23:54PM +0300, Jarkko Sakkinen wrote:
> Merged transmit_cmd() functions in tpm-interface.c and tpm-sysfs.c.
> Added "tpm_" prefix for consistency sake. Changed cmd parameter as
> opaque. This enables to use separate command structures for TPM1
> and TPM2 commands in future. Loose coupling works fine here.
>
> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Reviewed-By: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
Thanks Jarkko!
Jason
^ permalink raw reply
* Aw: [PATCH v1 0/3] tpm: prepare for TPM2
From: Peter Huewe @ 2014-10-22 16:34 UTC (permalink / raw)
Cc: Ashley Lai, Marcel Selhorst,
tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA,
josh.triplett-ral2JQCrhuEAvxtiuMwx3w,
christophe.ricard-Re5JQEeQqe8AvxtiuMwx3w,
jason.gunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/, Jarkko Sakkinen
In-Reply-To: <1413995036-22497-1-git-send-email-jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Hi Jarkko,
> Jarkko Sakkinen (3):
> tpm: merge duplicate transmit_cmd() functions
> tpm: two-phase chip management functions
> tpm: fix multiple race conditions in tpm_ppi.c
the patchset introduces a sparse error:
CHECK /home/phuewe/linux-2.6-host/drivers/char/tpm/tpm_tis.c
/home/phuewe/linux-2.6-host/drivers/char/tpm/tpm_tis.c:783:39: warning: Using plain integer as NULL pointer
/home/phuewe/linux-2.6-host/drivers/char/tpm/tpm_tis.c:868:39: warning: Using plain integer as NULL pointer
apart from that I'll review it this evening.
Thanks,
Peter
^ permalink raw reply
* [PATCH v1 3/3] tpm: fix multiple race conditions in tpm_ppi.c
From: Jarkko Sakkinen @ 2014-10-22 16:23 UTC (permalink / raw)
To: Peter Huewe, Ashley Lai, Marcel Selhorst
Cc: tpmdd-devel, linux-kernel, linux-api, josh.triplett,
christophe.ricard, jason.gunthorpe, Jarkko Sakkinen
In-Reply-To: <1413995036-22497-1-git-send-email-jarkko.sakkinen@linux.intel.com>
Traversal of the ACPI device tree was not done right. It should lookup
PPI only under the ACPI device that it is associated. Otherwise, it could
match to a wrong PPI interface if there are two TPM devices in the device
tree.
Removed global ACPI handle and version string from tpm_ppi.c as this
is racy. Instead they should be associated with the chip.
Moved code just a tiny bit towards two-phase allocation to implement
fix for the PPI race conditions.
Added missing copyright platter to tpm_ppi.c.
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
drivers/char/tpm/tpm-chip.c | 4 +-
drivers/char/tpm/tpm.h | 16 ++++--
drivers/char/tpm/tpm_ppi.c | 137 +++++++++++++++++++++++++++-----------------
drivers/char/tpm/tpm_tis.c | 15 +++--
4 files changed, 110 insertions(+), 62 deletions(-)
diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index 4a29421..2446422 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -143,7 +143,7 @@ int tpm_chip_register(struct tpm_chip *chip)
if (rc)
goto del_misc;
- rc = tpm_add_ppi(&chip->dev->kobj);
+ rc = tpm_add_ppi(chip);
if (rc)
goto del_sysfs;
@@ -183,7 +183,7 @@ void tpm_chip_unregister(struct tpm_chip *chip)
synchronize_rcu();
tpm_sysfs_del_device(chip);
- tpm_remove_ppi(&chip->dev->kobj);
+ tpm_remove_ppi(chip);
tpm_bios_log_teardown(chip->bios_dir);
tpm_dev_del_device(chip);
}
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index fa0974c..16fab4f 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -27,6 +27,7 @@
#include <linux/platform_device.h>
#include <linux/io.h>
#include <linux/tpm.h>
+#include <linux/acpi.h>
enum tpm_const {
TPM_MINOR = 224, /* officially assigned */
@@ -94,6 +95,8 @@ struct tpm_vendor_specific {
#define TPM_VID_WINBOND 0x1050
#define TPM_VID_STM 0x104A
+#define TPM_PPI_VERSION_LEN 3
+
struct tpm_chip {
struct device *dev; /* Device stuff */
const struct tpm_class_ops *ops;
@@ -109,6 +112,11 @@ struct tpm_chip {
struct dentry **bios_dir;
+#ifdef CONFIG_ACPI
+ acpi_handle acpi_dev_handle;
+ char ppi_version[TPM_PPI_VERSION_LEN + 1];
+#endif /* CONFIG_ACPI */
+
struct list_head list;
};
@@ -338,15 +346,15 @@ void tpm_sysfs_del_device(struct tpm_chip *chip);
int tpm_pcr_read_dev(struct tpm_chip *chip, int pcr_idx, u8 *res_buf);
#ifdef CONFIG_ACPI
-extern int tpm_add_ppi(struct kobject *);
-extern void tpm_remove_ppi(struct kobject *);
+extern int tpm_add_ppi(struct tpm_chip *chip);
+extern void tpm_remove_ppi(struct tpm_chip *chip);
#else
-static inline int tpm_add_ppi(struct kobject *parent)
+static inline int tpm_add_ppi(struct tpm_chip *chip)
{
return 0;
}
-static inline void tpm_remove_ppi(struct kobject *parent)
+static inline void tpm_remove_ppi(struct tpm_chip *chip)
{
}
#endif
diff --git a/drivers/char/tpm/tpm_ppi.c b/drivers/char/tpm/tpm_ppi.c
index 61dcc80..f8b2326 100644
--- a/drivers/char/tpm/tpm_ppi.c
+++ b/drivers/char/tpm/tpm_ppi.c
@@ -1,3 +1,22 @@
+/*
+ * Copyright (C) 2012-2014 Intel Corporation
+ *
+ * Authors:
+ * Xiaoyan Zhang <xiaoyan.zhang@intel.com>
+ * Jiang Liu <jiang.liu@linux.intel.com>
+ * Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
+ *
+ * Maintained by: <tpmdd-devel@lists.sourceforge.net>
+ *
+ * This file contains implementation of the sysfs interface for PPI.
+ *
+ * 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; version 2
+ * of the License.
+ */
+
+
#include <linux/acpi.h>
#include "tpm.h"
@@ -22,45 +41,22 @@ static const u8 tpm_ppi_uuid[] = {
0x8D, 0x10, 0x08, 0x9D, 0x16, 0x53
};
-static char tpm_ppi_version[PPI_VERSION_LEN + 1];
-static acpi_handle tpm_ppi_handle;
-
-static acpi_status ppi_callback(acpi_handle handle, u32 level, void *context,
- void **return_value)
-{
- union acpi_object *obj;
-
- if (!acpi_check_dsm(handle, tpm_ppi_uuid, TPM_PPI_REVISION_ID,
- 1 << TPM_PPI_FN_VERSION))
- return AE_OK;
-
- /* Cache version string */
- obj = acpi_evaluate_dsm_typed(handle, tpm_ppi_uuid,
- TPM_PPI_REVISION_ID, TPM_PPI_FN_VERSION,
- NULL, ACPI_TYPE_STRING);
- if (obj) {
- strlcpy(tpm_ppi_version, obj->string.pointer,
- PPI_VERSION_LEN + 1);
- ACPI_FREE(obj);
- }
-
- *return_value = handle;
-
- return AE_CTRL_TERMINATE;
-}
-
static inline union acpi_object *
-tpm_eval_dsm(int func, acpi_object_type type, union acpi_object *argv4)
+tpm_eval_dsm(acpi_handle dev_handle, int func, acpi_object_type type,
+ union acpi_object *argv4)
{
- BUG_ON(!tpm_ppi_handle);
- return acpi_evaluate_dsm_typed(tpm_ppi_handle, tpm_ppi_uuid,
- TPM_PPI_REVISION_ID, func, argv4, type);
+ BUG_ON(!dev_handle);
+ return acpi_evaluate_dsm_typed(dev_handle, tpm_ppi_uuid,
+ TPM_PPI_REVISION_ID,
+ func, argv4, type);
}
static ssize_t tpm_show_ppi_version(struct device *dev,
struct device_attribute *attr, char *buf)
{
- return scnprintf(buf, PAGE_SIZE, "%s\n", tpm_ppi_version);
+ struct tpm_chip *chip = dev_get_drvdata(dev);
+
+ return scnprintf(buf, PAGE_SIZE, "%s\n", chip->ppi_version);
}
static ssize_t tpm_show_ppi_request(struct device *dev,
@@ -68,8 +64,10 @@ static ssize_t tpm_show_ppi_request(struct device *dev,
{
ssize_t size = -EINVAL;
union acpi_object *obj;
+ struct tpm_chip *chip = dev_get_drvdata(dev);
- obj = tpm_eval_dsm(TPM_PPI_FN_GETREQ, ACPI_TYPE_PACKAGE, NULL);
+ obj = tpm_eval_dsm(chip->acpi_dev_handle, TPM_PPI_FN_GETREQ,
+ ACPI_TYPE_PACKAGE, NULL);
if (!obj)
return -ENXIO;
@@ -103,14 +101,15 @@ static ssize_t tpm_store_ppi_request(struct device *dev,
int func = TPM_PPI_FN_SUBREQ;
union acpi_object *obj, tmp;
union acpi_object argv4 = ACPI_INIT_DSM_ARGV4(1, &tmp);
+ struct tpm_chip *chip = dev_get_drvdata(dev);
/*
* the function to submit TPM operation request to pre-os environment
* is updated with function index from SUBREQ to SUBREQ2 since PPI
* version 1.1
*/
- if (acpi_check_dsm(tpm_ppi_handle, tpm_ppi_uuid, TPM_PPI_REVISION_ID,
- 1 << TPM_PPI_FN_SUBREQ2))
+ if (acpi_check_dsm(chip->acpi_dev_handle, tpm_ppi_uuid,
+ TPM_PPI_REVISION_ID, 1 << TPM_PPI_FN_SUBREQ2))
func = TPM_PPI_FN_SUBREQ2;
/*
@@ -119,7 +118,7 @@ static ssize_t tpm_store_ppi_request(struct device *dev,
* string/package type. For PPI version 1.0 and 1.1, use buffer type
* for compatibility, and use package type since 1.2 according to spec.
*/
- if (strcmp(tpm_ppi_version, "1.2") < 0) {
+ if (strcmp(chip->ppi_version, "1.2") < 0) {
if (sscanf(buf, "%d", &req) != 1)
return -EINVAL;
argv4.type = ACPI_TYPE_BUFFER;
@@ -131,7 +130,8 @@ static ssize_t tpm_store_ppi_request(struct device *dev,
return -EINVAL;
}
- obj = tpm_eval_dsm(func, ACPI_TYPE_INTEGER, &argv4);
+ obj = tpm_eval_dsm(chip->acpi_dev_handle, func, ACPI_TYPE_INTEGER,
+ &argv4);
if (!obj) {
return -ENXIO;
} else {
@@ -157,6 +157,7 @@ static ssize_t tpm_show_ppi_transition_action(struct device *dev,
.buffer.length = 0,
.buffer.pointer = NULL
};
+ struct tpm_chip *chip = dev_get_drvdata(dev);
static char *info[] = {
"None",
@@ -171,9 +172,10 @@ static ssize_t tpm_show_ppi_transition_action(struct device *dev,
* (e.g. Capella with PPI 1.0) need integer/string/buffer type, so for
* compatibility, define params[3].type as buffer, if PPI version < 1.2
*/
- if (strcmp(tpm_ppi_version, "1.2") < 0)
+ if (strcmp(chip->ppi_version, "1.2") < 0)
obj = &tmp;
- obj = tpm_eval_dsm(TPM_PPI_FN_GETACT, ACPI_TYPE_INTEGER, obj);
+ obj = tpm_eval_dsm(chip->acpi_dev_handle, TPM_PPI_FN_GETACT,
+ ACPI_TYPE_INTEGER, obj);
if (!obj) {
return -ENXIO;
} else {
@@ -196,8 +198,10 @@ static ssize_t tpm_show_ppi_response(struct device *dev,
acpi_status status = -EINVAL;
union acpi_object *obj, *ret_obj;
u64 req, res;
+ struct tpm_chip *chip = dev_get_drvdata(dev);
- obj = tpm_eval_dsm(TPM_PPI_FN_GETRSP, ACPI_TYPE_PACKAGE, NULL);
+ obj = tpm_eval_dsm(chip->acpi_dev_handle, TPM_PPI_FN_GETRSP,
+ ACPI_TYPE_PACKAGE, NULL);
if (!obj)
return -ENXIO;
@@ -248,7 +252,8 @@ cleanup:
return status;
}
-static ssize_t show_ppi_operations(char *buf, u32 start, u32 end)
+static ssize_t show_ppi_operations(acpi_handle dev_handle, char *buf, u32 start,
+ u32 end)
{
int i;
u32 ret;
@@ -264,14 +269,15 @@ static ssize_t show_ppi_operations(char *buf, u32 start, u32 end)
"User not required",
};
- if (!acpi_check_dsm(tpm_ppi_handle, tpm_ppi_uuid, TPM_PPI_REVISION_ID,
+ if (!acpi_check_dsm(dev_handle, tpm_ppi_uuid, TPM_PPI_REVISION_ID,
1 << TPM_PPI_FN_GETOPR))
return -EPERM;
tmp.integer.type = ACPI_TYPE_INTEGER;
for (i = start; i <= end; i++) {
tmp.integer.value = i;
- obj = tpm_eval_dsm(TPM_PPI_FN_GETOPR, ACPI_TYPE_INTEGER, &argv);
+ obj = tpm_eval_dsm(dev_handle, TPM_PPI_FN_GETOPR,
+ ACPI_TYPE_INTEGER, &argv);
if (!obj) {
return -ENOMEM;
} else {
@@ -291,14 +297,20 @@ static ssize_t tpm_show_ppi_tcg_operations(struct device *dev,
struct device_attribute *attr,
char *buf)
{
- return show_ppi_operations(buf, 0, PPI_TPM_REQ_MAX);
+ struct tpm_chip *chip = dev_get_drvdata(dev);
+
+ return show_ppi_operations(chip->acpi_dev_handle, buf, 0,
+ PPI_TPM_REQ_MAX);
}
static ssize_t tpm_show_ppi_vs_operations(struct device *dev,
struct device_attribute *attr,
char *buf)
{
- return show_ppi_operations(buf, PPI_VS_REQ_START, PPI_VS_REQ_END);
+ struct tpm_chip *chip = dev_get_drvdata(dev);
+
+ return show_ppi_operations(chip->acpi_dev_handle, buf, PPI_VS_REQ_START,
+ PPI_VS_REQ_END);
}
static DEVICE_ATTR(version, S_IRUGO, tpm_show_ppi_version, NULL);
@@ -323,16 +335,37 @@ static struct attribute_group ppi_attr_grp = {
.attrs = ppi_attrs
};
-int tpm_add_ppi(struct kobject *parent)
+int tpm_add_ppi(struct tpm_chip *chip)
{
- /* Cache TPM ACPI handle and version string */
- acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT, ACPI_UINT32_MAX,
- ppi_callback, NULL, NULL, &tpm_ppi_handle);
- return tpm_ppi_handle ? sysfs_create_group(parent, &ppi_attr_grp) : 0;
+ union acpi_object *obj;
+ struct kobject *parent = &chip->dev->kobj;
+
+ if (!chip->acpi_dev_handle)
+ return 0;
+
+ if (!acpi_check_dsm(chip->acpi_dev_handle, tpm_ppi_uuid,
+ TPM_PPI_REVISION_ID, 1 << TPM_PPI_FN_VERSION))
+ return 0;
+
+ /* Cache PPI version string. */
+ obj = acpi_evaluate_dsm_typed(chip->acpi_dev_handle, tpm_ppi_uuid,
+ TPM_PPI_REVISION_ID, TPM_PPI_FN_VERSION,
+ NULL, ACPI_TYPE_STRING);
+ if (obj) {
+ strlcpy(chip->ppi_version, obj->string.pointer,
+ PPI_VERSION_LEN + 1);
+ ACPI_FREE(obj);
+ } else
+ return -ENOMEM;
+
+ return chip->acpi_dev_handle ?
+ sysfs_create_group(parent, &ppi_attr_grp) : 0;
}
-void tpm_remove_ppi(struct kobject *parent)
+void tpm_remove_ppi(struct tpm_chip *chip)
{
- if (tpm_ppi_handle)
+ struct kobject *parent = &chip->dev->kobj;
+
+ if (chip->ppi_version[0] != '\0')
sysfs_remove_group(parent, &ppi_attr_grp);
}
diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c
index e2cb04d..c427da2 100644
--- a/drivers/char/tpm/tpm_tis.c
+++ b/drivers/char/tpm/tpm_tis.c
@@ -536,8 +536,9 @@ static void tpm_tis_remove(struct tpm_chip *chip)
release_locality(chip, chip->vendor.locality, 1);
}
-static int tpm_tis_init(struct device *dev, resource_size_t start,
- resource_size_t len, unsigned int irq)
+static int tpm_tis_init(struct device *dev, acpi_handle acpi_dev_handle,
+ resource_size_t start, resource_size_t len,
+ unsigned int irq)
{
u32 vendor, intfcaps, intmask;
int rc, i, irq_s, irq_e, probe;
@@ -547,6 +548,8 @@ static int tpm_tis_init(struct device *dev, resource_size_t start,
if (IS_ERR(chip))
return PTR_ERR(chip);
+ chip->acpi_dev_handle = acpi_dev_handle;
+
chip->vendor.iobase = devm_ioremap(dev, start, len);
if (!chip->vendor.iobase)
return -EIO;
@@ -777,6 +780,7 @@ static int tpm_tis_pnp_init(struct pnp_dev *pnp_dev,
{
resource_size_t start, len;
unsigned int irq = 0;
+ acpi_handle acpi_dev_handle = 0;
start = pnp_mem_start(pnp_dev, 0);
len = pnp_mem_len(pnp_dev, 0);
@@ -789,7 +793,10 @@ static int tpm_tis_pnp_init(struct pnp_dev *pnp_dev,
if (is_itpm(pnp_dev))
itpm = true;
- return tpm_tis_init(&pnp_dev->dev, start, len, irq);
+ if (pnp_acpi_device(pnp_dev))
+ acpi_dev_handle = pnp_acpi_device(pnp_dev)->handle;
+
+ return tpm_tis_init(&pnp_dev->dev, acpi_dev_handle, start, len, irq);
}
static struct pnp_device_id tpm_pnp_tbl[] = {
@@ -858,7 +865,7 @@ static int __init init_tis(void)
rc = PTR_ERR(pdev);
goto err_dev;
}
- rc = tpm_tis_init(&pdev->dev, TIS_MEM_BASE, TIS_MEM_LEN, 0);
+ rc = tpm_tis_init(&pdev->dev, 0, TIS_MEM_BASE, TIS_MEM_LEN, 0);
if (rc)
goto err_init;
return 0;
--
2.1.0
^ permalink raw reply related
* [PATCH v1 2/3] tpm: two-phase chip management functions
From: Jarkko Sakkinen @ 2014-10-22 16:23 UTC (permalink / raw)
To: Peter Huewe, Ashley Lai, Marcel Selhorst
Cc: tpmdd-devel, linux-kernel, linux-api, josh.triplett,
christophe.ricard, jason.gunthorpe, Jarkko Sakkinen
In-Reply-To: <1413995036-22497-1-git-send-email-jarkko.sakkinen@linux.intel.com>
tpm_register_hardware() and tpm_remove_hardware() are called often
before initializing the device. This is wrong order since it could
be that main TPM driver needs a fully initialized chip to be able to
do its job. For example, now it is impossible to move common startup
functions such as tpm_do_selftest() to tpm_register_hardware().
Added tpm_chip_alloc() and tpm_chip_register() where tpm_chip_alloc()
reserves memory resources and tpm_chip_register() initializes the
device driver. This way it is possible to alter struct tpm_chip
attributes and initialize the device driver before passing it to
tpm_chip_register().
The framework takes care of freeing struct tpm_chip by using devres
API. The broken release callback has been wiped. For example, ACPI
drivers do not ever get this callback.
This is a interm step to get proper life-cycle for TPM device drivers.
The next steps are adding proper ref counting and locking to tpm_chip
that is used in every place in the TPM driver.
Big thank you to Jason Gunthorpe for carefully reviewing this part
of the code. Without his contribution reaching the best quality would
not have been possible.
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
drivers/char/tpm/Makefile | 2 +-
drivers/char/tpm/tpm-chip.c | 190 ++++++++++++++++++++++++++++++++++++
drivers/char/tpm/tpm-interface.c | 148 +---------------------------
drivers/char/tpm/tpm.h | 11 ++-
drivers/char/tpm/tpm_atmel.c | 11 ++-
drivers/char/tpm/tpm_i2c_atmel.c | 33 +++----
drivers/char/tpm/tpm_i2c_infineon.c | 37 ++-----
drivers/char/tpm/tpm_i2c_nuvoton.c | 44 +++------
drivers/char/tpm/tpm_i2c_stm_st33.c | 22 ++---
drivers/char/tpm/tpm_ibmvtpm.c | 17 ++--
drivers/char/tpm/tpm_infineon.c | 13 ++-
drivers/char/tpm/tpm_nsc.c | 11 ++-
drivers/char/tpm/tpm_tis.c | 79 ++++++---------
drivers/char/tpm/xen-tpmfront.c | 14 +--
14 files changed, 316 insertions(+), 316 deletions(-)
create mode 100644 drivers/char/tpm/tpm-chip.c
diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
index 4d85dd6..837da04 100644
--- a/drivers/char/tpm/Makefile
+++ b/drivers/char/tpm/Makefile
@@ -2,7 +2,7 @@
# Makefile for the kernel tpm device drivers.
#
obj-$(CONFIG_TCG_TPM) += tpm.o
-tpm-y := tpm-interface.o tpm-dev.o tpm-sysfs.o
+tpm-y := tpm-interface.o tpm-dev.o tpm-sysfs.o tpm-chip.o
tpm-$(CONFIG_ACPI) += tpm_ppi.o
ifdef CONFIG_ACPI
diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
new file mode 100644
index 0000000..4a29421
--- /dev/null
+++ b/drivers/char/tpm/tpm-chip.c
@@ -0,0 +1,190 @@
+/*
+ * Copyright (C) 2004 IBM Corporation
+ * Copyright (C) 2014 Intel Corporation
+ *
+ * Authors:
+ * Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
+ * Leendert van Doorn <leendert@watson.ibm.com>
+ * Dave Safford <safford@watson.ibm.com>
+ * Reiner Sailer <sailer@watson.ibm.com>
+ * Kylene Hall <kjhall@us.ibm.com>
+ *
+ * Maintained by: <tpmdd-devel@lists.sourceforge.net>
+ *
+ * TPM chip management routines.
+ *
+ * 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, version 2 of the
+ * License.
+ *
+ */
+
+#include <linux/poll.h>
+#include <linux/slab.h>
+#include <linux/mutex.h>
+#include <linux/spinlock.h>
+#include <linux/freezer.h>
+#include "tpm.h"
+#include "tpm_eventlog.h"
+
+static DECLARE_BITMAP(dev_mask, TPM_NUM_DEVICES);
+static LIST_HEAD(tpm_chip_list);
+static DEFINE_SPINLOCK(driver_lock);
+
+/*
+ * tpm_chip_find_get - return tpm_chip for a given chip number
+ * @chip_num the device number for the chip
+ */
+struct tpm_chip *tpm_chip_find_get(int chip_num)
+{
+ struct tpm_chip *pos, *chip = NULL;
+
+ rcu_read_lock();
+ list_for_each_entry_rcu(pos, &tpm_chip_list, list) {
+ if (chip_num != TPM_ANY_NUM && chip_num != pos->dev_num)
+ continue;
+
+ if (try_module_get(pos->dev->driver->owner)) {
+ chip = pos;
+ break;
+ }
+ }
+ rcu_read_unlock();
+ return chip;
+}
+
+/**
+ * tpmm_chip_remove() - free chip memory and device number
+ * @data: points to struct tpm_chip instance
+ *
+ * This is used internally by tpmm_chip_alloc() and called by devres
+ * when the device is released. This funcion does the opposite of
+ * tpmm_chip_alloc() freeing memory and the device number.
+ */
+static void tpmm_chip_remove(void *data)
+{
+ struct tpm_chip *chip = (struct tpm_chip *) data;
+ dev_dbg(chip->dev, "%s\n", __func__);
+ clear_bit(chip->dev_num, dev_mask);
+ kfree(chip);
+}
+
+/**
+ * tpmm_chip_alloc() - allocate a new struct tpm_chip instance
+ * @dev: device to which the chip is associated
+ * @ops: struct tpm_class_ops instance
+ *
+ * Allocates a new struct tpm_chip instance and assigns a free
+ * device number for it. Caller does not have to worry about
+ * freeing the allocated resources. When the devices is removed
+ * devres calls tpmm_chip_remove() to do the job.
+ */
+struct tpm_chip *tpmm_chip_alloc(struct device *dev,
+ const struct tpm_class_ops *ops)
+{
+ struct tpm_chip *chip;
+
+ chip = kzalloc(sizeof(*chip), GFP_KERNEL);
+ if (chip == NULL)
+ return ERR_PTR(-ENOMEM);
+
+ mutex_init(&chip->tpm_mutex);
+ INIT_LIST_HEAD(&chip->list);
+
+ chip->ops = ops;
+ chip->dev_num = find_first_zero_bit(dev_mask, TPM_NUM_DEVICES);
+
+ if (chip->dev_num >= TPM_NUM_DEVICES) {
+ dev_err(dev, "No available tpm device numbers\n");
+ kfree(chip);
+ return ERR_PTR(-ENOMEM);
+ }
+
+ set_bit(chip->dev_num, dev_mask);
+
+ scnprintf(chip->devname, sizeof(chip->devname), "%s%d", "tpm",
+ chip->dev_num);
+
+ chip->dev = dev;
+ devm_add_action(dev, tpmm_chip_remove, chip);
+ dev_set_drvdata(dev, chip);
+
+ return chip;
+}
+EXPORT_SYMBOL_GPL(tpmm_chip_alloc);
+
+/*
+ * tpm_chip_register() - create a misc driver for the TPM chip
+ * @chip: TPM chip to use.
+ *
+ * Creates a misc driver for the TPM chip and adds sysfs interfaces for
+ * the device, PPI and TCPA. As the last step this function adds the
+ * chip to the list of TPM chips available for use.
+ *
+ * NOTE: This function should be only called after the chip initialization
+ * is complete.
+ *
+ * Called from tpm_<specific>.c probe function only for devices
+ * the driver has determined it should claim. Prior to calling
+ * this function the specific probe function has called pci_enable_device
+ * upon errant exit from this function specific probe function should call
+ * pci_disable_device
+ */
+int tpm_chip_register(struct tpm_chip *chip)
+{
+ int rc;
+
+ rc = tpm_dev_add_device(chip);
+ if (rc)
+ return rc;
+
+ rc = tpm_sysfs_add_device(chip);
+ if (rc)
+ goto del_misc;
+
+ rc = tpm_add_ppi(&chip->dev->kobj);
+ if (rc)
+ goto del_sysfs;
+
+ chip->bios_dir = tpm_bios_log_setup(chip->devname);
+
+ /* Make the chip available. */
+ spin_lock(&driver_lock);
+ list_add_rcu(&chip->list, &tpm_chip_list);
+ spin_unlock(&driver_lock);
+
+ return 0;
+del_sysfs:
+ tpm_sysfs_del_device(chip);
+del_misc:
+ tpm_dev_del_device(chip);
+ return rc;
+}
+EXPORT_SYMBOL_GPL(tpm_chip_register);
+
+/*
+ * tpm_chip_unregister() - release the TPM driver
+ * @chip: TPM chip to use.
+ *
+ * Takes the chip first away from the list of available TPM chips and then
+ * cleans up all the resources reserved by tpm_chip_register().
+ *
+ * NOTE: This function should be only called before deinitializing chip
+ * resources.
+ */
+void tpm_chip_unregister(struct tpm_chip *chip)
+{
+ dev_dbg(chip->dev, "%s\n", __func__);
+
+ spin_lock(&driver_lock);
+ list_del_rcu(&chip->list);
+ spin_unlock(&driver_lock);
+ synchronize_rcu();
+
+ tpm_sysfs_del_device(chip);
+ tpm_remove_ppi(&chip->dev->kobj);
+ tpm_bios_log_teardown(chip->bios_dir);
+ tpm_dev_del_device(chip);
+}
+EXPORT_SYMBOL_GPL(tpm_chip_unregister);
diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index fedb4d5..59d6654 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -1,5 +1,6 @@
/*
* Copyright (C) 2004 IBM Corporation
+ * Copyright (C) 2014 Intel Corporation
*
* Authors:
* Leendert van Doorn <leendert@watson.ibm.com>
@@ -47,10 +48,6 @@ module_param_named(suspend_pcr, tpm_suspend_pcr, uint, 0644);
MODULE_PARM_DESC(suspend_pcr,
"PCR to use for dummy writes to faciltate flush on suspend.");
-static LIST_HEAD(tpm_chip_list);
-static DEFINE_SPINLOCK(driver_lock);
-static DECLARE_BITMAP(dev_mask, TPM_NUM_DEVICES);
-
/*
* Array with one entry per ordinal defining the maximum amount
* of time the chip could take to return the result. The ordinal
@@ -639,27 +636,6 @@ static int tpm_continue_selftest(struct tpm_chip *chip)
return rc;
}
-/*
- * tpm_chip_find_get - return tpm_chip for given chip number
- */
-static struct tpm_chip *tpm_chip_find_get(int chip_num)
-{
- struct tpm_chip *pos, *chip = NULL;
-
- rcu_read_lock();
- list_for_each_entry_rcu(pos, &tpm_chip_list, list) {
- if (chip_num != TPM_ANY_NUM && chip_num != pos->dev_num)
- continue;
-
- if (try_module_get(pos->dev->driver->owner)) {
- chip = pos;
- break;
- }
- }
- rcu_read_unlock();
- return chip;
-}
-
#define TPM_ORDINAL_PCRREAD cpu_to_be32(21)
#define READ_PCR_RESULT_SIZE 30
static struct tpm_input_header pcrread_header = {
@@ -887,30 +863,6 @@ again:
}
EXPORT_SYMBOL_GPL(wait_for_tpm_stat);
-void tpm_remove_hardware(struct device *dev)
-{
- struct tpm_chip *chip = dev_get_drvdata(dev);
-
- if (chip == NULL) {
- dev_err(dev, "No device data found\n");
- return;
- }
-
- spin_lock(&driver_lock);
- list_del_rcu(&chip->list);
- spin_unlock(&driver_lock);
- synchronize_rcu();
-
- tpm_dev_del_device(chip);
- tpm_sysfs_del_device(chip);
- tpm_remove_ppi(&dev->kobj);
- tpm_bios_log_teardown(chip->bios_dir);
-
- /* write it this way to be explicit (chip->dev == dev) */
- put_device(chip->dev);
-}
-EXPORT_SYMBOL_GPL(tpm_remove_hardware);
-
#define TPM_ORD_SAVESTATE cpu_to_be32(152)
#define SAVESTATE_RESULT_SIZE 10
@@ -1044,104 +996,6 @@ int tpm_get_random(u32 chip_num, u8 *out, size_t max)
}
EXPORT_SYMBOL_GPL(tpm_get_random);
-/* In case vendor provided release function, call it too.*/
-
-void tpm_dev_vendor_release(struct tpm_chip *chip)
-{
- if (!chip)
- return;
-
- clear_bit(chip->dev_num, dev_mask);
-}
-EXPORT_SYMBOL_GPL(tpm_dev_vendor_release);
-
-
-/*
- * Once all references to platform device are down to 0,
- * release all allocated structures.
- */
-static void tpm_dev_release(struct device *dev)
-{
- struct tpm_chip *chip = dev_get_drvdata(dev);
-
- if (!chip)
- return;
-
- tpm_dev_vendor_release(chip);
-
- chip->release(dev);
- kfree(chip);
-}
-
-/*
- * Called from tpm_<specific>.c probe function only for devices
- * the driver has determined it should claim. Prior to calling
- * this function the specific probe function has called pci_enable_device
- * upon errant exit from this function specific probe function should call
- * pci_disable_device
- */
-struct tpm_chip *tpm_register_hardware(struct device *dev,
- const struct tpm_class_ops *ops)
-{
- struct tpm_chip *chip;
-
- /* Driver specific per-device data */
- chip = kzalloc(sizeof(*chip), GFP_KERNEL);
-
- if (chip == NULL)
- return NULL;
-
- mutex_init(&chip->tpm_mutex);
- INIT_LIST_HEAD(&chip->list);
-
- chip->ops = ops;
- chip->dev_num = find_first_zero_bit(dev_mask, TPM_NUM_DEVICES);
-
- if (chip->dev_num >= TPM_NUM_DEVICES) {
- dev_err(dev, "No available tpm device numbers\n");
- goto out_free;
- }
-
- set_bit(chip->dev_num, dev_mask);
-
- scnprintf(chip->devname, sizeof(chip->devname), "%s%d", "tpm",
- chip->dev_num);
-
- chip->dev = get_device(dev);
- chip->release = dev->release;
- dev->release = tpm_dev_release;
- dev_set_drvdata(dev, chip);
-
- if (tpm_dev_add_device(chip))
- goto put_device;
-
- if (tpm_sysfs_add_device(chip))
- goto del_misc;
-
- if (tpm_add_ppi(&dev->kobj))
- goto del_sysfs;
-
- chip->bios_dir = tpm_bios_log_setup(chip->devname);
-
- /* Make chip available */
- spin_lock(&driver_lock);
- list_add_rcu(&chip->list, &tpm_chip_list);
- spin_unlock(&driver_lock);
-
- return chip;
-
-del_sysfs:
- tpm_sysfs_del_device(chip);
-del_misc:
- tpm_dev_del_device(chip);
-put_device:
- put_device(chip->dev);
-out_free:
- kfree(chip);
- return NULL;
-}
-EXPORT_SYMBOL_GPL(tpm_register_hardware);
-
MODULE_AUTHOR("Leendert van Doorn (leendert@watson.ibm.com)");
MODULE_DESCRIPTION("TPM Driver");
MODULE_VERSION("2.0");
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 12326e1..fa0974c 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -110,7 +110,6 @@ struct tpm_chip {
struct dentry **bios_dir;
struct list_head list;
- void (*release) (struct device *);
};
#define to_tpm_chip(n) container_of(n, struct tpm_chip, vendor)
@@ -320,15 +319,17 @@ extern int tpm_get_timeouts(struct tpm_chip *);
extern void tpm_gen_interrupt(struct tpm_chip *);
extern int tpm_do_selftest(struct tpm_chip *);
extern unsigned long tpm_calc_ordinal_duration(struct tpm_chip *, u32);
-extern struct tpm_chip* tpm_register_hardware(struct device *,
- const struct tpm_class_ops *ops);
-extern void tpm_dev_vendor_release(struct tpm_chip *);
-extern void tpm_remove_hardware(struct device *);
extern int tpm_pm_suspend(struct device *);
extern int tpm_pm_resume(struct device *);
extern int wait_for_tpm_stat(struct tpm_chip *, u8, unsigned long,
wait_queue_head_t *, bool);
+struct tpm_chip *tpm_chip_find_get(int chip_num);
+extern struct tpm_chip *tpmm_chip_alloc(struct device *dev,
+ const struct tpm_class_ops *ops);
+extern int tpm_chip_register(struct tpm_chip *chip);
+extern void tpm_chip_unregister(struct tpm_chip *chip);
+
int tpm_dev_add_device(struct tpm_chip *chip);
void tpm_dev_del_device(struct tpm_chip *chip);
int tpm_sysfs_add_device(struct tpm_chip *chip);
diff --git a/drivers/char/tpm/tpm_atmel.c b/drivers/char/tpm/tpm_atmel.c
index 6069d13..8e2576a 100644
--- a/drivers/char/tpm/tpm_atmel.c
+++ b/drivers/char/tpm/tpm_atmel.c
@@ -138,11 +138,11 @@ static void atml_plat_remove(void)
struct tpm_chip *chip = dev_get_drvdata(&pdev->dev);
if (chip) {
+ tpm_chip_unregister(chip);
if (chip->vendor.have_region)
atmel_release_region(chip->vendor.base,
chip->vendor.region_size);
atmel_put_base_addr(chip->vendor.iobase);
- tpm_remove_hardware(chip->dev);
platform_device_unregister(pdev);
}
}
@@ -184,8 +184,9 @@ static int __init init_atmel(void)
goto err_rel_reg;
}
- if (!(chip = tpm_register_hardware(&pdev->dev, &tpm_atmel))) {
- rc = -ENODEV;
+ chip = tpmm_chip_alloc(&pdev->dev, &tpm_atmel);
+ if (IS_ERR(chip)) {
+ rc = PTR_ERR(chip);
goto err_unreg_dev;
}
@@ -194,6 +195,10 @@ static int __init init_atmel(void)
chip->vendor.have_region = have_region;
chip->vendor.region_size = region_size;
+ rc = tpm_chip_register(chip);
+ if (rc)
+ goto err_unreg_dev;
+
return 0;
err_unreg_dev:
diff --git a/drivers/char/tpm/tpm_i2c_atmel.c b/drivers/char/tpm/tpm_i2c_atmel.c
index 7727292..8af3b4a 100644
--- a/drivers/char/tpm/tpm_i2c_atmel.c
+++ b/drivers/char/tpm/tpm_i2c_atmel.c
@@ -160,11 +160,9 @@ static int i2c_atmel_probe(struct i2c_client *client,
if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C))
return -ENODEV;
- chip = tpm_register_hardware(dev, &i2c_atmel);
- if (!chip) {
- dev_err(dev, "%s() error in tpm_register_hardware\n", __func__);
- return -ENODEV;
- }
+ chip = tpmm_chip_alloc(dev, &i2c_atmel);
+ if (IS_ERR(chip))
+ return PTR_ERR(chip);
chip->vendor.priv = devm_kzalloc(dev, sizeof(struct priv_data),
GFP_KERNEL);
@@ -179,21 +177,16 @@ static int i2c_atmel_probe(struct i2c_client *client,
/* There is no known way to probe for this device, and all version
* information seems to be read via TPM commands. Thus we rely on the
* TPM startup process in the common code to detect the device. */
- if (tpm_get_timeouts(chip)) {
- rc = -ENODEV;
- goto out_err;
- }
+ if (tpm_get_timeouts(chip))
+ return -ENODEV;
- if (tpm_do_selftest(chip)) {
- rc = -ENODEV;
- goto out_err;
- }
+ if (tpm_do_selftest(chip))
+ return -ENODEV;
- return 0;
+ rc = tpm_chip_register(chip);
+ if (rc)
+ return rc;
-out_err:
- tpm_dev_vendor_release(chip);
- tpm_remove_hardware(chip->dev);
return rc;
}
@@ -201,11 +194,7 @@ static int i2c_atmel_remove(struct i2c_client *client)
{
struct device *dev = &(client->dev);
struct tpm_chip *chip = dev_get_drvdata(dev);
-
- if (chip)
- tpm_dev_vendor_release(chip);
- tpm_remove_hardware(dev);
- kfree(chip);
+ tpm_chip_unregister(chip);
return 0;
}
diff --git a/drivers/char/tpm/tpm_i2c_infineon.c b/drivers/char/tpm/tpm_i2c_infineon.c
index 472af4b..6f00bc3 100644
--- a/drivers/char/tpm/tpm_i2c_infineon.c
+++ b/drivers/char/tpm/tpm_i2c_infineon.c
@@ -581,10 +581,9 @@ static int tpm_tis_i2c_init(struct device *dev)
int rc = 0;
struct tpm_chip *chip;
- chip = tpm_register_hardware(dev, &tpm_tis_i2c);
- if (!chip) {
- dev_err(dev, "could not register hardware\n");
- rc = -ENODEV;
+ chip = tpmm_chip_alloc(dev, &tpm_tis_i2c);
+ if (IS_ERR(chip)) {
+ rc = PTR_ERR(chip);
goto out_err;
}
@@ -600,7 +599,7 @@ static int tpm_tis_i2c_init(struct device *dev)
if (request_locality(chip, 0) != 0) {
dev_err(dev, "could not request locality\n");
rc = -ENODEV;
- goto out_vendor;
+ goto out_err;
}
/* read four bytes from DID_VID register */
@@ -628,21 +627,13 @@ static int tpm_tis_i2c_init(struct device *dev)
tpm_get_timeouts(chip);
tpm_do_selftest(chip);
- return 0;
+ rc = tpm_chip_register(chip);
+ if (rc)
+ goto out_release;
+ return 0;
out_release:
release_locality(chip, chip->vendor.locality, 1);
-
-out_vendor:
- /* close file handles */
- tpm_dev_vendor_release(chip);
-
- /* remove hardware */
- tpm_remove_hardware(chip->dev);
-
- /* reset these pointers, otherwise we oops */
- chip->dev->release = NULL;
- chip->release = NULL;
tpm_dev.client = NULL;
out_err:
return rc;
@@ -712,17 +703,9 @@ static int tpm_tis_i2c_probe(struct i2c_client *client,
static int tpm_tis_i2c_remove(struct i2c_client *client)
{
struct tpm_chip *chip = tpm_dev.chip;
- release_locality(chip, chip->vendor.locality, 1);
- /* close file handles */
- tpm_dev_vendor_release(chip);
-
- /* remove hardware */
- tpm_remove_hardware(chip->dev);
-
- /* reset these pointers, otherwise we oops */
- chip->dev->release = NULL;
- chip->release = NULL;
+ tpm_chip_unregister(chip);
+ release_locality(chip, chip->vendor.locality, 1);
tpm_dev.client = NULL;
return 0;
diff --git a/drivers/char/tpm/tpm_i2c_nuvoton.c b/drivers/char/tpm/tpm_i2c_nuvoton.c
index 7b158ef..09f0c46 100644
--- a/drivers/char/tpm/tpm_i2c_nuvoton.c
+++ b/drivers/char/tpm/tpm_i2c_nuvoton.c
@@ -530,11 +530,9 @@ static int i2c_nuvoton_probe(struct i2c_client *client,
dev_info(dev, "VID: %04X DID: %02X RID: %02X\n", (u16) vid,
(u8) (vid >> 16), (u8) (vid >> 24));
- chip = tpm_register_hardware(dev, &tpm_i2c);
- if (!chip) {
- dev_err(dev, "%s() error in tpm_register_hardware\n", __func__);
- return -ENODEV;
- }
+ chip = tpmm_chip_alloc(dev, &tpm_i2c);
+ if (IS_ERR(chip))
+ return PTR_ERR(chip);
chip->vendor.priv = devm_kzalloc(dev, sizeof(struct priv_data),
GFP_KERNEL);
@@ -584,7 +582,7 @@ static int i2c_nuvoton_probe(struct i2c_client *client,
TPM_DATA_FIFO_W,
1, (u8 *) (&rc));
if (rc < 0)
- goto out_err;
+ return rc;
/* TPM_STS <- 0x40 (commandReady) */
i2c_nuvoton_ready(chip);
} else {
@@ -594,45 +592,33 @@ static int i2c_nuvoton_probe(struct i2c_client *client,
* only TPM_STS_VALID should be set
*/
if (i2c_nuvoton_read_status(chip) !=
- TPM_STS_VALID) {
- rc = -EIO;
- goto out_err;
- }
+ TPM_STS_VALID)
+ return -EIO;
}
}
}
- if (tpm_get_timeouts(chip)) {
- rc = -ENODEV;
- goto out_err;
- }
+ if (tpm_get_timeouts(chip))
+ return -ENODEV;
- if (tpm_do_selftest(chip)) {
- rc = -ENODEV;
- goto out_err;
- }
+ if (tpm_do_selftest(chip))
+ return -ENODEV;
- return 0;
+ rc = tpm_chip_register(chip);
+ if (rc)
+ return rc;
-out_err:
- tpm_dev_vendor_release(chip);
- tpm_remove_hardware(chip->dev);
- return rc;
+ return 0;
}
static int i2c_nuvoton_remove(struct i2c_client *client)
{
struct device *dev = &(client->dev);
struct tpm_chip *chip = dev_get_drvdata(dev);
-
- if (chip)
- tpm_dev_vendor_release(chip);
- tpm_remove_hardware(dev);
- kfree(chip);
+ tpm_chip_unregister(chip);
return 0;
}
-
static const struct i2c_device_id i2c_nuvoton_id[] = {
{I2C_DRIVER_NAME, 0},
{}
diff --git a/drivers/char/tpm/tpm_i2c_stm_st33.c b/drivers/char/tpm/tpm_i2c_stm_st33.c
index 4669e37..dd32429 100644
--- a/drivers/char/tpm/tpm_i2c_stm_st33.c
+++ b/drivers/char/tpm/tpm_i2c_stm_st33.c
@@ -619,10 +619,9 @@ tpm_st33_i2c_probe(struct i2c_client *client, const struct i2c_device_id *id)
goto end;
}
- chip = tpm_register_hardware(&client->dev, &st_i2c_tpm);
- if (!chip) {
- dev_info(&client->dev, "fail chip\n");
- err = -ENODEV;
+ chip = tpmm_chip_alloc(&client->dev, &st_i2c_tpm);
+ if (IS_ERR(chip)) {
+ err = PTR_ERR(chip);
goto end;
}
@@ -631,14 +630,14 @@ tpm_st33_i2c_probe(struct i2c_client *client, const struct i2c_device_id *id)
if (!platform_data) {
dev_info(&client->dev, "chip not available\n");
err = -ENODEV;
- goto _tpm_clean_answer;
+ goto end;
}
platform_data->tpm_i2c_buffer[0] =
kmalloc(TPM_BUFSIZE * sizeof(u8), GFP_KERNEL);
if (platform_data->tpm_i2c_buffer[0] == NULL) {
err = -ENOMEM;
- goto _tpm_clean_answer;
+ goto end;
}
platform_data->tpm_i2c_buffer[1] =
kmalloc(TPM_BUFSIZE * sizeof(u8), GFP_KERNEL);
@@ -716,7 +715,10 @@ tpm_st33_i2c_probe(struct i2c_client *client, const struct i2c_device_id *id)
tpm_get_timeouts(chip);
tpm_do_selftest(chip);
- dev_info(chip->dev, "TPM I2C Initialized\n");
+ err = tpm_chip_register(chip);
+ if (err)
+ goto _irq_set;
+
return 0;
_irq_set:
free_irq(gpio_to_irq(platform_data->io_serirq), (void *)chip);
@@ -732,8 +734,6 @@ _tpm_clean_response2:
_tpm_clean_response1:
kzfree(platform_data->tpm_i2c_buffer[0]);
platform_data->tpm_i2c_buffer[0] = NULL;
-_tpm_clean_answer:
- tpm_remove_hardware(chip->dev);
end:
pr_info("TPM I2C initialisation fail\n");
return err;
@@ -752,13 +752,13 @@ static int tpm_st33_i2c_remove(struct i2c_client *client)
((struct i2c_client *)TPM_VPRIV(chip))->dev.platform_data;
if (pin_infos != NULL) {
+ tpm_chip_unregister(chip);
+
free_irq(pin_infos->io_serirq, chip);
gpio_free(pin_infos->io_serirq);
gpio_free(pin_infos->io_lpcpd);
- tpm_remove_hardware(chip->dev);
-
if (pin_infos->tpm_i2c_buffer[1] != NULL) {
kzfree(pin_infos->tpm_i2c_buffer[1]);
pin_infos->tpm_i2c_buffer[1] = NULL;
diff --git a/drivers/char/tpm/tpm_ibmvtpm.c b/drivers/char/tpm/tpm_ibmvtpm.c
index af74c57..eb95796 100644
--- a/drivers/char/tpm/tpm_ibmvtpm.c
+++ b/drivers/char/tpm/tpm_ibmvtpm.c
@@ -270,8 +270,11 @@ static int ibmvtpm_crq_send_init(struct ibmvtpm_dev *ibmvtpm)
static int tpm_ibmvtpm_remove(struct vio_dev *vdev)
{
struct ibmvtpm_dev *ibmvtpm = ibmvtpm_get_data(&vdev->dev);
+ struct tpm_chip *chip = dev_get_drvdata(ibmvtpm->dev);
int rc = 0;
+ tpm_chip_unregister(chip);
+
free_irq(vdev->irq, ibmvtpm);
do {
@@ -290,8 +293,6 @@ static int tpm_ibmvtpm_remove(struct vio_dev *vdev)
kfree(ibmvtpm->rtce_buf);
}
- tpm_remove_hardware(ibmvtpm->dev);
-
kfree(ibmvtpm);
return 0;
@@ -555,11 +556,9 @@ static int tpm_ibmvtpm_probe(struct vio_dev *vio_dev,
struct tpm_chip *chip;
int rc = -ENOMEM, rc1;
- chip = tpm_register_hardware(dev, &tpm_ibmvtpm);
- if (!chip) {
- dev_err(dev, "tpm_register_hardware failed\n");
- return -ENODEV;
- }
+ chip = tpmm_chip_alloc(dev, &tpm_ibmvtpm);
+ if (IS_ERR(chip))
+ return PTR_ERR(chip);
ibmvtpm = kzalloc(sizeof(struct ibmvtpm_dev), GFP_KERNEL);
if (!ibmvtpm) {
@@ -629,7 +628,7 @@ static int tpm_ibmvtpm_probe(struct vio_dev *vio_dev,
if (rc)
goto init_irq_cleanup;
- return rc;
+ return tpm_chip_register(chip);
init_irq_cleanup:
do {
rc1 = plpar_hcall_norets(H_FREE_CRQ, vio_dev->unit_address);
@@ -644,8 +643,6 @@ cleanup:
kfree(ibmvtpm);
}
- tpm_remove_hardware(dev);
-
return rc;
}
diff --git a/drivers/char/tpm/tpm_infineon.c b/drivers/char/tpm/tpm_infineon.c
index dc0a255..d1559f6 100644
--- a/drivers/char/tpm/tpm_infineon.c
+++ b/drivers/char/tpm/tpm_infineon.c
@@ -546,7 +546,14 @@ static int tpm_inf_pnp_probe(struct pnp_dev *dev,
vendorid[0], vendorid[1],
productid[0], productid[1], chipname);
- if (!(chip = tpm_register_hardware(&dev->dev, &tpm_inf)))
+ chip = tpmm_chip_alloc(&dev->dev, &tpm_inf);
+ if (IS_ERR(chip)) {
+ rc = PTR_ERR(chip);
+ goto err_release_region;
+ }
+
+ rc = tpm_chip_register(chip);
+ if (rc)
goto err_release_region;
return 0;
@@ -573,6 +580,8 @@ static void tpm_inf_pnp_remove(struct pnp_dev *dev)
struct tpm_chip *chip = pnp_get_drvdata(dev);
if (chip) {
+ tpm_chip_unregister(chip);
+
if (tpm_dev.iotype == TPM_INF_IO_PORT) {
release_region(tpm_dev.data_regs, tpm_dev.data_size);
release_region(tpm_dev.config_port,
@@ -581,8 +590,6 @@ static void tpm_inf_pnp_remove(struct pnp_dev *dev)
iounmap(tpm_dev.mem_base);
release_mem_region(tpm_dev.map_base, tpm_dev.map_size);
}
- tpm_dev_vendor_release(chip);
- tpm_remove_hardware(chip->dev);
}
}
diff --git a/drivers/char/tpm/tpm_nsc.c b/drivers/char/tpm/tpm_nsc.c
index 3179ec9..151f96a 100644
--- a/drivers/char/tpm/tpm_nsc.c
+++ b/drivers/char/tpm/tpm_nsc.c
@@ -247,9 +247,9 @@ static struct platform_device *pdev = NULL;
static void tpm_nsc_remove(struct device *dev)
{
struct tpm_chip *chip = dev_get_drvdata(dev);
- if ( chip ) {
+ if (chip) {
+ tpm_chip_unregister(chip);
release_region(chip->vendor.base, 2);
- tpm_remove_hardware(chip->dev);
}
}
@@ -308,11 +308,16 @@ static int __init init_nsc(void)
goto err_del_dev;
}
- if (!(chip = tpm_register_hardware(&pdev->dev, &tpm_nsc))) {
+ chip = tpmm_chip_alloc(&pdev->dev, &tpm_nsc);
+ if (IS_ERR(chip)) {
rc = -ENODEV;
goto err_rel_reg;
}
+ rc = tpm_chip_register(chip);
+ if (rc)
+ goto err_rel_reg;
+
dev_dbg(&pdev->dev, "NSC TPM detected\n");
dev_dbg(&pdev->dev,
"NSC LDN 0x%x, SID 0x%x, SRID 0x%x\n",
diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c
index 2c46734..e2cb04d 100644
--- a/drivers/char/tpm/tpm_tis.c
+++ b/drivers/char/tpm/tpm_tis.c
@@ -75,9 +75,6 @@ enum tis_defaults {
#define TPM_DID_VID(l) (0x0F00 | ((l) << 12))
#define TPM_RID(l) (0x0F04 | ((l) << 12))
-static LIST_HEAD(tis_chips);
-static DEFINE_MUTEX(tis_lock);
-
#if defined(CONFIG_PNP) && defined(CONFIG_ACPI)
static int is_itpm(struct pnp_dev *dev)
{
@@ -528,6 +525,17 @@ static bool interrupts = true;
module_param(interrupts, bool, 0444);
MODULE_PARM_DESC(interrupts, "Enable interrupts");
+static void tpm_tis_remove(struct tpm_chip *chip)
+{
+ iowrite32(~TPM_GLOBAL_INT_ENABLE &
+ ioread32(chip->vendor.iobase +
+ TPM_INT_ENABLE(chip->vendor.
+ locality)),
+ chip->vendor.iobase +
+ TPM_INT_ENABLE(chip->vendor.locality));
+ release_locality(chip, chip->vendor.locality, 1);
+}
+
static int tpm_tis_init(struct device *dev, resource_size_t start,
resource_size_t len, unsigned int irq)
{
@@ -535,14 +543,13 @@ static int tpm_tis_init(struct device *dev, resource_size_t start,
int rc, i, irq_s, irq_e, probe;
struct tpm_chip *chip;
- if (!(chip = tpm_register_hardware(dev, &tpm_tis)))
- return -ENODEV;
+ chip = tpmm_chip_alloc(dev, &tpm_tis);
+ if (IS_ERR(chip))
+ return PTR_ERR(chip);
- chip->vendor.iobase = ioremap(start, len);
- if (!chip->vendor.iobase) {
- rc = -EIO;
- goto out_err;
- }
+ chip->vendor.iobase = devm_ioremap(dev, start, len);
+ if (!chip->vendor.iobase)
+ return -EIO;
/* Default timeouts */
chip->vendor.timeout_a = msecs_to_jiffies(TIS_SHORT_TIMEOUT);
@@ -649,8 +656,8 @@ static int tpm_tis_init(struct device *dev, resource_size_t start,
for (i = irq_s; i <= irq_e && chip->vendor.irq == 0; i++) {
iowrite8(i, chip->vendor.iobase +
TPM_INT_VECTOR(chip->vendor.locality));
- if (request_irq
- (i, tis_int_probe, IRQF_SHARED,
+ if (devm_request_irq
+ (dev, i, tis_int_probe, IRQF_SHARED,
chip->vendor.miscdev.name, chip) != 0) {
dev_info(chip->dev,
"Unable to request irq: %d for probe\n",
@@ -690,15 +697,14 @@ static int tpm_tis_init(struct device *dev, resource_size_t start,
iowrite32(intmask,
chip->vendor.iobase +
TPM_INT_ENABLE(chip->vendor.locality));
- free_irq(i, chip);
}
}
if (chip->vendor.irq) {
iowrite8(chip->vendor.irq,
chip->vendor.iobase +
TPM_INT_VECTOR(chip->vendor.locality));
- if (request_irq
- (chip->vendor.irq, tis_int_handler, IRQF_SHARED,
+ if (devm_request_irq
+ (dev, chip->vendor.irq, tis_int_handler, IRQF_SHARED,
chip->vendor.miscdev.name, chip) != 0) {
dev_info(chip->dev,
"Unable to request irq: %d for use\n",
@@ -719,17 +725,9 @@ static int tpm_tis_init(struct device *dev, resource_size_t start,
}
}
- INIT_LIST_HEAD(&chip->vendor.list);
- mutex_lock(&tis_lock);
- list_add(&chip->vendor.list, &tis_chips);
- mutex_unlock(&tis_lock);
-
-
- return 0;
+ return tpm_chip_register(chip);
out_err:
- if (chip->vendor.iobase)
- iounmap(chip->vendor.iobase);
- tpm_remove_hardware(chip->dev);
+ tpm_tis_remove(chip);
return rc;
}
@@ -811,13 +809,10 @@ MODULE_DEVICE_TABLE(pnp, tpm_pnp_tbl);
static void tpm_tis_pnp_remove(struct pnp_dev *dev)
{
struct tpm_chip *chip = pnp_get_drvdata(dev);
-
- tpm_dev_vendor_release(chip);
-
- kfree(chip);
+ tpm_chip_unregister(chip);
+ tpm_tis_remove(chip);
}
-
static struct pnp_driver tis_pnp_driver = {
.name = "tpm_tis",
.id_table = tpm_pnp_tbl,
@@ -836,7 +831,7 @@ MODULE_PARM_DESC(hid, "Set additional specific HID for this driver to probe");
static struct platform_driver tis_drv = {
.driver = {
- .name = "tpm_tis",
+ .name = "tpm_tis",
.owner = THIS_MODULE,
.pm = &tpm_tis_pm,
},
@@ -876,31 +871,17 @@ err_dev:
static void __exit cleanup_tis(void)
{
- struct tpm_vendor_specific *i, *j;
struct tpm_chip *chip;
- mutex_lock(&tis_lock);
- list_for_each_entry_safe(i, j, &tis_chips, list) {
- chip = to_tpm_chip(i);
- tpm_remove_hardware(chip->dev);
- iowrite32(~TPM_GLOBAL_INT_ENABLE &
- ioread32(chip->vendor.iobase +
- TPM_INT_ENABLE(chip->vendor.
- locality)),
- chip->vendor.iobase +
- TPM_INT_ENABLE(chip->vendor.locality));
- release_locality(chip, chip->vendor.locality, 1);
- if (chip->vendor.irq)
- free_irq(chip->vendor.irq, chip);
- iounmap(i->iobase);
- list_del(&i->list);
- }
- mutex_unlock(&tis_lock);
#ifdef CONFIG_PNP
if (!force) {
pnp_unregister_driver(&tis_pnp_driver);
return;
}
#endif
+ chip = dev_get_drvdata(&pdev->dev);
+ /* FIXME might be broken when using force */
+ tpm_chip_unregister(chip);
+ tpm_tis_remove(chip);
platform_device_unregister(pdev);
platform_driver_unregister(&tis_drv);
}
diff --git a/drivers/char/tpm/xen-tpmfront.c b/drivers/char/tpm/xen-tpmfront.c
index 441b44e..c3b4f5a 100644
--- a/drivers/char/tpm/xen-tpmfront.c
+++ b/drivers/char/tpm/xen-tpmfront.c
@@ -175,9 +175,9 @@ static int setup_chip(struct device *dev, struct tpm_private *priv)
{
struct tpm_chip *chip;
- chip = tpm_register_hardware(dev, &tpm_vtpm);
- if (!chip)
- return -ENODEV;
+ chip = tpmm_chip_alloc(dev, &tpm_vtpm);
+ if (IS_ERR(chip))
+ return PTR_ERR(chip);
init_waitqueue_head(&chip->vendor.read_queue);
@@ -286,6 +286,7 @@ static int tpmfront_probe(struct xenbus_device *dev,
const struct xenbus_device_id *id)
{
struct tpm_private *priv;
+ struct tpm_chip *chip;
int rv;
priv = kzalloc(sizeof(*priv), GFP_KERNEL);
@@ -302,21 +303,22 @@ static int tpmfront_probe(struct xenbus_device *dev,
rv = setup_ring(dev, priv);
if (rv) {
- tpm_remove_hardware(&dev->dev);
+ chip = dev_get_drvdata(&dev->dev);
+ tpm_chip_unregister(chip);
ring_free(priv);
return rv;
}
tpm_get_timeouts(priv->chip);
- return rv;
+ return tpm_chip_register(priv->chip);
}
static int tpmfront_remove(struct xenbus_device *dev)
{
struct tpm_chip *chip = dev_get_drvdata(&dev->dev);
struct tpm_private *priv = TPM_VPRIV(chip);
- tpm_remove_hardware(&dev->dev);
+ tpm_chip_unregister(chip);
ring_free(priv);
TPM_VPRIV(chip) = NULL;
return 0;
--
2.1.0
^ permalink raw reply related
* [PATCH v1 1/3] tpm: merge duplicate transmit_cmd() functions
From: Jarkko Sakkinen @ 2014-10-22 16:23 UTC (permalink / raw)
To: Peter Huewe, Ashley Lai, Marcel Selhorst
Cc: tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA,
josh.triplett-ral2JQCrhuEAvxtiuMwx3w,
christophe.ricard-Re5JQEeQqe8AvxtiuMwx3w,
jason.gunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/, Jarkko Sakkinen
In-Reply-To: <1413995036-22497-1-git-send-email-jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Merged transmit_cmd() functions in tpm-interface.c and tpm-sysfs.c.
Added "tpm_" prefix for consistency sake. Changed cmd parameter as
opaque. This enables to use separate command structures for TPM1
and TPM2 commands in future. Loose coupling works fine here.
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
drivers/char/tpm/tpm-dev.c | 4 +--
drivers/char/tpm/tpm-interface.c | 53 +++++++++++++++++++++-------------------
drivers/char/tpm/tpm-sysfs.c | 23 ++---------------
drivers/char/tpm/tpm.h | 5 ++--
4 files changed, 34 insertions(+), 51 deletions(-)
diff --git a/drivers/char/tpm/tpm-dev.c b/drivers/char/tpm/tpm-dev.c
index d9b774e..bd79d33 100644
--- a/drivers/char/tpm/tpm-dev.c
+++ b/drivers/char/tpm/tpm-dev.c
@@ -140,8 +140,8 @@ static ssize_t tpm_write(struct file *file, const char __user *buf,
}
/* atomic tpm command send and result receive */
- out_size = tpm_transmit(priv->chip, priv->data_buffer,
- sizeof(priv->data_buffer));
+ out_size = tpm_transmit_cmd(priv->chip, priv->data_buffer,
+ sizeof(priv->data_buffer), NULL);
if (out_size < 0) {
mutex_unlock(&priv->buffer_mutex);
return out_size;
diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index 6af1700..fedb4d5 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -331,8 +331,8 @@ EXPORT_SYMBOL_GPL(tpm_calc_ordinal_duration);
/*
* Internal kernel interface to transmit TPM commands
*/
-ssize_t tpm_transmit(struct tpm_chip *chip, const char *buf,
- size_t bufsiz)
+static ssize_t tpm_transmit(struct tpm_chip *chip, const char *buf,
+ size_t bufsiz)
{
ssize_t rc;
u32 count, ordinal;
@@ -398,9 +398,10 @@ out:
#define TPM_DIGEST_SIZE 20
#define TPM_RET_CODE_IDX 6
-static ssize_t transmit_cmd(struct tpm_chip *chip, struct tpm_cmd_t *cmd,
- int len, const char *desc)
+ssize_t tpm_transmit_cmd(struct tpm_chip *chip, void *cmd,
+ int len, const char *desc)
{
+ struct tpm_output_header *header;
int err;
len = tpm_transmit(chip, (u8 *) cmd, len);
@@ -409,7 +410,9 @@ static ssize_t transmit_cmd(struct tpm_chip *chip, struct tpm_cmd_t *cmd,
else if (len < TPM_HEADER_SIZE)
return -EFAULT;
- err = be32_to_cpu(cmd->header.out.return_code);
+ header = (struct tpm_output_header *) cmd;
+
+ err = be32_to_cpu(header->return_code);
if (err != 0 && desc)
dev_err(chip->dev, "A TPM error (%d) occurred %s\n", err, desc);
@@ -448,7 +451,7 @@ ssize_t tpm_getcap(struct device *dev, __be32 subcap_id, cap_t *cap,
tpm_cmd.params.getcap_in.subcap_size = cpu_to_be32(4);
tpm_cmd.params.getcap_in.subcap = subcap_id;
}
- rc = transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE, desc);
+ rc = tpm_transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE, desc);
if (!rc)
*cap = tpm_cmd.params.getcap_out.cap;
return rc;
@@ -464,8 +467,8 @@ void tpm_gen_interrupt(struct tpm_chip *chip)
tpm_cmd.params.getcap_in.subcap_size = cpu_to_be32(4);
tpm_cmd.params.getcap_in.subcap = TPM_CAP_PROP_TIS_TIMEOUT;
- rc = transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE,
- "attempting to determine the timeouts");
+ rc = tpm_transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE,
+ "attempting to determine the timeouts");
}
EXPORT_SYMBOL_GPL(tpm_gen_interrupt);
@@ -484,8 +487,8 @@ static int tpm_startup(struct tpm_chip *chip, __be16 startup_type)
struct tpm_cmd_t start_cmd;
start_cmd.header.in = tpm_startup_header;
start_cmd.params.startup_in.startup_type = startup_type;
- return transmit_cmd(chip, &start_cmd, TPM_INTERNAL_RESULT_SIZE,
- "attempting to start the TPM");
+ return tpm_transmit_cmd(chip, &start_cmd, TPM_INTERNAL_RESULT_SIZE,
+ "attempting to start the TPM");
}
int tpm_get_timeouts(struct tpm_chip *chip)
@@ -500,7 +503,7 @@ int tpm_get_timeouts(struct tpm_chip *chip)
tpm_cmd.params.getcap_in.cap = TPM_CAP_PROP;
tpm_cmd.params.getcap_in.subcap_size = cpu_to_be32(4);
tpm_cmd.params.getcap_in.subcap = TPM_CAP_PROP_TIS_TIMEOUT;
- rc = transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE, NULL);
+ rc = tpm_transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE, NULL);
if (rc == TPM_ERR_INVALID_POSTINIT) {
/* The TPM is not started, we are the first to talk to it.
@@ -513,7 +516,7 @@ int tpm_get_timeouts(struct tpm_chip *chip)
tpm_cmd.params.getcap_in.cap = TPM_CAP_PROP;
tpm_cmd.params.getcap_in.subcap_size = cpu_to_be32(4);
tpm_cmd.params.getcap_in.subcap = TPM_CAP_PROP_TIS_TIMEOUT;
- rc = transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE,
+ rc = tpm_transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE,
NULL);
}
if (rc) {
@@ -575,8 +578,8 @@ duration:
tpm_cmd.params.getcap_in.subcap_size = cpu_to_be32(4);
tpm_cmd.params.getcap_in.subcap = TPM_CAP_PROP_TIS_DURATION;
- rc = transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE,
- "attempting to determine the durations");
+ rc = tpm_transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE,
+ "attempting to determine the durations");
if (rc)
return rc;
@@ -631,8 +634,8 @@ static int tpm_continue_selftest(struct tpm_chip *chip)
struct tpm_cmd_t cmd;
cmd.header.in = continue_selftest_header;
- rc = transmit_cmd(chip, &cmd, CONTINUE_SELFTEST_RESULT_SIZE,
- "continue selftest");
+ rc = tpm_transmit_cmd(chip, &cmd, CONTINUE_SELFTEST_RESULT_SIZE,
+ "continue selftest");
return rc;
}
@@ -672,8 +675,8 @@ int tpm_pcr_read_dev(struct tpm_chip *chip, int pcr_idx, u8 *res_buf)
cmd.header.in = pcrread_header;
cmd.params.pcrread_in.pcr_idx = cpu_to_be32(pcr_idx);
- rc = transmit_cmd(chip, &cmd, READ_PCR_RESULT_SIZE,
- "attempting to read a pcr value");
+ rc = tpm_transmit_cmd(chip, &cmd, READ_PCR_RESULT_SIZE,
+ "attempting to read a pcr value");
if (rc == 0)
memcpy(res_buf, cmd.params.pcrread_out.pcr_result,
@@ -737,8 +740,8 @@ int tpm_pcr_extend(u32 chip_num, int pcr_idx, const u8 *hash)
cmd.header.in = pcrextend_header;
cmd.params.pcrextend_in.pcr_idx = cpu_to_be32(pcr_idx);
memcpy(cmd.params.pcrextend_in.hash, hash, TPM_DIGEST_SIZE);
- rc = transmit_cmd(chip, &cmd, EXTEND_PCR_RESULT_SIZE,
- "attempting extend a PCR value");
+ rc = tpm_transmit_cmd(chip, &cmd, EXTEND_PCR_RESULT_SIZE,
+ "attempting extend a PCR value");
tpm_chip_put(chip);
return rc;
@@ -817,7 +820,7 @@ int tpm_send(u32 chip_num, void *cmd, size_t buflen)
if (chip == NULL)
return -ENODEV;
- rc = transmit_cmd(chip, cmd, buflen, "attempting tpm_cmd");
+ rc = tpm_transmit_cmd(chip, cmd, buflen, "attempting tpm_cmd");
tpm_chip_put(chip);
return rc;
@@ -938,14 +941,14 @@ int tpm_pm_suspend(struct device *dev)
cmd.params.pcrextend_in.pcr_idx = cpu_to_be32(tpm_suspend_pcr);
memcpy(cmd.params.pcrextend_in.hash, dummy_hash,
TPM_DIGEST_SIZE);
- rc = transmit_cmd(chip, &cmd, EXTEND_PCR_RESULT_SIZE,
- "extending dummy pcr before suspend");
+ rc = tpm_transmit_cmd(chip, &cmd, EXTEND_PCR_RESULT_SIZE,
+ "extending dummy pcr before suspend");
}
/* now do the actual savestate */
for (try = 0; try < TPM_RETRY; try++) {
cmd.header.in = savestate_header;
- rc = transmit_cmd(chip, &cmd, SAVESTATE_RESULT_SIZE, NULL);
+ rc = tpm_transmit_cmd(chip, &cmd, SAVESTATE_RESULT_SIZE, NULL);
/*
* If the TPM indicates that it is too busy to respond to
@@ -1022,7 +1025,7 @@ int tpm_get_random(u32 chip_num, u8 *out, size_t max)
tpm_cmd.header.in = tpm_getrandom_header;
tpm_cmd.params.getrandom_in.num_bytes = cpu_to_be32(num_bytes);
- err = transmit_cmd(chip, &tpm_cmd,
+ err = tpm_transmit_cmd(chip, &tpm_cmd,
TPM_GETRANDOM_RESULT_SIZE + num_bytes,
"attempting get random");
if (err)
diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
index 01730a2..8ecb052 100644
--- a/drivers/char/tpm/tpm-sysfs.c
+++ b/drivers/char/tpm/tpm-sysfs.c
@@ -20,25 +20,6 @@
#include <linux/device.h>
#include "tpm.h"
-/* XXX for now this helper is duplicated in tpm-interface.c */
-static ssize_t transmit_cmd(struct tpm_chip *chip, struct tpm_cmd_t *cmd,
- int len, const char *desc)
-{
- int err;
-
- len = tpm_transmit(chip, (u8 *) cmd, len);
- if (len < 0)
- return len;
- else if (len < TPM_HEADER_SIZE)
- return -EFAULT;
-
- err = be32_to_cpu(cmd->header.out.return_code);
- if (err != 0 && desc)
- dev_err(chip->dev, "A TPM error (%d) occurred %s\n", err, desc);
-
- return err;
-}
-
#define READ_PUBEK_RESULT_SIZE 314
#define TPM_ORD_READPUBEK cpu_to_be32(124)
static struct tpm_input_header tpm_readpubek_header = {
@@ -58,8 +39,8 @@ static ssize_t pubek_show(struct device *dev, struct device_attribute *attr,
struct tpm_chip *chip = dev_get_drvdata(dev);
tpm_cmd.header.in = tpm_readpubek_header;
- err = transmit_cmd(chip, &tpm_cmd, READ_PUBEK_RESULT_SIZE,
- "attempting to read the PUBEK");
+ err = tpm_transmit_cmd(chip, &tpm_cmd, READ_PUBEK_RESULT_SIZE,
+ "attempting to read the PUBEK");
if (err)
goto out;
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index e4d0888..12326e1 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -314,9 +314,8 @@ struct tpm_cmd_t {
} __packed;
ssize_t tpm_getcap(struct device *, __be32, cap_t *, const char *);
-
-ssize_t tpm_transmit(struct tpm_chip *chip, const char *buf,
- size_t bufsiz);
+ssize_t tpm_transmit_cmd(struct tpm_chip *chip, void *cmd, int len,
+ const char *desc);
extern int tpm_get_timeouts(struct tpm_chip *);
extern void tpm_gen_interrupt(struct tpm_chip *);
extern int tpm_do_selftest(struct tpm_chip *);
--
2.1.0
^ permalink raw reply related
* [PATCH v1 0/3] tpm: prepare for TPM2
From: Jarkko Sakkinen @ 2014-10-22 16:23 UTC (permalink / raw)
To: Peter Huewe, Ashley Lai, Marcel Selhorst
Cc: tpmdd-devel, linux-kernel, linux-api, josh.triplett,
christophe.ricard, jason.gunthorpe, Jarkko Sakkinen
This patch set fixes two race conditions in the TPM subsystem:
* Two-phase initialization for struct tpm_chip so that device can
initialize fully initialize before exposing itself to the user
space. Also, in future TPM2 devices must be flagged before they
can be registered.
* Machines where there are two TPM devices exposed by ACPI have
a racy lookup for the PPI interface. This patch set fixes this
issues
In addition, transmit_cmd() is renamed as tpm_transmit_cmd() and
made opaque so that separate command structure can be introduced
for TPM2.
Comments about v1:
* I think this could be pulled to 3.18 because this clearly fixes
bugs in the current implementation.
Jarkko Sakkinen (3):
tpm: merge duplicate transmit_cmd() functions
tpm: two-phase chip management functions
tpm: fix multiple race conditions in tpm_ppi.c
drivers/char/tpm/Makefile | 2 +-
drivers/char/tpm/tpm-chip.c | 190 ++++++++++++++++++++++++++++++++++
drivers/char/tpm/tpm-dev.c | 4 +-
drivers/char/tpm/tpm-interface.c | 201 ++++++------------------------------
drivers/char/tpm/tpm-sysfs.c | 23 +----
drivers/char/tpm/tpm.h | 32 +++---
drivers/char/tpm/tpm_atmel.c | 11 +-
drivers/char/tpm/tpm_i2c_atmel.c | 33 ++----
drivers/char/tpm/tpm_i2c_infineon.c | 37 ++-----
drivers/char/tpm/tpm_i2c_nuvoton.c | 44 +++-----
drivers/char/tpm/tpm_i2c_stm_st33.c | 22 ++--
drivers/char/tpm/tpm_ibmvtpm.c | 17 ++-
drivers/char/tpm/tpm_infineon.c | 13 ++-
drivers/char/tpm/tpm_nsc.c | 11 +-
drivers/char/tpm/tpm_ppi.c | 137 ++++++++++++++----------
drivers/char/tpm/tpm_tis.c | 94 ++++++++---------
drivers/char/tpm/xen-tpmfront.c | 14 +--
17 files changed, 458 insertions(+), 427 deletions(-)
create mode 100644 drivers/char/tpm/tpm-chip.c
--
2.1.0
^ permalink raw reply
* [PATCH RFC v2 16/16] virtio_blk: fix types for in memory structures
From: Michael S. Tsirkin @ 2014-10-22 15:51 UTC (permalink / raw)
To: linux-kernel; +Cc: Rusty Russell, virtualization, linux-api
In-Reply-To: <1413992894-22976-1-git-send-email-mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
include/uapi/linux/virtio_blk.h | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/include/uapi/linux/virtio_blk.h b/include/uapi/linux/virtio_blk.h
index 9ad67b2..247c8ba 100644
--- a/include/uapi/linux/virtio_blk.h
+++ b/include/uapi/linux/virtio_blk.h
@@ -28,6 +28,7 @@
#include <linux/types.h>
#include <linux/virtio_ids.h>
#include <linux/virtio_config.h>
+#include <linux/virtio_types.h>
/* Feature bits */
#define VIRTIO_BLK_F_BARRIER 0 /* Does host support barriers? */
@@ -114,18 +115,18 @@ struct virtio_blk_config {
/* This is the first element of the read scatter-gather list. */
struct virtio_blk_outhdr {
/* VIRTIO_BLK_T* */
- __u32 type;
+ __virtio32 type;
/* io priority. */
- __u32 ioprio;
+ __virtio32 ioprio;
/* Sector (ie. 512 byte offset) */
- __u64 sector;
+ __virtio64 sector;
};
struct virtio_scsi_inhdr {
- __u32 errors;
- __u32 data_len;
- __u32 sense_len;
- __u32 residual;
+ __virtio32 errors;
+ __virtio32 data_len;
+ __virtio32 sense_len;
+ __virtio32 residual;
};
/* And this is the final byte of the write scatter-gather list. */
--
MST
^ permalink raw reply related
* [PATCH RFC v2 15/16] virtio_net: fix types for in memory structures
From: Michael S. Tsirkin @ 2014-10-22 15:51 UTC (permalink / raw)
To: linux-kernel; +Cc: linux-api, virtualization
In-Reply-To: <1413992894-22976-1-git-send-email-mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
include/uapi/linux/virtio_net.h | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h
index 172a7f0..b5f1677 100644
--- a/include/uapi/linux/virtio_net.h
+++ b/include/uapi/linux/virtio_net.h
@@ -28,6 +28,7 @@
#include <linux/types.h>
#include <linux/virtio_ids.h>
#include <linux/virtio_config.h>
+#include <linux/virtio_types.h>
#include <linux/if_ether.h>
/* The feature bitmap for virtio net */
@@ -84,17 +85,17 @@ struct virtio_net_hdr {
#define VIRTIO_NET_HDR_GSO_TCPV6 4 // GSO frame, IPv6 TCP
#define VIRTIO_NET_HDR_GSO_ECN 0x80 // TCP has ECN set
__u8 gso_type;
- __u16 hdr_len; /* Ethernet + IP + tcp/udp hdrs */
- __u16 gso_size; /* Bytes to append to hdr_len per frame */
- __u16 csum_start; /* Position to start checksumming from */
- __u16 csum_offset; /* Offset after that to place checksum */
+ __virtio16 hdr_len; /* Ethernet + IP + tcp/udp hdrs */
+ __virtio16 gso_size; /* Bytes to append to hdr_len per frame */
+ __virtio16 csum_start; /* Position to start checksumming from */
+ __virtio16 csum_offset; /* Offset after that to place checksum */
};
/* This is the version of the header to use when the MRG_RXBUF
* feature has been negotiated. */
struct virtio_net_hdr_mrg_rxbuf {
struct virtio_net_hdr hdr;
- __u16 num_buffers; /* Number of merged rx buffers */
+ __virtio16 num_buffers; /* Number of merged rx buffers */
};
/*
@@ -149,7 +150,7 @@ typedef __u8 virtio_net_ctrl_ack;
* VIRTIO_NET_F_CTRL_MAC_ADDR feature is available.
*/
struct virtio_net_ctrl_mac {
- __u32 entries;
+ __virtio32 entries;
__u8 macs[][ETH_ALEN];
} __attribute__((packed));
@@ -193,7 +194,7 @@ struct virtio_net_ctrl_mac {
* specified.
*/
struct virtio_net_ctrl_mq {
- __u16 virtqueue_pairs;
+ __virtio16 virtqueue_pairs;
};
#define VIRTIO_NET_CTRL_MQ 4
--
MST
^ permalink raw reply related
* [PATCH RFC v2 09/16] virtio: set FEATURES_OK
From: Michael S. Tsirkin @ 2014-10-22 15:50 UTC (permalink / raw)
To: linux-kernel; +Cc: linux-api, virtualization
In-Reply-To: <1413992894-22976-1-git-send-email-mst@redhat.com>
set FEATURES_OK as per virtio 1.0 spec
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
include/uapi/linux/virtio_config.h | 2 ++
drivers/virtio/virtio.c | 29 ++++++++++++++++++++++-------
2 files changed, 24 insertions(+), 7 deletions(-)
diff --git a/include/uapi/linux/virtio_config.h b/include/uapi/linux/virtio_config.h
index f3fe33a..a6d0cde 100644
--- a/include/uapi/linux/virtio_config.h
+++ b/include/uapi/linux/virtio_config.h
@@ -38,6 +38,8 @@
#define VIRTIO_CONFIG_S_DRIVER 2
/* Driver has used its parts of the config, and is happy */
#define VIRTIO_CONFIG_S_DRIVER_OK 4
+/* Driver has finished configuring features */
+#define VIRTIO_CONFIG_S_FEATURES_OK 8
/* We've given up on this device. */
#define VIRTIO_CONFIG_S_FAILED 0x80
diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
index d213567..a3df817 100644
--- a/drivers/virtio/virtio.c
+++ b/drivers/virtio/virtio.c
@@ -160,6 +160,7 @@ static int virtio_dev_probe(struct device *_d)
struct virtio_device *dev = dev_to_virtio(_d);
struct virtio_driver *drv = drv_to_virtio(dev->dev.driver);
u64 device_features;
+ unsigned status;
/* We have a driver! */
add_status(dev, VIRTIO_CONFIG_S_DRIVER);
@@ -183,18 +184,32 @@ static int virtio_dev_probe(struct device *_d)
dev->config->finalize_features(dev);
+ if (virtio_has_feature(dev, VIRTIO_F_VERSION_1)) {
+ add_status(dev, VIRTIO_CONFIG_S_FEATURES_OK);
+ status = dev->config->get_status(dev);
+ if (!(status & VIRTIO_CONFIG_S_FEATURES_OK)) {
+ printk(KERN_ERR "virtio: device refuses features: %x\n",
+ status);
+ err = -ENODEV;
+ goto err;
+ }
+ }
+
err = drv->probe(dev);
if (err)
- add_status(dev, VIRTIO_CONFIG_S_FAILED);
- else {
- add_status(dev, VIRTIO_CONFIG_S_DRIVER_OK);
- if (drv->scan)
- drv->scan(dev);
+ goto err;
- virtio_config_enable(dev);
- }
+ add_status(dev, VIRTIO_CONFIG_S_DRIVER_OK);
+ if (drv->scan)
+ drv->scan(dev);
+
+ virtio_config_enable(dev);
+ return 0;
+err:
+ add_status(dev, VIRTIO_CONFIG_S_FAILED);
return err;
+
}
static int virtio_dev_remove(struct device *_d)
--
MST
^ permalink raw reply related
* [PATCH RFC v2 05/16] virtio: add virtio 1.0 feature bit
From: Michael S. Tsirkin @ 2014-10-22 15:50 UTC (permalink / raw)
To: linux-kernel; +Cc: linux-api, virtualization
In-Reply-To: <1413992894-22976-1-git-send-email-mst@redhat.com>
Based on original patches by Rusty Russell, Thomas Huth
and Cornelia Huck.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
include/uapi/linux/virtio_config.h | 7 +++++--
drivers/virtio/virtio_ring.c | 2 ++
2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/include/uapi/linux/virtio_config.h b/include/uapi/linux/virtio_config.h
index 3ce768c..f3fe33a 100644
--- a/include/uapi/linux/virtio_config.h
+++ b/include/uapi/linux/virtio_config.h
@@ -41,11 +41,11 @@
/* We've given up on this device. */
#define VIRTIO_CONFIG_S_FAILED 0x80
-/* Some virtio feature bits (currently bits 28 through 31) are reserved for the
+/* Some virtio feature bits (currently bits 28 through 32) are reserved for the
* transport being used (eg. virtio_ring), the rest are per-device feature
* bits. */
#define VIRTIO_TRANSPORT_F_START 28
-#define VIRTIO_TRANSPORT_F_END 32
+#define VIRTIO_TRANSPORT_F_END 33
/* Do we get callbacks when the ring is completely used, even if we've
* suppressed them? */
@@ -54,4 +54,7 @@
/* Can the device handle any descriptor layout? */
#define VIRTIO_F_ANY_LAYOUT 27
+/* v1.0 compliant. */
+#define VIRTIO_F_VERSION_1 32
+
#endif /* _UAPI_LINUX_VIRTIO_CONFIG_H */
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index b311fa7..9f5dfe3 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -780,6 +780,8 @@ void vring_transport_features(struct virtio_device *vdev)
break;
case VIRTIO_RING_F_EVENT_IDX:
break;
+ case VIRTIO_F_VERSION_1:
+ break;
default:
/* We don't understand this bit. */
vdev->features &= ~(1ULL << i);
--
MST
^ permalink raw reply related
* [PATCH RFC v2 01/16] virtio: memory access APIs
From: Michael S. Tsirkin @ 2014-10-22 15:50 UTC (permalink / raw)
To: linux-kernel
Cc: Bjarke Istrup Pedersen, Anup Patel, Greg Kroah-Hartman,
David Drysdale, Lad, Prabhakar, Geert Uytterhoeven, linux-api,
Alexei Starovoitov, virtualization, David S. Miller,
=?UTF-8?q?Piotr=20Kr=C3=B3l?=, Mauro Carvalho Chehab
In-Reply-To: <1413992894-22976-1-git-send-email-mst@redhat.com>
virtio 1.0 makes all memory structures LE, so
we need APIs to conditionally do a byteswap on BE
architectures.
To make it easier to check code statically,
add virtio specific types for multi-byte integers
in memory.
Add low level wrappers that do a byteswap conditionally, these will be
useful e.g. for vhost. Add high level wrappers that will (in the
future) query device endian-ness and act accordingly.
At the moment, stub them out and assume native endian-ness everywhere.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
include/linux/virtio_config.h | 16 +++++++++++++
include/uapi/linux/virtio_ring.h | 49 ++++++++++++++++++++--------------------
include/uapi/linux/Kbuild | 1 +
3 files changed, 42 insertions(+), 24 deletions(-)
diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
index 7f4ef66..d38d3c2 100644
--- a/include/linux/virtio_config.h
+++ b/include/linux/virtio_config.h
@@ -4,6 +4,7 @@
#include <linux/err.h>
#include <linux/bug.h>
#include <linux/virtio.h>
+#include <linux/virtio_byteorder.h>
#include <uapi/linux/virtio_config.h>
/**
@@ -152,6 +153,21 @@ int virtqueue_set_affinity(struct virtqueue *vq, int cpu)
return 0;
}
+/* Memory accessors */
+#define DEFINE_VIRTIO_XX_TO_CPU(bits) \
+static inline u##bits virtio##bits##_to_cpu(struct virtio_device *vdev, __virtio##bits val) \
+{ \
+ return __virtio##bits##_to_cpu(false, val); \
+} \
+static inline __virtio##bits cpu_to_virtio##bits(struct virtio_device *vdev, u##bits val) \
+{ \
+ return __cpu_to_virtio##bits(false, val); \
+}
+
+DEFINE_VIRTIO_XX_TO_CPU(16)
+DEFINE_VIRTIO_XX_TO_CPU(32)
+DEFINE_VIRTIO_XX_TO_CPU(64)
+
/* Config space accessors. */
#define virtio_cread(vdev, structname, member, ptr) \
do { \
diff --git a/include/uapi/linux/virtio_ring.h b/include/uapi/linux/virtio_ring.h
index a99f9b7..6c00632 100644
--- a/include/uapi/linux/virtio_ring.h
+++ b/include/uapi/linux/virtio_ring.h
@@ -32,6 +32,7 @@
*
* Copyright Rusty Russell IBM Corporation 2007. */
#include <linux/types.h>
+#include <linux/virtio_types.h>
/* This marks a buffer as continuing via the next field. */
#define VRING_DESC_F_NEXT 1
@@ -61,32 +62,32 @@
/* Virtio ring descriptors: 16 bytes. These can chain together via "next". */
struct vring_desc {
/* Address (guest-physical). */
- __u64 addr;
+ __virtio64 addr;
/* Length. */
- __u32 len;
+ __virtio32 len;
/* The flags as indicated above. */
- __u16 flags;
+ __virtio16 flags;
/* We chain unused descriptors via this, too */
- __u16 next;
+ __virtio16 next;
};
struct vring_avail {
- __u16 flags;
- __u16 idx;
- __u16 ring[];
+ __virtio16 flags;
+ __virtio16 idx;
+ __virtio16 ring[];
};
/* u32 is used here for ids for padding reasons. */
struct vring_used_elem {
/* Index of start of used descriptor chain. */
- __u32 id;
+ __virtio32 id;
/* Total length of the descriptor chain which was used (written to) */
- __u32 len;
+ __virtio32 len;
};
struct vring_used {
- __u16 flags;
- __u16 idx;
+ __virtio16 flags;
+ __virtio16 idx;
struct vring_used_elem ring[];
};
@@ -109,25 +110,25 @@ struct vring {
* struct vring_desc desc[num];
*
* // A ring of available descriptor heads with free-running index.
- * __u16 avail_flags;
- * __u16 avail_idx;
- * __u16 available[num];
- * __u16 used_event_idx;
+ * __virtio16 avail_flags;
+ * __virtio16 avail_idx;
+ * __virtio16 available[num];
+ * __virtio16 used_event_idx;
*
* // Padding to the next align boundary.
* char pad[];
*
* // A ring of used descriptor heads with free-running index.
- * __u16 used_flags;
- * __u16 used_idx;
+ * __virtio16 used_flags;
+ * __virtio16 used_idx;
* struct vring_used_elem used[num];
- * __u16 avail_event_idx;
+ * __virtio16 avail_event_idx;
* };
*/
/* We publish the used event index at the end of the available ring, and vice
* versa. They are at the end for backwards compatibility. */
#define vring_used_event(vr) ((vr)->avail->ring[(vr)->num])
-#define vring_avail_event(vr) (*(__u16 *)&(vr)->used->ring[(vr)->num])
+#define vring_avail_event(vr) (*(__virtio16 *)&(vr)->used->ring[(vr)->num])
static inline void vring_init(struct vring *vr, unsigned int num, void *p,
unsigned long align)
@@ -135,29 +136,29 @@ static inline void vring_init(struct vring *vr, unsigned int num, void *p,
vr->num = num;
vr->desc = p;
vr->avail = p + num*sizeof(struct vring_desc);
- vr->used = (void *)(((unsigned long)&vr->avail->ring[num] + sizeof(__u16)
+ vr->used = (void *)(((unsigned long)&vr->avail->ring[num] + sizeof(__virtio16)
+ align-1) & ~(align - 1));
}
static inline unsigned vring_size(unsigned int num, unsigned long align)
{
- return ((sizeof(struct vring_desc) * num + sizeof(__u16) * (3 + num)
+ return ((sizeof(struct vring_desc) * num + sizeof(__virtio16) * (3 + num)
+ align - 1) & ~(align - 1))
- + sizeof(__u16) * 3 + sizeof(struct vring_used_elem) * num;
+ + sizeof(__virtio16) * 3 + sizeof(struct vring_used_elem) * num;
}
/* The following is used with USED_EVENT_IDX and AVAIL_EVENT_IDX */
/* Assuming a given event_idx value from the other size, if
* we have just incremented index from old to new_idx,
* should we trigger an event? */
-static inline int vring_need_event(__u16 event_idx, __u16 new_idx, __u16 old)
+static inline int vring_need_event(__virtio16 event_idx, __virtio16 new_idx, __virtio16 old)
{
/* Note: Xen has similar logic for notification hold-off
* in include/xen/interface/io/ring.h with req_event and req_prod
* corresponding to event_idx + 1 and new_idx respectively.
* Note also that req_event and req_prod in Xen start at 1,
* event indexes in virtio start at 0. */
- return (__u16)(new_idx - event_idx - 1) < (__u16)(new_idx - old);
+ return (__virtio16)(new_idx - event_idx - 1) < (__virtio16)(new_idx - old);
}
#endif /* _UAPI_LINUX_VIRTIO_RING_H */
diff --git a/include/uapi/linux/Kbuild b/include/uapi/linux/Kbuild
index 6cad974..39c161a 100644
--- a/include/uapi/linux/Kbuild
+++ b/include/uapi/linux/Kbuild
@@ -419,6 +419,7 @@ header-y += virtio_blk.h
header-y += virtio_config.h
header-y += virtio_console.h
header-y += virtio_ids.h
+header-y += virtio_types.h
header-y += virtio_net.h
header-y += virtio_pci.h
header-y += virtio_ring.h
--
MST
^ permalink raw reply related
* Re: [PATCH v8 02/18] vfio: platform: probe to devices on the platform bus
From: Antonios Motakis @ 2014-10-22 13:53 UTC (permalink / raw)
To: Eric Auger
Cc: Alex Williamson, kvm-arm, Linux IOMMU, Will Deacon,
VirtualOpenSystems Technical Team, Christoffer Dall, Kim Phillips,
Marc Zyngier, open list,
open list:VFIO DRIVER <kvm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>, open list:ABI/API
In-Reply-To: <54468BDA.6050606-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
On Tue, Oct 21, 2014 at 6:37 PM, Eric Auger <eric.auger-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> On 10/21/2014 06:17 PM, Alex Williamson wrote:
>> On Mon, 2014-10-13 at 15:10 +0200, Antonios Motakis wrote:
>>> Driver to bind to Linux platform devices, and callbacks to discover their
>>> resources to be used by the main VFIO PLATFORM code.
>>>
>>> Signed-off-by: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>
>>> ---
>>> drivers/vfio/platform/vfio_platform.c | 107 ++++++++++++++++++++++++++++++++++
>>> include/uapi/linux/vfio.h | 1 +
>>> 2 files changed, 108 insertions(+)
>>> create mode 100644 drivers/vfio/platform/vfio_platform.c
>>>
>>> diff --git a/drivers/vfio/platform/vfio_platform.c b/drivers/vfio/platform/vfio_platform.c
>>> new file mode 100644
>>> index 0000000..baeb7da
>>> --- /dev/null
>>> +++ b/drivers/vfio/platform/vfio_platform.c
>>> @@ -0,0 +1,107 @@
>>> +/*
>>> + * Copyright (C) 2013 - Virtual Open Systems
>>> + * Author: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>
>>> + *
>>> + * This program is free software; you can redistribute it and/or modify
>>> + * it under the terms of the GNU General Public License, version 2, as
>>> + * published by the Free Software Foundation.
>>> + *
>>> + * This program is distributed in the hope that it will be useful,
>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>>> + * GNU General Public License for more details.
>>> + */
>>> +
>>> +#include <linux/device.h>
>>> +#include <linux/eventfd.h>
>>> +#include <linux/interrupt.h>
>>> +#include <linux/iommu.h>
>>> +#include <linux/module.h>
>>> +#include <linux/mutex.h>
>>> +#include <linux/notifier.h>
>>> +#include <linux/pm_runtime.h>
>>> +#include <linux/slab.h>
>>> +#include <linux/types.h>
>>> +#include <linux/uaccess.h>
>>> +#include <linux/vfio.h>
>>> +#include <linux/io.h>
>>> +#include <linux/platform_device.h>
>>> +#include <linux/irq.h>
>>> +
>>> +#include "vfio_platform_private.h"
>>> +
>>> +#define DRIVER_VERSION "0.8"
>>> +#define DRIVER_AUTHOR "Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>"
>>> +#define DRIVER_DESC "VFIO for platform devices - User Level meta-driver"
>>> +
>>> +/* probing devices from the linux platform bus */
>>> +
>>> +static struct resource *get_platform_resource(struct vfio_platform_device *vdev,
>>> + int num)
>>> +{
>>> + struct platform_device *dev = (struct platform_device *) vdev->opaque;
>>> + int i;
>>> +
>>> + for (i = 0; i < dev->num_resources; i++) {
>>> + struct resource *r = &dev->resource[i];
>>> +
>>> + if (resource_type(r) & (IORESOURCE_MEM|IORESOURCE_IO)) {
>>> + num--;
>>> +
>>> + if (!num)
>>> + return r;
>>
>> Has this been tested? What happens when we call this with num = 0?
> Yep. I confirm I enter that case with my xgmac where the IORESOURCE_MEM
> is the 1st resource. I Just ended to the same cause ;-)
Right, num-- should be after the check, or we miss one region.
>
> Best Regards
>
> Eric
>>
>>> + }
>>> + }
>>> + return NULL;
>>> +}
>>> +
>>> +static int get_platform_irq(struct vfio_platform_device *vdev, int i)
>>> +{
>>> + struct platform_device *pdev = (struct platform_device *) vdev->opaque;
>>> +
>>> + return platform_get_irq(pdev, i);
>>> +}
>>> +
>>> +
>>> +static int vfio_platform_probe(struct platform_device *pdev)
>>> +{
>>> + struct vfio_platform_device *vdev;
>>> + int ret;
>>> +
>>> + vdev = kzalloc(sizeof(*vdev), GFP_KERNEL);
>>> + if (!vdev)
>>> + return -ENOMEM;
>>> +
>>> + vdev->opaque = (void *) pdev;
>>> + vdev->name = pdev->name;
>>> + vdev->flags = VFIO_DEVICE_FLAGS_PLATFORM;
>>> + vdev->get_resource = get_platform_resource;
>>> + vdev->get_irq = get_platform_irq;
>>> +
>>> + ret = vfio_platform_probe_common(vdev, &pdev->dev);
>>> + if (ret)
>>> + kfree(vdev);
>>> +
>>> + return ret;
>>> +}
>>> +
>>> +static int vfio_platform_remove(struct platform_device *pdev)
>>> +{
>>> + return vfio_platform_remove_common(&pdev->dev);
>>> +}
>>> +
>>> +static struct platform_driver vfio_platform_driver = {
>>> + .probe = vfio_platform_probe,
>>> + .remove = vfio_platform_remove,
>>> + .driver = {
>>> + .name = "vfio-platform",
>>> + .owner = THIS_MODULE,
>>> + },
>>> +};
>>> +
>>> +module_platform_driver(vfio_platform_driver);
>>> +
>>> +MODULE_VERSION(DRIVER_VERSION);
>>> +MODULE_LICENSE("GPL v2");
>>> +MODULE_AUTHOR(DRIVER_AUTHOR);
>>> +MODULE_DESCRIPTION(DRIVER_DESC);
>>> diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
>>> index 111b5e8..aca6d3e 100644
>>> --- a/include/uapi/linux/vfio.h
>>> +++ b/include/uapi/linux/vfio.h
>>> @@ -158,6 +158,7 @@ struct vfio_device_info {
>>> __u32 flags;
>>> #define VFIO_DEVICE_FLAGS_RESET (1 << 0) /* Device supports reset */
>>> #define VFIO_DEVICE_FLAGS_PCI (1 << 1) /* vfio-pci device */
>>> +#define VFIO_DEVICE_FLAGS_PLATFORM (1 << 2) /* vfio-platform device */
>>> __u32 num_regions; /* Max region index + 1 */
>>> __u32 num_irqs; /* Max IRQ index + 1 */
>>> };
>>
>>
>>
>
--
Antonios Motakis
Virtual Open Systems
^ permalink raw reply
* Re: [PATCH] [media] Fix smatch warning: unknown field name in initializer
From: Hans Verkuil @ 2014-10-22 12:39 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Linux Media Mailing List
Cc: Mauro Carvalho Chehab, linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <a51536f693c64c92151b4a7f530e97fa9b0d867d.1411563071.git.mchehab-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
I sadly missed this patch and it is merged now.
But:
Nacked-by: Hans Verkuil <hans.verkuil-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>
This sparse warnings are due to a sparse bug that has been fixed in the sparse
git repo. It's not included in sparse-0.5.0, the fix came in later.
The #define that you kept it the version that does not comply with the C11 spec, so
this is likely to fail with non-gcc compilers (gcc apparently kept support for the
old pre-4.6 syntax).
Regards,
Hans
On 09/24/2014 02:51 PM, Mauro Carvalho Chehab wrote:
> This is detected with:
> gcc-4.8.3-7.fc20.x86_64
>
> Smatch, up to this patch:
> commit de462ba2c79d9347368c887ed93113e7818a7b07
> Author: Dan Carpenter <dan.carpenter-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> Date: Wed Sep 17 13:31:16 2014 +0300
>
> drivers/media/v4l2-core/v4l2-dv-timings.c:34:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:35:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:36:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:37:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:38:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:39:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:40:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:41:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:42:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:43:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:44:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:45:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:46:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:47:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:48:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:49:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:50:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:51:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:52:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:53:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:54:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:55:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:56:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:57:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:58:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:59:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:60:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:61:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:62:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:63:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:64:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:65:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:66:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:67:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:68:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:69:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:70:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:71:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:72:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:73:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:74:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:75:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:76:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:77:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:78:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:79:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:80:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:81:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:82:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:83:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:84:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:85:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:86:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:87:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:88:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:89:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:90:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:91:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:92:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:93:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:94:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:95:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:96:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:97:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:98:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:99:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:100:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:101:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:102:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:103:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:104:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:105:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:106:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:107:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:108:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:109:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:110:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:111:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:112:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:113:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:114:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:115:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:116:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:117:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:118:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:119:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:120:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:121:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:122:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:123:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:124:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:125:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:126:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:127:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:128:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:129:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:130:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:131:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:132:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:133:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:134:9: error: unknown field name in initializer
> drivers/media/v4l2-core/v4l2-dv-timings.c:135:9: error: too many errors
> drivers/media/usb/hdpvr/hdpvr-video.c:42:9: error: unknown field name in initializer
> drivers/media/usb/hdpvr/hdpvr-video.c:43:9: error: unknown field name in initializer
> drivers/media/usb/hdpvr/hdpvr-video.c:44:9: error: unknown field name in initializer
> drivers/media/usb/hdpvr/hdpvr-video.c:45:9: error: unknown field name in initializer
> drivers/media/usb/hdpvr/hdpvr-video.c:46:9: error: unknown field name in initializer
> drivers/media/usb/hdpvr/hdpvr-video.c:47:9: error: unknown field name in initializer
> drivers/media/usb/hdpvr/hdpvr-video.c:48:9: error: unknown field name in initializer
> drivers/media/usb/hdpvr/hdpvr-video.c:49:9: error: unknown field name in initializer
> drivers/media/platform/s5p-tv/hdmi_drv.c:484:18: error: unknown field name in initializer
> drivers/media/platform/s5p-tv/hdmi_drv.c:485:18: error: unknown field name in initializer
> drivers/media/platform/s5p-tv/hdmi_drv.c:486:18: error: unknown field name in initializer
> drivers/media/platform/s5p-tv/hdmi_drv.c:487:18: error: unknown field name in initializer
> drivers/media/platform/s5p-tv/hdmi_drv.c:488:18: error: unknown field name in initializer
> drivers/media/platform/s5p-tv/hdmi_drv.c:489:18: error: unknown field name in initializer
> drivers/media/platform/s5p-tv/hdmi_drv.c:490:18: error: unknown field name in initializer
> drivers/media/platform/s5p-tv/hdmi_drv.c:491:18: error: unknown field name in initializer
> drivers/media/platform/s5p-tv/hdmi_drv.c:492:18: error: unknown field name in initializer
> drivers/media/platform/s5p-tv/hdmi_drv.c:493:18: error: unknown field name in initializer
>
> Signed-off-by: Mauro Carvalho Chehab <mchehab-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
>
> diff --git a/include/uapi/linux/v4l2-dv-timings.h b/include/uapi/linux/v4l2-dv-timings.h
> index 6c8f159e416e..6a0764c89fcb 100644
> --- a/include/uapi/linux/v4l2-dv-timings.h
> +++ b/include/uapi/linux/v4l2-dv-timings.h
> @@ -21,17 +21,8 @@
> #ifndef _V4L2_DV_TIMINGS_H
> #define _V4L2_DV_TIMINGS_H
>
> -#if __GNUC__ < 4 || (__GNUC__ == 4 && (__GNUC_MINOR__ < 6))
> -/* Sadly gcc versions older than 4.6 have a bug in how they initialize
> - anonymous unions where they require additional curly brackets.
> - This violates the C1x standard. This workaround adds the curly brackets
> - if needed. */
> #define V4L2_INIT_BT_TIMINGS(_width, args...) \
> { .bt = { _width , ## args } }
> -#else
> -#define V4L2_INIT_BT_TIMINGS(_width, args...) \
> - .bt = { _width , ## args }
> -#endif
>
> /* CEA-861-E timings (i.e. standard HDTV timings) */
>
>
^ permalink raw reply
* Re: [PATCHv4 RESEND 0/3] syscalls,x86: Add execveat() system call
From: Christoph Hellwig @ 2014-10-22 11:54 UTC (permalink / raw)
To: Andy Lutomirski
Cc: David Drysdale, Eric W. Biederman, Alexander Viro, Meredydd Luff,
linux-kernel@vger.kernel.org, Thomas Gleixner, Ingo Molnar,
H. Peter Anvin, Andrew Morton, Kees Cook, Arnd Bergmann, X86 ML,
linux-arch, Linux API, Rich Felker
In-Reply-To: <20141022115405.GA8593@infradead.org>
On Wed, Oct 22, 2014 at 04:54:05AM -0700, Christoph Hellwig wrote:
> [adding Rich Felker to the Cc list, who has been very interested in a
> O_SEARCH implementation for which this would be an important building
> block]
s/O_SEARCH/O_EXEC/, sorry.
^ permalink raw reply
* Re: [PATCHv4 RESEND 0/3] syscalls,x86: Add execveat() system call
From: Christoph Hellwig @ 2014-10-22 11:54 UTC (permalink / raw)
To: Andy Lutomirski
Cc: David Drysdale, Eric W. Biederman, Alexander Viro, Meredydd Luff,
linux-kernel@vger.kernel.org, Thomas Gleixner, Ingo Molnar,
H. Peter Anvin, Andrew Morton, Kees Cook, Arnd Bergmann, X86 ML,
linux-arch, Linux API, Rich Felker
In-Reply-To: <CALCETrVraoD+r4zxBoGd+BV5P275AXcRV_R00SSr8fjQzRHnUg@mail.gmail.com>
[adding Rich Felker to the Cc list, who has been very interested in a
O_SEARCH implementation for which this would be an important building
block]
On Fri, Oct 17, 2014 at 02:45:03PM -0700, Andy Lutomirski wrote:
> [Added Eric Biederman, since I think your tree might be a reasonable
> route forward for these patches.]
>
> On Thu, Jun 5, 2014 at 6:40 AM, David Drysdale <drysdale@google.com> wrote:
> > Resending, adding cc:linux-api.
> >
> > Also, it may help to add a little more background -- this patch is
> > needed as a (small) part of implementing Capsicum in the Linux kernel.
> >
> > Capsicum is a security framework that has been present in FreeBSD since
> > version 9.0 (Jan 2012), and is based on concepts from object-capability
> > security [1].
> >
> > One of the features of Capsicum is capability mode, which locks down
> > access to global namespaces such as the filesystem hierarchy. In
> > capability mode, /proc is thus inaccessible and so fexecve(3) doesn't
> > work -- hence the need for a kernel-space
>
> I just found myself wanting this syscall for another reason: injecting
> programs into sandboxes or otherwise heavily locked-down namespaces.
>
> For example, I want to be able to reliably do something like nsenter
> --namespace-flags-here toybox sh. Toybox's shell is unusual in that
> it is more or less fully functional, so this should Just Work (tm),
> except that the toybox binary might not exist in the namespace being
> entered. If execveat were available, I could rig nsenter or a similar
> tool to open it with O_CLOEXEC, enter the namespace, and then call
> execveat.
>
> Is there any reason that these patches can't be merged more or less as
> is for 3.19?
>
> --Andy
>
> >
> > [1] http://www.cl.cam.ac.uk/research/security/capsicum/papers/2010usenix-security-capsicum-website.pdf
> >
> > ------
> >
> > This patch set adds execveat(2) for x86, and is derived from Meredydd
> > Luff's patch from Sept 2012 (https://lkml.org/lkml/2012/9/11/528).
> >
> > The primary aim of adding an execveat syscall is to allow an
> > implementation of fexecve(3) that does not rely on the /proc
> > filesystem. The current glibc version of fexecve(3) is implemented
> > via /proc, which causes problems in sandboxed or otherwise restricted
> > environments.
> >
> > Given the desire for a /proc-free fexecve() implementation, HPA
> > suggested (https://lkml.org/lkml/2006/7/11/556) that an execveat(2)
> > syscall would be an appropriate generalization.
> >
> > Also, having a new syscall means that it can take a flags argument
> > without back-compatibility concerns. The current implementation just
> > defines the AT_SYMLINK_NOFOLLOW flag, but other flags could be added
> > in future -- for example, flags for new namespaces (as suggested at
> > https://lkml.org/lkml/2006/7/11/474).
> >
> > Related history:
> > - https://lkml.org/lkml/2006/12/27/123 is an example of someone
> > realizing that fexecve() is likely to fail in a chroot environment.
> > - http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=514043 covered
> > documenting the /proc requirement of fexecve(3) in its manpage, to
> > "prevent other people from wasting their time".
> > - https://bugzilla.kernel.org/show_bug.cgi?id=74481 documented that
> > it's not possible to fexecve() a file descriptor for a script with
> > close-on-exec set (which is possible with the implementation here).
> > - https://bugzilla.redhat.com/show_bug.cgi?id=241609 described a
> > problem where a process that did setuid() could not fexecve()
> > because it no longer had access to /proc/self/fd; this has since
> > been fixed.
> >
> >
> > Changes since Meredydd's v3 patch:
> > - Added a selftest.
> > - Added a man page.
> > - Left open_exec() signature untouched to reduce patch impact
> > elsewhere (as suggested by Al Viro).
> > - Filled in bprm->filename with d_path() into a buffer, to avoid use
> > of potentially-ephemeral dentry->d_name.
> > - Patch against v3.14 (455c6fdbd21916).
> >
> >
> > David Drysdale (2):
> > syscalls,x86: implement execveat() system call
> > syscalls,x86: add selftest for execveat(2)
> >
> > arch/x86/ia32/audit.c | 1 +
> > arch/x86/ia32/ia32entry.S | 1 +
> > arch/x86/kernel/audit_64.c | 1 +
> > arch/x86/kernel/entry_64.S | 28 ++++
> > arch/x86/syscalls/syscall_32.tbl | 1 +
> > arch/x86/syscalls/syscall_64.tbl | 2 +
> > arch/x86/um/sys_call_table_64.c | 1 +
> > fs/exec.c | 153 ++++++++++++++++---
> > include/linux/compat.h | 3 +
> > include/linux/sched.h | 4 +
> > include/linux/syscalls.h | 4 +
> > include/uapi/asm-generic/unistd.h | 4 +-
> > kernel/sys_ni.c | 3 +
> > lib/audit.c | 3 +
> > tools/testing/selftests/Makefile | 1 +
> > tools/testing/selftests/exec/.gitignore | 6 +
> > tools/testing/selftests/exec/Makefile | 32 ++++
> > tools/testing/selftests/exec/execveat.c | 251 ++++++++++++++++++++++++++++++++
> > 18 files changed, 476 insertions(+), 23 deletions(-)
> > create mode 100644 tools/testing/selftests/exec/.gitignore
> > create mode 100644 tools/testing/selftests/exec/Makefile
> > create mode 100644 tools/testing/selftests/exec/execveat.c
> >
> > --
> > 1.9.1.423.g4596e3a
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-api" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
>
> --
> Andy Lutomirski
> AMA Capital Management, LLC
> --
> To unsubscribe from this list: send the line "unsubscribe linux-api" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
---end quoted text---
^ permalink raw reply
* [PATCHv5 man-pages 3/3] execveat.2: initial man page for execveat(2)
From: David Drysdale @ 2014-10-22 11:44 UTC (permalink / raw)
To: Eric W. Biederman, Andy Lutomirski, Alexander Viro, Meredydd Luff,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, Andrew Morton,
Kees Cook, Arnd Bergmann, x86-DgEjT+Ai2ygdnm+yROfE0A,
linux-arch-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA, David Drysdale
In-Reply-To: <1413978269-17274-1-git-send-email-drysdale-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
Signed-off-by: David Drysdale <drysdale-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
---
man2/execveat.2 | 144 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 144 insertions(+)
create mode 100644 man2/execveat.2
diff --git a/man2/execveat.2 b/man2/execveat.2
new file mode 100644
index 000000000000..d19571a3eb9d
--- /dev/null
+++ b/man2/execveat.2
@@ -0,0 +1,144 @@
+.\" Copyright (c) 2014 Google, Inc.
+.\"
+.\" %%%LICENSE_START(VERBATIM)
+.\" Permission is granted to make and distribute verbatim copies of this
+.\" manual provided the copyright notice and this permission notice are
+.\" preserved on all copies.
+.\"
+.\" Permission is granted to copy and distribute modified versions of this
+.\" manual under the conditions for verbatim copying, provided that the
+.\" entire resulting derived work is distributed under the terms of a
+.\" permission notice identical to this one.
+.\"
+.\" Since the Linux kernel and libraries are constantly changing, this
+.\" manual page may be incorrect or out-of-date. The author(s) assume no
+.\" responsibility for errors or omissions, or for damages resulting from
+.\" the use of the information contained herein. The author(s) may not
+.\" have taken the same level of care in the production of this manual,
+.\" which is licensed free of charge, as they might when working
+.\" professionally.
+.\"
+.\" Formatted or processed versions of this manual, if unaccompanied by
+.\" the source, must acknowledge the copyright and authors of this work.
+.\" %%%LICENSE_END
+.\"
+.TH EXECVEAT 2 2014-04-02 "Linux" "Linux Programmer's Manual"
+.SH NAME
+execveat \- execute program relative to a directory file descriptor
+.SH SYNOPSIS
+.B #include <unistd.h>
+.sp
+.BI "int execve(int " fd ", const char *" pathname ","
+.br
+.BI " char *const " argv "[], char *const " envp "[],"
+.br
+.BI " int " flags);
+.SH DESCRIPTION
+The
+.BR execveat ()
+system call executes the program pointed to by the combination of \fIfd\fP and \fIpathname\fP.
+The
+.BR execveat ()
+system call operates in exactly the same way as
+.BR execve (2),
+except for the differences described in this manual page.
+
+If the pathname given in
+.I pathname
+is relative, then it is interpreted relative to the directory
+referred to by the file descriptor
+.I fd
+(rather than relative to the current working directory of
+the calling process, as is done by
+.BR execve (2)
+for a relative pathname).
+
+If
+.I pathname
+is relative and
+.I fd
+is the special value
+.BR AT_FDCWD ,
+then
+.I pathname
+is interpreted relative to the current working
+directory of the calling process (like
+.BR execve (2)).
+
+If
+.I pathname
+is absolute, then
+.I fd
+is ignored.
+
+If
+.I pathname
+is an empty string and the
+.BR AT_EMPTY_PATH
+flag is specified, then the file descriptor
+.I fd
+specifies the file to be executed.
+
+.I flags
+can either be 0, or include the following flags:
+.TP
+.BR AT_EMPTY_PATH
+If
+.I pathname
+is an empty string, operate on the file referred to by
+.IR fd
+(which may have been obtained using the
+.BR open (2)
+.B O_PATH
+flag).
+.TP
+.B AT_SYMLINK_NOFOLLOW
+If the file identified by
+.I fd
+and a non-NULL
+.I pathname
+is a symbolic link, then the call fails with the error
+.BR EINVAL .
+.SH "RETURN VALUE"
+On success,
+.BR execveat ()
+does not return. On error \-1 is returned, and
+.I errno
+is set appropriately.
+.SH ERRORS
+The same errors that occur for
+.BR execve (2)
+can also occur for
+.BR execveat ().
+The following additional errors can occur for
+.BR execveat ():
+.TP
+.B EBADF
+.I fd
+is not a valid file descriptor.
+.TP
+.B EINVAL
+Invalid flag specified in
+.IR flags .
+.TP
+.B ENOTDIR
+.I pathname
+is relative and
+.I fd
+is a file descriptor referring to a file other than a directory.
+.SH VERSIONS
+.BR execveat ()
+was added to Linux in kernel 3.???.
+.SH NOTES
+In addition to the reasons explained in
+.BR openat (2),
+the
+.BR execveat ()
+system call is also needed to allow
+.BR fexecve (3)
+to be implemented on systems that do not have the
+.I /proc
+filesystem mounted.
+.SH SEE ALSO
+.BR execve (2),
+.BR fexecve (3)
--
2.1.0.rc2.206.gedb03e5
^ permalink raw reply related
* [PATCHv5 2/3] syscalls,x86: add selftest for execveat(2)
From: David Drysdale @ 2014-10-22 11:44 UTC (permalink / raw)
To: Eric W. Biederman, Andy Lutomirski, Alexander Viro, Meredydd Luff,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, Andrew Morton,
Kees Cook, Arnd Bergmann, x86-DgEjT+Ai2ygdnm+yROfE0A,
linux-arch-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA, David Drysdale
In-Reply-To: <1413978269-17274-1-git-send-email-drysdale-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
Signed-off-by: David Drysdale <drysdale-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
---
tools/testing/selftests/Makefile | 1 +
tools/testing/selftests/exec/.gitignore | 6 +
tools/testing/selftests/exec/Makefile | 22 +++
tools/testing/selftests/exec/execveat.c | 295 ++++++++++++++++++++++++++++++++
4 files changed, 324 insertions(+)
create mode 100644 tools/testing/selftests/exec/.gitignore
create mode 100644 tools/testing/selftests/exec/Makefile
create mode 100644 tools/testing/selftests/exec/execveat.c
diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index 36ff2e4c7b6f..210cf68b3511 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -14,6 +14,7 @@ TARGETS += powerpc
TARGETS += user
TARGETS += sysctl
TARGETS += firmware
+TARGETS += exec
TARGETS_HOTPLUG = cpu-hotplug
TARGETS_HOTPLUG += memory-hotplug
diff --git a/tools/testing/selftests/exec/.gitignore b/tools/testing/selftests/exec/.gitignore
new file mode 100644
index 000000000000..349a786899a7
--- /dev/null
+++ b/tools/testing/selftests/exec/.gitignore
@@ -0,0 +1,6 @@
+subdir*
+script*
+execveat
+execveat.symlink
+execveat.moved
+execveat.ephemeral
diff --git a/tools/testing/selftests/exec/Makefile b/tools/testing/selftests/exec/Makefile
new file mode 100644
index 000000000000..428cb1d2a06a
--- /dev/null
+++ b/tools/testing/selftests/exec/Makefile
@@ -0,0 +1,22 @@
+CC = $(CROSS_COMPILE)gcc
+CFLAGS = -Wall
+BINARIES = execveat
+DEPS = execveat.symlink script subdir
+all: $(BINARIES) $(DEPS)
+
+subdir:
+ mkdir -p $@
+script:
+ echo '#!/bin/sh' > $@
+ echo 'exit $$*' >> $@
+ chmod +x $@
+execveat.symlink: execveat
+ ln -s -f $< $@
+%: %.c
+ $(CC) $(CFLAGS) -o $@ $^
+
+run_tests: all
+ ./execveat
+
+clean:
+ rm -rf $(BINARIES) $(DEPS) subdir.moved execveat.moved
diff --git a/tools/testing/selftests/exec/execveat.c b/tools/testing/selftests/exec/execveat.c
new file mode 100644
index 000000000000..1f2722ed3847
--- /dev/null
+++ b/tools/testing/selftests/exec/execveat.c
@@ -0,0 +1,295 @@
+/*
+ * Copyright (c) 2014 Google, Inc.
+ *
+ * Licensed under the terms of the GNU GPL License version 2
+ *
+ * Selftests for execveat(2).
+ */
+
+#define _GNU_SOURCE /* to get O_PATH, AT_EMPTY_PATH */
+#include <sys/sendfile.h>
+#include <sys/stat.h>
+#include <sys/syscall.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <limits.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+
+static char *envp[] = { "IN_TEST=yes", NULL };
+static char *argv[] = { "execveat", "99", NULL };
+
+static int execveat_(int fd, const char *path, char **argv, char **envp,
+ int flags)
+{
+#ifdef __NR_execveat
+ return syscall(__NR_execveat, fd, path, argv, envp, flags);
+#else
+ errno = -ENOSYS;
+ return -1;
+#endif
+}
+
+#define check_execveat_fail(fd, path, flags, errno) \
+ _check_execveat_fail(fd, path, flags, errno, #errno)
+static int _check_execveat_fail(int fd, const char *path, int flags,
+ int expected_errno, const char *errno_str)
+{
+ int rc;
+
+ errno = 0;
+ printf("Check failure of execveat(%d, '%s', %d) with %s... ",
+ fd, path?:"(null)", flags, errno_str);
+ rc = execveat_(fd, path, argv, envp, flags);
+
+ if (rc > 0) {
+ printf("[FAIL] (unexpected success from execveat(2))\n");
+ return 1;
+ }
+ if (errno != expected_errno) {
+ printf("[FAIL] (expected errno %d (%s) not %d (%s)\n",
+ expected_errno, strerror(expected_errno),
+ errno, strerror(errno));
+ return 1;
+ }
+ printf("[OK]\n");
+ return 0;
+}
+
+static int check_execveat_invoked_rc(int fd, const char *path, int flags,
+ int expected_rc)
+{
+ int status;
+ int rc;
+ pid_t child;
+
+ printf("Check success of execveat(%d, '%s', %d)... ",
+ fd, path?:"(null)", flags);
+ child = fork();
+ if (child < 0) {
+ printf("[FAIL] (fork() failed)\n");
+ return 1;
+ }
+ if (child == 0) {
+ /* Child: do execveat(). */
+ rc = execveat_(fd, path, argv, envp, flags);
+ printf("[FAIL]: execveat() failed, rc=%d errno=%d (%s)\n",
+ rc, errno, strerror(errno));
+ exit(1); /* should not reach here */
+ }
+ /* Parent: wait for & check child's exit status. */
+ rc = waitpid(child, &status, 0);
+ if (rc != child) {
+ printf("[FAIL] (waitpid(%d,...) returned %d)\n", child, rc);
+ return 1;
+ }
+ if (!WIFEXITED(status)) {
+ printf("[FAIL] (child %d did not exit cleanly, status=%08x)\n",
+ child, status);
+ return 1;
+ }
+ if (WEXITSTATUS(status) != expected_rc) {
+ printf("[FAIL] (child %d exited with %d not %d)\n",
+ child, WEXITSTATUS(status), expected_rc);
+ return 1;
+ }
+ printf("[OK]\n");
+ return 0;
+}
+
+static int check_execveat(int fd, const char *path, int flags)
+{
+ return check_execveat_invoked_rc(fd, path, flags, 99);
+}
+
+static char *concat(const char *left, const char *right)
+{
+ char *result = malloc(strlen(left) + strlen(right) + 1);
+
+ strcpy(result, left);
+ strcat(result, right);
+ return result;
+}
+
+static int open_or_die(const char *filename, int flags)
+{
+ int fd = open(filename, flags);
+
+ if (fd < 0) {
+ printf("Failed to open '%s'; "
+ "check prerequisites are available\n", filename);
+ exit(1);
+ }
+ return fd;
+}
+
+static int run_tests(void)
+{
+ int fail = 0;
+ char *fullname = realpath("execveat", NULL);
+ char *fullname_script = realpath("script", NULL);
+ char *fullname_symlink = concat(fullname, ".symlink");
+ int subdir_dfd = open_or_die("subdir", O_DIRECTORY|O_RDONLY);
+ int subdir_dfd_ephemeral = open_or_die("subdir.ephemeral",
+ O_DIRECTORY|O_RDONLY);
+ int dot_dfd = open_or_die(".", O_DIRECTORY|O_RDONLY);
+ int dot_dfd_path = open_or_die(".", O_DIRECTORY|O_RDONLY|O_PATH);
+ int fd = open_or_die("execveat", O_RDONLY);
+ int fd_path = open_or_die("execveat", O_RDONLY|O_PATH);
+ int fd_symlink = open_or_die("execveat.symlink", O_RDONLY);
+ int fd_script = open_or_die("script", O_RDONLY);
+ int fd_ephemeral = open_or_die("execveat.ephemeral", O_RDONLY);
+ int fd_script_ephemeral = open_or_die("script.ephemeral", O_RDONLY);
+
+ /* Normal executable file: */
+ /* dfd + path */
+ fail |= check_execveat(subdir_dfd, "../execveat", 0);
+ fail |= check_execveat(dot_dfd, "execveat", 0);
+ fail |= check_execveat(dot_dfd_path, "execveat", 0);
+ /* absolute path */
+ fail |= check_execveat(AT_FDCWD, fullname, 0);
+ /* absolute path with nonsense dfd */
+ fail |= check_execveat(99, fullname, 0);
+ /* fd + no path */
+ fail |= check_execveat(fd, "", AT_EMPTY_PATH);
+
+ /* Mess with executable file that's already open: */
+ /* fd + no path to a file that's been renamed */
+ rename("execveat.ephemeral", "execveat.moved");
+ fail |= check_execveat(fd_ephemeral, "", AT_EMPTY_PATH);
+ /* fd + no path to a file that's been deleted */
+ unlink("execveat.moved"); /* remove the file now fd open */
+ fail |= check_execveat(fd_ephemeral, "", AT_EMPTY_PATH);
+
+ /* Invalid argument failures */
+ fail |= check_execveat_fail(fd, "", 0, ENOENT);
+ fail |= check_execveat_fail(fd, NULL, AT_EMPTY_PATH, EFAULT);
+
+ /* Symlink to executable file: */
+ /* dfd + path */
+ fail |= check_execveat(dot_dfd, "execveat.symlink", 0);
+ fail |= check_execveat(dot_dfd_path, "execveat.symlink", 0);
+ /* absolute path */
+ fail |= check_execveat(AT_FDCWD, fullname_symlink, 0);
+ /* fd + no path, even with AT_SYMLINK_NOFOLLOW (already followed) */
+ fail |= check_execveat(fd_symlink, "", AT_EMPTY_PATH);
+ fail |= check_execveat(fd_symlink, "",
+ AT_EMPTY_PATH|AT_SYMLINK_NOFOLLOW);
+
+ /* Symlink fails when AT_SYMLINK_NOFOLLOW set: */
+ /* dfd + path */
+ fail |= check_execveat_fail(dot_dfd, "execveat.symlink",
+ AT_SYMLINK_NOFOLLOW, ELOOP);
+ fail |= check_execveat_fail(dot_dfd_path, "execveat.symlink",
+ AT_SYMLINK_NOFOLLOW, ELOOP);
+ /* absolute path */
+ fail |= check_execveat_fail(AT_FDCWD, fullname_symlink,
+ AT_SYMLINK_NOFOLLOW, ELOOP);
+
+ /* Shell script wrapping executable file: */
+ /* dfd + path */
+ fail |= check_execveat(subdir_dfd, "../script", 0);
+ fail |= check_execveat(dot_dfd, "script", 0);
+ fail |= check_execveat(dot_dfd_path, "script", 0);
+ /* absolute path */
+ fail |= check_execveat(AT_FDCWD, fullname_script, 0);
+ /* fd + no path */
+ fail |= check_execveat(fd_script, "", AT_EMPTY_PATH);
+ fail |= check_execveat(fd_script, "",
+ AT_EMPTY_PATH|AT_SYMLINK_NOFOLLOW);
+
+ /* Mess with script file that's already open: */
+ /* fd + no path to a file that's been renamed */
+ rename("script.ephemeral", "script.moved");
+ fail |= check_execveat(fd_script_ephemeral, "", AT_EMPTY_PATH);
+ /* fd + no path to a file that's been deleted */
+ unlink("script.moved"); /* remove the file while fd open */
+ fail |= check_execveat(fd_script_ephemeral, "", AT_EMPTY_PATH);
+
+ /* Rename a subdirectory in the path: */
+ rename("subdir.ephemeral", "subdir.moved");
+ fail |= check_execveat(subdir_dfd_ephemeral, "../script", 0);
+ fail |= check_execveat(subdir_dfd_ephemeral, "script", 0);
+ /* Remove the subdir and its contents */
+ unlink("subdir.moved/script");
+ unlink("subdir.moved");
+ /* Shell loads via deleted subdir OK because name starts with .. */
+ fail |= check_execveat(subdir_dfd_ephemeral, "../script", 0);
+ fail |= check_execveat_fail(subdir_dfd_ephemeral, "script", 0, ENOENT);
+
+ /* Flag values other than AT_SYMLINK_NOFOLLOW => EINVAL */
+ fail |= check_execveat_fail(dot_dfd, "execveat", 0xFFFF, EINVAL);
+ /* Invalid path => ENOENT */
+ fail |= check_execveat_fail(dot_dfd, "no-such-file", 0, ENOENT);
+ fail |= check_execveat_fail(dot_dfd_path, "no-such-file", 0, ENOENT);
+ fail |= check_execveat_fail(AT_FDCWD, "no-such-file", 0, ENOENT);
+ /* Attempt to execute directory => EACCES */
+ fail |= check_execveat_fail(dot_dfd, "", AT_EMPTY_PATH, EACCES);
+ /* Attempt to execute non-executable => EACCES */
+ fail |= check_execveat_fail(dot_dfd, "Makefile", 0, EACCES);
+ /* Attempt to execute file opened with O_PATH => EBADF */
+ fail |= check_execveat_fail(dot_dfd_path, "", AT_EMPTY_PATH, EBADF);
+ fail |= check_execveat_fail(fd_path, "", AT_EMPTY_PATH, EBADF);
+ /* Attempt to execute nonsense FD => EBADF */
+ fail |= check_execveat_fail(99, "", AT_EMPTY_PATH, EBADF);
+ fail |= check_execveat_fail(99, "execveat", 0, EBADF);
+ /* Attempt to execute relative to non-directory => ENOTDIR */
+ fail |= check_execveat_fail(fd, "execveat", 0, ENOTDIR);
+
+ return fail ? -1 : 0;
+}
+
+void exe_cp(const char *src, const char *dest)
+{
+ int in_fd = open_or_die(src, O_RDONLY);
+ int out_fd = open(dest, O_RDWR|O_CREAT|O_TRUNC, 0755);
+ struct stat info;
+
+ fstat(in_fd, &info);
+ sendfile(out_fd, in_fd, NULL, info.st_size);
+ close(in_fd);
+ close(out_fd);
+}
+
+void prerequisites(void)
+{
+ int fd;
+ const char *script = "#!/bin/sh\nexit $*\n";
+
+ /* Create ephemeral copies of files */
+ exe_cp("execveat", "execveat.ephemeral");
+ exe_cp("script", "script.ephemeral");
+ mkdir("subdir.ephemeral", 0755);
+
+ fd = open("subdir.ephemeral/script", O_RDWR|O_CREAT|O_TRUNC, 0755);
+ write(fd, script, strlen(script));
+ close(fd);
+}
+
+int main(int argc, char **argv)
+{
+ int rc;
+
+ if (argc >= 2) {
+ /* If we are invoked with an argument, exit immediately. */
+ /* Check expected environment transferred. */
+ const char *in_test = getenv("IN_TEST");
+
+ if (!in_test || strcmp(in_test, "yes") != 0) {
+ printf("[FAIL] (no IN_TEST=yes in env)\n");
+ return 1;
+ }
+
+ /* Use the final argument as an exit code. */
+ rc = atoi(argv[argc - 1]);
+ fflush(stdout);
+ } else {
+ prerequisites();
+ rc = run_tests();
+ }
+ return rc;
+}
--
2.1.0.rc2.206.gedb03e5
^ permalink raw reply related
* [PATCHv5 1/3] syscalls,x86: implement execveat() system call
From: David Drysdale @ 2014-10-22 11:44 UTC (permalink / raw)
To: Eric W. Biederman, Andy Lutomirski, Alexander Viro, Meredydd Luff,
linux-kernel
Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, Andrew Morton,
Kees Cook, Arnd Bergmann, x86, linux-arch, linux-api,
David Drysdale
In-Reply-To: <1413978269-17274-1-git-send-email-drysdale@google.com>
Add a new system execveat(2) syscall. execveat() is to execve() as
openat() is to open(): it takes a file descriptor that refers to a
directory, and resolves the filename relative to that.
In addition, if the filename is empty and AT_EMPTY_PATH is specified,
execveat() executes the file to which the file descriptor refers. This
replicates the functionality of fexecve(), which is a system call in
other UNIXen, but in Linux glibc it depends on opening
"/proc/self/fd/<fd>" (and so relies on /proc being mounted).
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.
Only x86-64, i386 and x32 ABIs are supported in this patch.
Based on patches by Meredydd Luff <meredydd@senatehouse.org>
Signed-off-by: David Drysdale <drysdale@google.com>
---
arch/x86/ia32/audit.c | 1 +
arch/x86/ia32/ia32entry.S | 1 +
arch/x86/kernel/audit_64.c | 1 +
arch/x86/kernel/entry_64.S | 28 ++++++++
arch/x86/syscalls/syscall_32.tbl | 1 +
arch/x86/syscalls/syscall_64.tbl | 2 +
arch/x86/um/sys_call_table_64.c | 1 +
fs/exec.c | 130 ++++++++++++++++++++++++++++++++++----
fs/namei.c | 2 +-
include/linux/compat.h | 3 +
include/linux/fs.h | 1 +
include/linux/sched.h | 4 ++
include/linux/syscalls.h | 4 ++
include/uapi/asm-generic/unistd.h | 4 +-
kernel/sys_ni.c | 3 +
lib/audit.c | 3 +
16 files changed, 173 insertions(+), 16 deletions(-)
diff --git a/arch/x86/ia32/audit.c b/arch/x86/ia32/audit.c
index 5d7b381da692..2eccc8932ae6 100644
--- a/arch/x86/ia32/audit.c
+++ b/arch/x86/ia32/audit.c
@@ -35,6 +35,7 @@ int ia32_classify_syscall(unsigned syscall)
case __NR_socketcall:
return 4;
case __NR_execve:
+ case __NR_execveat:
return 5;
default:
return 1;
diff --git a/arch/x86/ia32/ia32entry.S b/arch/x86/ia32/ia32entry.S
index 4299eb05023c..2516c09743e0 100644
--- a/arch/x86/ia32/ia32entry.S
+++ b/arch/x86/ia32/ia32entry.S
@@ -464,6 +464,7 @@ GLOBAL(\label)
PTREGSCALL stub32_rt_sigreturn, sys32_rt_sigreturn
PTREGSCALL stub32_sigreturn, sys32_sigreturn
PTREGSCALL stub32_execve, compat_sys_execve
+ PTREGSCALL stub32_execveat, compat_sys_execveat
PTREGSCALL stub32_fork, sys_fork
PTREGSCALL stub32_vfork, sys_vfork
diff --git a/arch/x86/kernel/audit_64.c b/arch/x86/kernel/audit_64.c
index 06d3e5a14d9d..f3672508b249 100644
--- a/arch/x86/kernel/audit_64.c
+++ b/arch/x86/kernel/audit_64.c
@@ -50,6 +50,7 @@ int audit_classify_syscall(int abi, unsigned syscall)
case __NR_openat:
return 3;
case __NR_execve:
+ case __NR_execveat:
return 5;
default:
return 0;
diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S
index 2fac1343a90b..00c4526e6ffe 100644
--- a/arch/x86/kernel/entry_64.S
+++ b/arch/x86/kernel/entry_64.S
@@ -665,6 +665,20 @@ ENTRY(stub_execve)
CFI_ENDPROC
END(stub_execve)
+ENTRY(stub_execveat)
+ CFI_STARTPROC
+ addq $8, %rsp
+ PARTIAL_FRAME 0
+ SAVE_REST
+ FIXUP_TOP_OF_STACK %r11
+ call sys_execveat
+ RESTORE_TOP_OF_STACK %r11
+ movq %rax,RAX(%rsp)
+ RESTORE_REST
+ jmp int_ret_from_sys_call
+ CFI_ENDPROC
+END(stub_execveat)
+
/*
* sigreturn is special because it needs to restore all registers on return.
* This cannot be done with SYSRET, so use the IRET return path instead.
@@ -710,6 +724,20 @@ ENTRY(stub_x32_execve)
CFI_ENDPROC
END(stub_x32_execve)
+ENTRY(stub_x32_execveat)
+ CFI_STARTPROC
+ addq $8, %rsp
+ PARTIAL_FRAME 0
+ SAVE_REST
+ FIXUP_TOP_OF_STACK %r11
+ call compat_sys_execveat
+ RESTORE_TOP_OF_STACK %r11
+ movq %rax,RAX(%rsp)
+ RESTORE_REST
+ jmp int_ret_from_sys_call
+ CFI_ENDPROC
+END(stub_x32_execveat)
+
#endif
/*
diff --git a/arch/x86/syscalls/syscall_32.tbl b/arch/x86/syscalls/syscall_32.tbl
index 028b78168d85..2633e3195455 100644
--- a/arch/x86/syscalls/syscall_32.tbl
+++ b/arch/x86/syscalls/syscall_32.tbl
@@ -363,3 +363,4 @@
354 i386 seccomp sys_seccomp
355 i386 getrandom sys_getrandom
356 i386 memfd_create sys_memfd_create
+357 i386 execveat sys_execveat stub32_execveat
diff --git a/arch/x86/syscalls/syscall_64.tbl b/arch/x86/syscalls/syscall_64.tbl
index 35dd922727b9..1af5badd159c 100644
--- a/arch/x86/syscalls/syscall_64.tbl
+++ b/arch/x86/syscalls/syscall_64.tbl
@@ -327,6 +327,7 @@
318 common getrandom sys_getrandom
319 common memfd_create sys_memfd_create
320 common kexec_file_load sys_kexec_file_load
+321 64 execveat stub_execveat
#
# x32-specific system call numbers start at 512 to avoid cache impact
@@ -365,3 +366,4 @@
542 x32 getsockopt compat_sys_getsockopt
543 x32 io_setup compat_sys_io_setup
544 x32 io_submit compat_sys_io_submit
+545 x32 execveat stub_x32_execveat
diff --git a/arch/x86/um/sys_call_table_64.c b/arch/x86/um/sys_call_table_64.c
index f2f0723070ca..20c3649d0691 100644
--- a/arch/x86/um/sys_call_table_64.c
+++ b/arch/x86/um/sys_call_table_64.c
@@ -31,6 +31,7 @@
#define stub_fork sys_fork
#define stub_vfork sys_vfork
#define stub_execve sys_execve
+#define stub_execveat sys_execveat
#define stub_rt_sigreturn sys_rt_sigreturn
#define __SYSCALL_COMMON(nr, sym, compat) __SYSCALL_64(nr, sym, compat)
diff --git a/fs/exec.c b/fs/exec.c
index a2b42a98c743..92a6e14f096a 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -747,7 +747,7 @@ EXPORT_SYMBOL(setup_arg_pages);
#endif /* CONFIG_MMU */
-static struct file *do_open_exec(struct filename *name)
+static struct file *do_open_execat(int fd, struct filename *name, int flags)
{
struct file *file;
int err;
@@ -757,10 +757,34 @@ static struct file *do_open_exec(struct filename *name)
.intent = LOOKUP_OPEN,
.lookup_flags = LOOKUP_FOLLOW,
};
+ static const struct open_flags open_exec_nofollow_flags = {
+ .open_flag = O_LARGEFILE | O_RDONLY | __FMODE_EXEC,
+ .acc_mode = MAY_EXEC | MAY_OPEN,
+ .intent = LOOKUP_OPEN,
+ .lookup_flags = 0,
+ };
- file = do_filp_open(AT_FDCWD, name, &open_exec_flags);
- if (IS_ERR(file))
- goto out;
+ if ((flags & ~(AT_SYMLINK_NOFOLLOW | AT_EMPTY_PATH)) != 0)
+ return ERR_PTR(-EINVAL);
+
+ if (name->name[0] != '\0') {
+ const struct open_flags *oflags = ((flags & AT_SYMLINK_NOFOLLOW)
+ ? &open_exec_nofollow_flags
+ : &open_exec_flags);
+
+ file = do_filp_open(fd, name, oflags);
+ if (IS_ERR(file))
+ goto out;
+ } else {
+ file = fget(fd);
+ if (!file)
+ return ERR_PTR(-EBADF);
+
+ err = inode_permission(file->f_path.dentry->d_inode,
+ open_exec_flags.acc_mode);
+ if (err)
+ goto exit;
+ }
err = -EACCES;
if (!S_ISREG(file_inode(file)->i_mode))
@@ -769,12 +793,13 @@ static struct file *do_open_exec(struct filename *name)
if (file->f_path.mnt->mnt_flags & MNT_NOEXEC)
goto exit;
- fsnotify_open(file);
-
err = deny_write_access(file);
if (err)
goto exit;
+ if (name->name[0] != '\0')
+ fsnotify_open(file);
+
out:
return file;
@@ -786,7 +811,7 @@ exit:
struct file *open_exec(const char *name)
{
struct filename tmp = { .name = name };
- return do_open_exec(&tmp);
+ return do_open_execat(AT_FDCWD, &tmp, 0);
}
EXPORT_SYMBOL(open_exec);
@@ -1422,10 +1447,12 @@ static int exec_binprm(struct linux_binprm *bprm)
/*
* sys_execve() executes a new program.
*/
-static int do_execve_common(struct filename *filename,
- struct user_arg_ptr argv,
- struct user_arg_ptr envp)
+static int do_execveat_common(int fd, struct filename *filename,
+ struct user_arg_ptr argv,
+ struct user_arg_ptr envp,
+ int flags)
{
+ char *pathbuf = NULL;
struct linux_binprm *bprm;
struct file *file;
struct files_struct *displaced;
@@ -1466,7 +1493,7 @@ static int do_execve_common(struct filename *filename,
check_unsafe_exec(bprm);
current->in_execve = 1;
- file = do_open_exec(filename);
+ file = do_open_execat(fd, filename, flags);
retval = PTR_ERR(file);
if (IS_ERR(file))
goto out_unmark;
@@ -1474,7 +1501,27 @@ static int do_execve_common(struct filename *filename,
sched_exec();
bprm->file = file;
- bprm->filename = bprm->interp = filename->name;
+ if (fd == AT_FDCWD || filename->name[0] == '/') {
+ bprm->filename = filename->name;
+ } else {
+ /*
+ * Build a pathname that reflects how we got to the file,
+ * either "/dev/fd/<fd>" (for an empty filename) or
+ * "/dev/fd/<fd>/<filename>".
+ */
+ pathbuf = kmalloc(PATH_MAX, GFP_TEMPORARY);
+ if (!pathbuf) {
+ retval = -ENOMEM;
+ goto out_unmark;
+ }
+ bprm->filename = pathbuf;
+ if (filename->name[0] == '\0')
+ sprintf(pathbuf, "/dev/fd/%d", fd);
+ else
+ snprintf(pathbuf, PATH_MAX,
+ "/dev/fd/%d/%s", fd, filename->name);
+ }
+ bprm->interp = bprm->filename;
retval = bprm_mm_init(bprm);
if (retval)
@@ -1532,6 +1579,7 @@ out_unmark:
out_free:
free_bprm(bprm);
+ kfree(pathbuf);
out_files:
if (displaced)
@@ -1547,7 +1595,18 @@ int do_execve(struct filename *filename,
{
struct user_arg_ptr argv = { .ptr.native = __argv };
struct user_arg_ptr envp = { .ptr.native = __envp };
- return do_execve_common(filename, argv, envp);
+ return do_execveat_common(AT_FDCWD, filename, argv, envp, 0);
+}
+
+int do_execveat(int fd, struct filename *filename,
+ const char __user *const __user *__argv,
+ const char __user *const __user *__envp,
+ int flags)
+{
+ struct user_arg_ptr argv = { .ptr.native = __argv };
+ struct user_arg_ptr envp = { .ptr.native = __envp };
+
+ return do_execveat_common(fd, filename, argv, envp, flags);
}
#ifdef CONFIG_COMPAT
@@ -1563,7 +1622,23 @@ static int compat_do_execve(struct filename *filename,
.is_compat = true,
.ptr.compat = __envp,
};
- return do_execve_common(filename, argv, envp);
+ return do_execveat_common(AT_FDCWD, filename, argv, envp, 0);
+}
+
+static int compat_do_execveat(int fd, struct filename *filename,
+ const compat_uptr_t __user *__argv,
+ const compat_uptr_t __user *__envp,
+ int flags)
+{
+ struct user_arg_ptr argv = {
+ .is_compat = true,
+ .ptr.compat = __argv,
+ };
+ struct user_arg_ptr envp = {
+ .is_compat = true,
+ .ptr.compat = __envp,
+ };
+ return do_execveat_common(fd, filename, argv, envp, flags);
}
#endif
@@ -1603,6 +1678,20 @@ SYSCALL_DEFINE3(execve,
{
return do_execve(getname(filename), argv, envp);
}
+
+SYSCALL_DEFINE5(execveat,
+ int, fd, const char __user *, filename,
+ const char __user *const __user *, argv,
+ const char __user *const __user *, envp,
+ int, flags)
+{
+ int lookup_flags = (flags & AT_EMPTY_PATH) ? LOOKUP_EMPTY : 0;
+
+ return do_execveat(fd,
+ getname_flags(filename, lookup_flags, NULL),
+ argv, envp, flags);
+}
+
#ifdef CONFIG_COMPAT
COMPAT_SYSCALL_DEFINE3(execve, const char __user *, filename,
const compat_uptr_t __user *, argv,
@@ -1610,4 +1699,17 @@ COMPAT_SYSCALL_DEFINE3(execve, const char __user *, filename,
{
return compat_do_execve(getname(filename), argv, envp);
}
+
+COMPAT_SYSCALL_DEFINE5(execveat, int, fd,
+ const char __user *, filename,
+ const compat_uptr_t __user *, argv,
+ const compat_uptr_t __user *, envp,
+ int, flags)
+{
+ int lookup_flags = (flags & AT_EMPTY_PATH) ? LOOKUP_EMPTY : 0;
+
+ return compat_do_execveat(fd,
+ getname_flags(filename, lookup_flags, NULL),
+ argv, envp, flags);
+}
#endif
diff --git a/fs/namei.c b/fs/namei.c
index a7b05bf82d31..553c84d3e0cc 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -130,7 +130,7 @@ void final_putname(struct filename *name)
#define EMBEDDED_NAME_MAX (PATH_MAX - sizeof(struct filename))
-static struct filename *
+struct filename *
getname_flags(const char __user *filename, int flags, int *empty)
{
struct filename *result, *err;
diff --git a/include/linux/compat.h b/include/linux/compat.h
index e6494261eaff..7450ca2ac1fc 100644
--- a/include/linux/compat.h
+++ b/include/linux/compat.h
@@ -357,6 +357,9 @@ asmlinkage long compat_sys_lseek(unsigned int, compat_off_t, unsigned int);
asmlinkage long compat_sys_execve(const char __user *filename, const compat_uptr_t __user *argv,
const compat_uptr_t __user *envp);
+asmlinkage long compat_sys_execveat(int dfd, const char __user *filename,
+ const compat_uptr_t __user *argv,
+ const compat_uptr_t __user *envp, int flags);
asmlinkage long compat_sys_select(int n, compat_ulong_t __user *inp,
compat_ulong_t __user *outp, compat_ulong_t __user *exp,
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 94187721ad41..e9818574d738 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2060,6 +2060,7 @@ extern struct file *file_open_root(struct dentry *, struct vfsmount *,
extern struct file * dentry_open(const struct path *, int, const struct cred *);
extern int filp_close(struct file *, fl_owner_t id);
+extern struct filename *getname_flags(const char __user *, int, int *);
extern struct filename *getname(const char __user *);
extern struct filename *getname_kernel(const char *);
diff --git a/include/linux/sched.h b/include/linux/sched.h
index b867a4dab38a..33e056da7d33 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -2430,6 +2430,10 @@ extern void do_group_exit(int);
extern int do_execve(struct filename *,
const char __user * const __user *,
const char __user * const __user *);
+extern int do_execveat(int, struct filename *,
+ const char __user * const __user *,
+ const char __user * const __user *,
+ int);
extern long do_fork(unsigned long, unsigned long, unsigned long, int __user *, int __user *);
struct task_struct *fork_idle(int);
extern pid_t kernel_thread(int (*fn)(void *), void *arg, unsigned long flags);
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index 0f86d85a9ce4..df5422294deb 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -876,4 +876,8 @@ asmlinkage long sys_seccomp(unsigned int op, unsigned int flags,
asmlinkage long sys_getrandom(char __user *buf, size_t count,
unsigned int flags);
+asmlinkage long sys_execveat(int dfd, const char __user *filename,
+ const char __user *const __user *argv,
+ const char __user *const __user *envp, int flags);
+
#endif
diff --git a/include/uapi/asm-generic/unistd.h b/include/uapi/asm-generic/unistd.h
index 11d11bc5c78f..feef07d29663 100644
--- a/include/uapi/asm-generic/unistd.h
+++ b/include/uapi/asm-generic/unistd.h
@@ -705,9 +705,11 @@ __SYSCALL(__NR_seccomp, sys_seccomp)
__SYSCALL(__NR_getrandom, sys_getrandom)
#define __NR_memfd_create 279
__SYSCALL(__NR_memfd_create, sys_memfd_create)
+#define __NR_execveat 280
+__SC_COMP(__NR_execveat, sys_execveat, compat_sys_execveat)
#undef __NR_syscalls
-#define __NR_syscalls 280
+#define __NR_syscalls 281
/*
* All syscalls below here should go away really,
diff --git a/kernel/sys_ni.c b/kernel/sys_ni.c
index 391d4ddb6f4b..efb06058ad3e 100644
--- a/kernel/sys_ni.c
+++ b/kernel/sys_ni.c
@@ -218,3 +218,6 @@ cond_syscall(sys_kcmp);
/* operate on Secure Computing state */
cond_syscall(sys_seccomp);
+
+/* execveat */
+cond_syscall(sys_execveat);
diff --git a/lib/audit.c b/lib/audit.c
index 1d726a22565b..b8fb5ee81e26 100644
--- a/lib/audit.c
+++ b/lib/audit.c
@@ -54,6 +54,9 @@ int audit_classify_syscall(int abi, unsigned syscall)
case __NR_socketcall:
return 4;
#endif
+#ifdef __NR_execveat
+ case __NR_execveat:
+#endif
case __NR_execve:
return 5;
default:
--
2.1.0.rc2.206.gedb03e5
^ permalink raw reply related
* [PATCHv5 0/3] syscalls,x86: Add execveat() system call
From: David Drysdale @ 2014-10-22 11:44 UTC (permalink / raw)
To: Eric W. Biederman, Andy Lutomirski, Alexander Viro, Meredydd Luff,
linux-kernel
Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, Andrew Morton,
Kees Cook, Arnd Bergmann, x86, linux-arch, linux-api,
David Drysdale
This patch set adds execveat(2) for x86, and is derived from Meredydd
Luff's patch from Sept 2012 (https://lkml.org/lkml/2012/9/11/528).
The primary aim of adding an execveat syscall is to allow an
implementation of fexecve(3) that does not rely on the /proc
filesystem. The current glibc version of fexecve(3) is implemented
via /proc, which causes problems in sandboxed or otherwise restricted
environments.
Given the desire for a /proc-free fexecve() implementation, HPA
suggested (https://lkml.org/lkml/2006/7/11/556) that an execveat(2)
syscall would be an appropriate generalization.
Also, having a new syscall means that it can take a flags argument
without back-compatibility concerns. The current implementation just
defines the AT_EMPTY_PATH and AT_SYMLINK_NOFOLLOW flags, but other
flags could be added in future -- for example, flags for new namespaces
(as suggested at https://lkml.org/lkml/2006/7/11/474).
Related history:
- https://lkml.org/lkml/2006/12/27/123 is an example of someone
realizing that fexecve() is likely to fail in a chroot environment.
- http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=514043 covered
documenting the /proc requirement of fexecve(3) in its manpage, to
"prevent other people from wasting their time".
- https://bugzilla.kernel.org/show_bug.cgi?id=74481 documented that
it's not possible to fexecve() a file descriptor for a script with
close-on-exec set (which is possible with the implementation here).
- https://bugzilla.redhat.com/show_bug.cgi?id=241609 described a
problem where a process that did setuid() could not fexecve()
because it no longer had access to /proc/self/fd; this has since
been fixed.
Changes since v4, suggested by Eric W. Biederman:
- Use empty filename with AT_EMPTY_PATH flag rather than NULL
pathname to request fexecve-like behaviour.
- Build pathname as "/dev/fd/<fd>/<filename>" (or "/dev/fd/<fd>")
rather than using d_path().
- Patch against v3.17 (bfe01a5ba249)
Changes since Meredydd's v3 patch:
- Added a selftest.
- Added a man page.
- Left open_exec() signature untouched to reduce patch impact
elsewhere (as suggested by Al Viro).
- Filled in bprm->filename with d_path() into a buffer, to avoid use
of potentially-ephemeral dentry->d_name.
- Patch against v3.14 (455c6fdbd21916).
David Drysdale (2):
syscalls,x86: implement execveat() system call
syscalls,x86: add selftest for execveat(2)
arch/x86/ia32/audit.c | 1 +
arch/x86/ia32/ia32entry.S | 1 +
arch/x86/kernel/audit_64.c | 1 +
arch/x86/kernel/entry_64.S | 28 +++
arch/x86/syscalls/syscall_32.tbl | 1 +
arch/x86/syscalls/syscall_64.tbl | 2 +
arch/x86/um/sys_call_table_64.c | 1 +
fs/exec.c | 130 ++++++++++++--
fs/namei.c | 2 +-
include/linux/compat.h | 3 +
include/linux/fs.h | 1 +
include/linux/sched.h | 4 +
include/linux/syscalls.h | 4 +
include/uapi/asm-generic/unistd.h | 4 +-
kernel/sys_ni.c | 3 +
lib/audit.c | 3 +
tools/testing/selftests/Makefile | 1 +
tools/testing/selftests/exec/.gitignore | 6 +
tools/testing/selftests/exec/Makefile | 22 +++
tools/testing/selftests/exec/execveat.c | 295 ++++++++++++++++++++++++++++++++
20 files changed, 497 insertions(+), 16 deletions(-)
create mode 100644 tools/testing/selftests/exec/.gitignore
create mode 100644 tools/testing/selftests/exec/Makefile
create mode 100644 tools/testing/selftests/exec/execveat.c
--
2.1.0.rc2.206.gedb03e5
^ permalink raw reply
* Re: [PATCHv4 RESEND 0/3] syscalls,x86: Add execveat() system call
From: David Drysdale @ 2014-10-22 11:08 UTC (permalink / raw)
To: Eric W. Biederman
Cc: Andy Lutomirski, Alexander Viro, Meredydd Luff,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Thomas Gleixner, Ingo Molnar, H. Peter Anvin, Andrew Morton,
Kees Cook, Arnd Bergmann, X86 ML, linux-arch, Linux API
In-Reply-To: <87ioje2ggq.fsf-JOvCrm2gF+uungPnsOpG7nhyD016LWXt@public.gmane.org>
On Tue, Oct 21, 2014 at 5:29 AM, Eric W. Biederman
<ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org> wrote:
> Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org> writes:
>
>> On Mon, Oct 20, 2014 at 6:48 AM, David Drysdale <drysdale-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> wrote:
>>> On Sun, Oct 19, 2014 at 1:20 AM, Eric W. Biederman
>>> <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org> wrote:
>>>> Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org> writes:
>>>>
>>>>> [Added Eric Biederman, since I think your tree might be a reasonable
>>>>> route forward for these patches.]
>>>>>
>>>>> On Thu, Jun 5, 2014 at 6:40 AM, David Drysdale <drysdale-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> wrote:
>>>>>> Resending, adding cc:linux-api.
>>>>>>
>>>>>> Also, it may help to add a little more background -- this patch is
>>>>>> needed as a (small) part of implementing Capsicum in the Linux kernel.
>>>>>>
>>>>>> Capsicum is a security framework that has been present in FreeBSD since
>>>>>> version 9.0 (Jan 2012), and is based on concepts from object-capability
>>>>>> security [1].
>>>>>>
>>>>>> One of the features of Capsicum is capability mode, which locks down
>>>>>> access to global namespaces such as the filesystem hierarchy. In
>>>>>> capability mode, /proc is thus inaccessible and so fexecve(3) doesn't
>>>>>> work -- hence the need for a kernel-space
>>>>>
>>>>> I just found myself wanting this syscall for another reason: injecting
>>>>> programs into sandboxes or otherwise heavily locked-down namespaces.
>>>>>
>>>>> For example, I want to be able to reliably do something like nsenter
>>>>> --namespace-flags-here toybox sh. Toybox's shell is unusual in that
>>>>> it is more or less fully functional, so this should Just Work (tm),
>>>>> except that the toybox binary might not exist in the namespace being
>>>>> entered. If execveat were available, I could rig nsenter or a similar
>>>>> tool to open it with O_CLOEXEC, enter the namespace, and then call
>>>>> execveat.
>>>>>
>>>>> Is there any reason that these patches can't be merged more or less as
>>>>> is for 3.19?
>>>>
>>>> Yes. There is a silliness in how it implements fexecve. The fexecve
>>>> case should be use the empty string "" not a NULL pointer to indication
>>>> that. That change will then harmonize execveat with the other ...at
>>>> system calls and simplify the code and remove a special case. I believe
>>>> using the empty string "" requires implementing the AT_EMPTY_PATH flag.
>>>
>>> Good point -- I'll shift to "" + AT_EMPTY_PATH.
>>
>> Pending a better idea, I would also see if the patches can be changed
>> to return an error if d_path ends up with an "(unreachable)" thing
>> rather than failing inexplicably later on.
>
> For my reference we are talking about
>
>> @@ -1489,7 +1524,21 @@ static int do_execve_common(struct filename *filename,
>> sched_exec();
>>
>> bprm->file = file;
>> - bprm->filename = bprm->interp = filename->name;
>> + if (filename && fd == AT_FDCWD) {
>> + bprm->filename = filename->name;
>> + } else {
>> + pathbuf = kmalloc(PATH_MAX, GFP_TEMPORARY);
>> + if (!pathbuf) {
>> + retval = -ENOMEM;
>> + goto out_unmark;
>> + }
>> + bprm->filename = d_path(&file->f_path, pathbuf, PATH_MAX);
>> + if (IS_ERR(bprm->filename)) {
>> + retval = PTR_ERR(bprm->filename);
>> + goto out_unmark;
>> + }
>> + }
>> + bprm->interp = bprm->filename;
>>
>> retval = bprm_mm_init(bprm);
>> if (retval)
>
> The interesting case for fexecve is when we either don't know what files
> are present or we don't want to depend on which files are present.
>
> As Al pointed out d_path really isn't the right solution. It fails when
> printing /proc/self/fd/${fd}/${filename->name} would work, and the
> "(deleted)" or "(unreachable)" strings are wrong.
>
> The test for today's cases should be:
> if ((filename->name[0] == '/') || fd == AT_FDCWD) {
> bprm->filename = filename->name;
> }
>
> To handle the case where the file descriptor is relevant.
(s/relevant/irrelevant)
Yep, good spot.
> For the case where the file descriptor is relevant let me suggest
> setting bprm->filename and bprm->interp to:
>
> /dev/fd/${fd}/${filename->name}
I'll send out an updated patchset with this approach, but I have a slight
reservation. Given that /dev/fd is a symlink to /proc/self/fd, this approach
means that script invocations will always fail on a /proc-less system,
where the previous iteration might have worked.
(As it happens, this isn't a restriction that affects the things I'm
working on, as Capsicum wouldn't allow script invocation anyway.
However, scenarios without /proc were nominally one of the motivating
factors for execveat in the first place...)
> It is more a description of what we have done but as a magic string it
> is descriptive. Documetation/devices.txt documents that /dev/fd/ should
> exist, making it an unambiguous path. Further these days the kernel
> sets the device naming policy in dev, so I think we are strongly safe in
> using that path in any event.
>
> I think execveat is interesting in the kernel because the motivating
> cases are the cases where anything except a static executable is
> uninteresting.
FYI, there is potential in the future for something other than static
executables -- the FreeBSD Capsicum implementation includes changes
to the dynamic linker to get its search path as a list of pre-opened dfds
(in LD_LIBRARY_PATH_FDS) rather than paths.
> Now it has been suggested creating a dupfs or a mini-proc. I think that
> sounds like a nice companion, to the concept of a locked down root.
> But I don't think it removes the need for execveat (because we still
> have the case where we don't want to care what is mounted, and are happy
> to use static executables).
>
> Eric
>
^ 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