* [PATCH 2/2] Documentation: iio-trig-sysfs: Add remove_trigger attribute ABI
From: Cristina Opriceana @ 2015-08-12 11:29 UTC (permalink / raw)
To: jic23-DgEjT+Ai2ygdnm+yROfE0A
Cc: daniel.baluta-ral2JQCrhuEAvxtiuMwx3w,
linux-iio-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <cover.1439378363.git.cristina.opriceana-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
This patch adds the documentation ABI for the remove_trigger
attribute, provided by the iio-trig-sysfs stand-alone driver.
Signed-off-by: Cristina Opriceana <cristina.opriceana-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
Documentation/ABI/testing/sysfs-bus-iio-trigger-sysfs | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/Documentation/ABI/testing/sysfs-bus-iio-trigger-sysfs b/Documentation/ABI/testing/sysfs-bus-iio-trigger-sysfs
index 887aa92..6de6e6b 100644
--- a/Documentation/ABI/testing/sysfs-bus-iio-trigger-sysfs
+++ b/Documentation/ABI/testing/sysfs-bus-iio-trigger-sysfs
@@ -20,3 +20,14 @@ Description:
into the associated file, which will serve as the id of the
trigger. If the trigger with the specified id is already present
in the system, an invalid argument message will be returned.
+
+What: /sys/bus/iio/devices/iio_sysfs_trigger/remove_trigger
+KernelVersion: 2.6.39
+Contact: linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
+Description:
+ This attribute is used to unregister and delete a previously
+ created trigger from the list of available triggers. In order to
+ achieve this, one should write a positive integer into the
+ associated file, representing the id of the trigger that needs
+ to be removed. If the trigger can't be found, an invalid
+ argument message will be returned to the user.
--
1.9.1
^ permalink raw reply related
* Re: [PATCH v7 3/6] mm: Introduce VM_LOCKONFAULT
From: Michal Hocko @ 2015-08-12 11:59 UTC (permalink / raw)
To: Eric B Munson
Cc: Andrew Morton, Vlastimil Babka, Jonathan Corbet,
Kirill A. Shutemov, linux-kernel, dri-devel, linux-mm, linux-api
In-Reply-To: <1439097776-27695-4-git-send-email-emunson@akamai.com>
On Sun 09-08-15 01:22:53, Eric B Munson wrote:
> The cost of faulting in all memory to be locked can be very high when
> working with large mappings. If only portions of the mapping will be
> used this can incur a high penalty for locking.
>
> For the example of a large file, this is the usage pattern for a large
> statical language model (probably applies to other statical or graphical
> models as well). For the security example, any application transacting
> in data that cannot be swapped out (credit card data, medical records,
> etc).
>
> This patch introduces the ability to request that pages are not
> pre-faulted, but are placed on the unevictable LRU when they are finally
> faulted in. The VM_LOCKONFAULT flag will be used together with
> VM_LOCKED and has no effect when set without VM_LOCKED.
I do not like this very much to be honest. We have only few bits
left there and it seems this is not really necessary. I thought that
LOCKONFAULT acts as a modifier to the mlock call to tell whether to
poppulate or not. The only place we have to persist it is
mlockall(MCL_FUTURE) AFAICS. And this can be handled by an additional
field in the mm_struct. This could be handled at __mm_populate level.
So unless I am missing something this would be much more easier
in the end we no new bit in VM flags would be necessary.
This would obviously mean that the LOCKONFAULT couldn't be exported to
the userspace but is this really necessary?
--
Michal Hocko
SUSE Labs
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* [PATCH v9 0/3] Enable PPI sysfs interface for TPM 2.0
From: Jarkko Sakkinen @ 2015-08-12 14:05 UTC (permalink / raw)
To: tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: peterhuewe-Mmb7MZpHnFY, gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/, Jarkko Sakkinen,
Eric W. Biederman, Guenter Roeck, open list:ABI/API, NeilBrown,
Tejun Heo
v9:
* Fixed naming for __compat_only_sysfs_link_entry_to_kobj()
v8:
* Renamed sysfs_link_entry_to_kobj() to
__compat_only_sysfs_link_entry_to_kobj()
v7:
* Fixed compile error when CONFIG_SYSFS is not enabled.
v6:
* Updated documentation.
v5:
* Removed dangling export of kernfs_remove_by_name_ns() from the sysfs
patch.
v4:
* Use sysfs_remove_link()
v3:
* Fixed to_tpm_chip() macro.
* Split into two patches.
* Renamed sysfs_link_group_to_kobj to sysfs_link_entry_to_kobj
* Only create the "backwards compatibility" symlink for TPM 1.x devices.
Jarkko Sakkinen (3):
sysfs: added __compat_only_sysfs_link_entry_to_kobj()
tpm: move the PPI attributes to character device directory.
tpm: update PPI documentation to address the location change.
Documentation/ABI/testing/sysfs-driver-ppi | 19 ++++++++-----
drivers/char/tpm/tpm-chip.c | 24 ++++++++++------
drivers/char/tpm/tpm.h | 17 ++++--------
drivers/char/tpm/tpm_ppi.c | 34 ++++++++---------------
fs/sysfs/group.c | 44 ++++++++++++++++++++++++++++++
include/linux/sysfs.h | 11 ++++++++
6 files changed, 100 insertions(+), 49 deletions(-)
--
2.5.0
^ permalink raw reply
* [PATCH v9 3/3] tpm: update PPI documentation to address the location change.
From: Jarkko Sakkinen @ 2015-08-12 14:05 UTC (permalink / raw)
To: tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: peterhuewe-Mmb7MZpHnFY, gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/, Jarkko Sakkinen,
open list:ABI/API
In-Reply-To: <1439388347-22325-1-git-send-email-jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Updated Documentation/ABI/testing/sysfs-driver-ppi in order to explain
where PPI attributes are located and how backwards compatiblity is
addressed.
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
Documentation/ABI/testing/sysfs-driver-ppi | 19 ++++++++++++-------
1 file changed, 12 insertions(+), 7 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-driver-ppi b/Documentation/ABI/testing/sysfs-driver-ppi
index 7d1435b..9921ef2 100644
--- a/Documentation/ABI/testing/sysfs-driver-ppi
+++ b/Documentation/ABI/testing/sysfs-driver-ppi
@@ -1,4 +1,4 @@
-What: /sys/devices/pnp0/<bus-num>/ppi/
+What: /sys/class/tpm/tpmX/ppi/
Date: August 2012
Kernel Version: 3.6
Contact: xiaoyan.zhang-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org
@@ -8,9 +8,14 @@ Description:
folder makes sense. The folder path can be got by command
'find /sys/ -name 'pcrs''. For the detail information of PPI,
please refer to the PPI specification from
+
http://www.trustedcomputinggroup.org/
-What: /sys/devices/pnp0/<bus-num>/ppi/version
+ In Linux 4.2 ppi was moved to the character device directory.
+ A symlink from tpmX/device/ppi to tpmX/ppi to provide backwards
+ compatibility.
+
+What: /sys/class/tpm/tpmX/ppi/version
Date: August 2012
Contact: xiaoyan.zhang-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org
Description:
@@ -18,7 +23,7 @@ Description:
platform.
This file is readonly.
-What: /sys/devices/pnp0/<bus-num>/ppi/request
+What: /sys/class/tpm/tpmX/ppi/request
Date: August 2012
Contact: xiaoyan.zhang-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org
Description:
@@ -28,7 +33,7 @@ Description:
integer value range from 1 to 160, and 0 means no request.
This file can be read and written.
-What: /sys/devices/pnp0/00:<bus-num>/ppi/response
+What: /sys/class/tpm/tpmX/ppi/response
Date: August 2012
Contact: xiaoyan.zhang-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org
Description:
@@ -37,7 +42,7 @@ Description:
: <response description>".
This file is readonly.
-What: /sys/devices/pnp0/<bus-num>/ppi/transition_action
+What: /sys/class/tpm/tpmX/ppi/transition_action
Date: August 2012
Contact: xiaoyan.zhang-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org
Description:
@@ -47,7 +52,7 @@ Description:
description>".
This file is readonly.
-What: /sys/devices/pnp0/<bus-num>/ppi/tcg_operations
+What: /sys/class/tpm/tpmX/ppi/tcg_operations
Date: August 2012
Contact: xiaoyan.zhang-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org
Description:
@@ -58,7 +63,7 @@ Description:
This attribute is only supported by PPI version 1.2+.
This file is readonly.
-What: /sys/devices/pnp0/<bus-num>/ppi/vs_operations
+What: /sys/class/tpm/tpmX/ppi/vs_operations
Date: August 2012
Contact: xiaoyan.zhang-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org
Description:
--
2.5.0
^ permalink raw reply related
* Re: [PATCH v5 2/6] cpu_isolated: add initial support
From: Frederic Weisbecker @ 2015-08-12 16:00 UTC (permalink / raw)
To: Chris Metcalf
Cc: Gilad Ben Yossef, Steven Rostedt, Ingo Molnar, Peter Zijlstra,
Andrew Morton, Rik van Riel, Tejun Heo, Thomas Gleixner,
Paul E. McKenney, Christoph Lameter, Viresh Kumar,
Catalin Marinas, Will Deacon, linux-doc-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1438112980-9981-3-git-send-email-cmetcalf-d5a29ZRxExrQT0dZR+AlfA@public.gmane.org>
On Tue, Jul 28, 2015 at 03:49:36PM -0400, Chris Metcalf wrote:
> The existing nohz_full mode is designed as a "soft" isolation mode
> that makes tradeoffs to minimize userspace interruptions while
> still attempting to avoid overheads in the kernel entry/exit path,
> to provide 100% kernel semantics, etc.
>
> However, some applications require a "hard" commitment from the
> kernel to avoid interruptions, in particular userspace device
> driver style applications, such as high-speed networking code.
>
> This change introduces a framework to allow applications
> to elect to have the "hard" semantics as needed, specifying
> prctl(PR_SET_CPU_ISOLATED, PR_CPU_ISOLATED_ENABLE) to do so.
> Subsequent commits will add additional flags and additional
> semantics.
We are doing this at the process level but the isolation works on
the CPU scope... Now I wonder if prctl is the right interface.
That said the user is rather interested in isolating a task. The CPU
being the backend eventually.
For example if the task is migrated by accident, we want it to be
warned about that. And if the isolation is done on the CPU level
instead of the task level, this won't happen.
I'm also afraid that the naming clashes with cpu_isolated_map,
although it could be a subset of it.
So probably in this case we should consider talking about task rather
than CPU isolation and change naming accordingly (sorry, I know I
suggested cpu_isolation.c, I guess I had to see the result to realize).
We must sort that out first. Either we consider isolation on the task
level (and thus the underlying CPU by backend effect) and we use prctl().
Or we do this on the CPU level and we use a specific syscall or sysfs
which takes effect on any task in the relevant isolated CPUs.
What do you think?
It would be nice to hear others opinions as well.
> The kernel must be built with the new CPU_ISOLATED Kconfig flag
> to enable this mode, and the kernel booted with an appropriate
> nohz_full=CPULIST boot argument. The "cpu_isolated" state is then
> indicated by setting a new task struct field, cpu_isolated_flags,
> to the value passed by prctl(). When the _ENABLE bit is set for a
> task, and it is returning to userspace on a nohz_full core, it calls
> the new cpu_isolated_enter() routine to take additional actions
> to help the task avoid being interrupted in the future.
>
> Initially, there are only three actions taken. First, the
> task calls lru_add_drain() to prevent being interrupted by a
> subsequent lru_add_drain_all() call on another core. Then, it calls
> quiet_vmstat() to quieten the vmstat worker to avoid a follow-on
> interrupt. Finally, the code checks for pending timer interrupts
> and quiesces until they are no longer pending. As a result, sys
> calls (and page faults, etc.) can be inordinately slow. However,
> this quiescing guarantees that no unexpected interrupts will occur,
> even if the application intentionally calls into the kernel.
>
> Signed-off-by: Chris Metcalf <cmetcalf-d5a29ZRxExrQT0dZR+AlfA@public.gmane.org>
> ---
> arch/tile/kernel/process.c | 9 ++++++
> include/linux/cpu_isolated.h | 24 +++++++++++++++
> include/linux/sched.h | 3 ++
> include/uapi/linux/prctl.h | 5 ++++
> kernel/context_tracking.c | 3 ++
> kernel/sys.c | 8 +++++
> kernel/time/Kconfig | 20 +++++++++++++
> kernel/time/Makefile | 1 +
> kernel/time/cpu_isolated.c | 71 ++++++++++++++++++++++++++++++++++++++++++++
It's not about time :-)
The timer is only a part of the isolation.
Moreover "isolatED" is a state. The filename should reflect the process. "isolatION" would
better fit.
kernel/task_isolation.c maybe or just kernel/isolation.c
I think I prefer the latter because I'm not only interested in that task
hard isolation feature, I would like to also drive all the general isolation
operations from there (workqueue affinity, rcu nocb, ...).
> 9 files changed, 144 insertions(+)
> create mode 100644 include/linux/cpu_isolated.h
> create mode 100644 kernel/time/cpu_isolated.c
>
> diff --git a/arch/tile/kernel/process.c b/arch/tile/kernel/process.c
> index e036c0aa9792..7db6f8386417 100644
> --- a/arch/tile/kernel/process.c
> +++ b/arch/tile/kernel/process.c
> @@ -70,6 +70,15 @@ void arch_cpu_idle(void)
> _cpu_idle();
> }
>
> +#ifdef CONFIG_CPU_ISOLATED
> +void cpu_isolated_wait(void)
> +{
> + set_current_state(TASK_INTERRUPTIBLE);
> + _cpu_idle();
> + set_current_state(TASK_RUNNING);
> +}
I'm still uncomfortable with that. A wake up model could work?
> +#endif
> +
> /*
> * Release a thread_info structure
> */
> diff --git a/include/linux/cpu_isolated.h b/include/linux/cpu_isolated.h
> new file mode 100644
> index 000000000000..a3d17360f7ae
> --- /dev/null
> +++ b/include/linux/cpu_isolated.h
> @@ -0,0 +1,24 @@
> +/*
> + * CPU isolation related global functions
> + */
> +#ifndef _LINUX_CPU_ISOLATED_H
> +#define _LINUX_CPU_ISOLATED_H
> +
> +#include <linux/tick.h>
> +#include <linux/prctl.h>
> +
> +#ifdef CONFIG_CPU_ISOLATED
> +static inline bool is_cpu_isolated(void)
> +{
> + return tick_nohz_full_cpu(smp_processor_id()) &&
> + (current->cpu_isolated_flags & PR_CPU_ISOLATED_ENABLE);
> +}
> +
> +extern void cpu_isolated_enter(void);
> +extern void cpu_isolated_wait(void);
> +#else
> +static inline bool is_cpu_isolated(void) { return false; }
> +static inline void cpu_isolated_enter(void) { }
> +#endif
And all the naming should be about task as well, if we take that task direction.
> +
> +#endif
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index 04b5ada460b4..0bb248385d88 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -1776,6 +1776,9 @@ struct task_struct {
> unsigned long task_state_change;
> #endif
> int pagefault_disabled;
> +#ifdef CONFIG_CPU_ISOLATED
> + unsigned int cpu_isolated_flags;
> +#endif
Can't we add a new flag to tsk->flags? There seem to be some values remaining.
Thanks.
^ permalink raw reply
* Re: [PATCH v5 2/6] cpu_isolated: add initial support
From: Chris Metcalf @ 2015-08-12 18:22 UTC (permalink / raw)
To: Frederic Weisbecker
Cc: Gilad Ben Yossef, Steven Rostedt, Ingo Molnar, Peter Zijlstra,
Andrew Morton, Rik van Riel, Tejun Heo, Thomas Gleixner,
Paul E. McKenney, Christoph Lameter, Viresh Kumar,
Catalin Marinas, Will Deacon, linux-doc-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20150812160020.GG21542@lerouge>
On 08/12/2015 12:00 PM, Frederic Weisbecker wrote:
> On Tue, Jul 28, 2015 at 03:49:36PM -0400, Chris Metcalf wrote:
>> The existing nohz_full mode is designed as a "soft" isolation mode
>> that makes tradeoffs to minimize userspace interruptions while
>> still attempting to avoid overheads in the kernel entry/exit path,
>> to provide 100% kernel semantics, etc.
>>
>> However, some applications require a "hard" commitment from the
>> kernel to avoid interruptions, in particular userspace device
>> driver style applications, such as high-speed networking code.
>>
>> This change introduces a framework to allow applications
>> to elect to have the "hard" semantics as needed, specifying
>> prctl(PR_SET_CPU_ISOLATED, PR_CPU_ISOLATED_ENABLE) to do so.
>> Subsequent commits will add additional flags and additional
>> semantics.
> We are doing this at the process level but the isolation works on
> the CPU scope... Now I wonder if prctl is the right interface.
>
> That said the user is rather interested in isolating a task. The CPU
> being the backend eventually.
>
> For example if the task is migrated by accident, we want it to be
> warned about that. And if the isolation is done on the CPU level
> instead of the task level, this won't happen.
>
> I'm also afraid that the naming clashes with cpu_isolated_map,
> although it could be a subset of it.
>
> So probably in this case we should consider talking about task rather
> than CPU isolation and change naming accordingly (sorry, I know I
> suggested cpu_isolation.c, I guess I had to see the result to realize).
>
> We must sort that out first. Either we consider isolation on the task
> level (and thus the underlying CPU by backend effect) and we use prctl().
> Or we do this on the CPU level and we use a specific syscall or sysfs
> which takes effect on any task in the relevant isolated CPUs.
>
> What do you think?
Yes, definitely task-centric is the right model.
With the original tilegx version of this code, we also checked that
the process had only a single core in its affinity mask, and that the
single core in question was a nohz_full core, before allowing the
"task isolated" mode to take effect. I didn't do that in this round
of patches because it seemed a little silly in that the user could
then immediately reset their affinity to another core and lose the
effect, and it wasn't clear how to handle that: do we return EINVAL
from sched_setaffinity() after enabling the "task isolated" mode?
That seems potentially ugly, maybe standards-violating, etc. So I
didn't bother.
But you could certainly argue for failing prctl() in that case anyway,
as a way to make sure users aren't doing something stupid like calling
the prctl() from a task that's running on a housekeeping core. And
you could even argue for doing some kind of console spew if you try to
migrate a task that is in "task isolation" state - though I suppose if
you migrate it to another isolcpus and nohz_full core, maybe that's
kind of reasonable and doesn't deserve a warning? I'm not sure.
>> The kernel must be built with the new CPU_ISOLATED Kconfig flag
>> to enable this mode, and the kernel booted with an appropriate
>> nohz_full=CPULIST boot argument. The "cpu_isolated" state is then
>> indicated by setting a new task struct field, cpu_isolated_flags,
>> to the value passed by prctl(). When the _ENABLE bit is set for a
>> task, and it is returning to userspace on a nohz_full core, it calls
>> the new cpu_isolated_enter() routine to take additional actions
>> to help the task avoid being interrupted in the future.
>>
>> Initially, there are only three actions taken. First, the
>> task calls lru_add_drain() to prevent being interrupted by a
>> subsequent lru_add_drain_all() call on another core. Then, it calls
>> quiet_vmstat() to quieten the vmstat worker to avoid a follow-on
>> interrupt. Finally, the code checks for pending timer interrupts
>> and quiesces until they are no longer pending. As a result, sys
>> calls (and page faults, etc.) can be inordinately slow. However,
>> this quiescing guarantees that no unexpected interrupts will occur,
>> even if the application intentionally calls into the kernel.
>>
>> Signed-off-by: Chris Metcalf <cmetcalf-d5a29ZRxExrQT0dZR+AlfA@public.gmane.org>
>> ---
>> arch/tile/kernel/process.c | 9 ++++++
>> include/linux/cpu_isolated.h | 24 +++++++++++++++
>> include/linux/sched.h | 3 ++
>> include/uapi/linux/prctl.h | 5 ++++
>> kernel/context_tracking.c | 3 ++
>> kernel/sys.c | 8 +++++
>> kernel/time/Kconfig | 20 +++++++++++++
>> kernel/time/Makefile | 1 +
>> kernel/time/cpu_isolated.c | 71 ++++++++++++++++++++++++++++++++++++++++++++
> It's not about time :-)
>
> The timer is only a part of the isolation.
>
> Moreover "isolatED" is a state. The filename should reflect the process. "isolatION" would
> better fit.
>
> kernel/task_isolation.c maybe or just kernel/isolation.c
>
> I think I prefer the latter because I'm not only interested in that task
> hard isolation feature, I would like to also drive all the general isolation
> operations from there (workqueue affinity, rcu nocb, ...).
That's reasonable, but I think the "task isolation" naming is probably
better for all the stuff that we're doing in this patch. In other words,
we probably should use "task_isolation" as the prefix for symbols
names and API names, even if we put the code in kernel/isolation.c
for now in anticipation of non-task isolation being added later.
I think my instinct would still be to call it kernel/task_isolation.c
until we actually add some non-task isolation, and at that point we
can decide if it makes sense to rename the file, or put the new
code somewhere else, but I'm OK with doing it the way I described
in the previous paragraph if you think it's better.
>> +#ifdef CONFIG_CPU_ISOLATED
>> +void cpu_isolated_wait(void)
>> +{
>> + set_current_state(TASK_INTERRUPTIBLE);
>> + _cpu_idle();
>> + set_current_state(TASK_RUNNING);
>> +}
> I'm still uncomfortable with that. A wake up model could work?
I don't know exactly what you have in mind. The theory is that
at this point we're ready to return to user space and we're just
waiting for a timer tick that is guaranteed to arrive, since there
is something pending for the timer.
And, this is an arch-specific method anyway; the generic method
is actually checking to see if a signal has been delivered,
scheduling is needed, etc., each time around the loop, so if
you're not sure your architecture will do the right thing, just
don't provide a method that idles while waiting. For tilegx I'm
sure it works correctly, so I'm OK providing that method.
>> +extern void cpu_isolated_enter(void);
>> +extern void cpu_isolated_wait(void);
>> +#else
>> +static inline bool is_cpu_isolated(void) { return false; }
>> +static inline void cpu_isolated_enter(void) { }
>> +#endif
> And all the naming should be about task as well, if we take that task direction.
As discussed above, probably task_isolation_enter(), etc.
>> +
>> +#endif
>> diff --git a/include/linux/sched.h b/include/linux/sched.h
>> index 04b5ada460b4..0bb248385d88 100644
>> --- a/include/linux/sched.h
>> +++ b/include/linux/sched.h
>> @@ -1776,6 +1776,9 @@ struct task_struct {
>> unsigned long task_state_change;
>> #endif
>> int pagefault_disabled;
>> +#ifdef CONFIG_CPU_ISOLATED
>> + unsigned int cpu_isolated_flags;
>> +#endif
> Can't we add a new flag to tsk->flags? There seem to be some values remaining.
Yeah, I thought of that, but it seems like a pretty scarce resource,
and I wasn't sure it was the right thing to do. Also, I'm not actually
sure why the lowest two bits aren't apparently being used; looks
like PF_EXITING (0x4) is the first bit used. And there are only three
more bits higher up in the word that are not assigned.
Also, right now we are allowing users to customize the signal delivered
for STRICT violation, and that signal value is stored in the
cpu_isolated_flags word as well, so we really don't have room in
tsk->flags for all of that anyway.
--
Chris Metcalf, EZChip Semiconductor
http://www.ezchip.com
^ permalink raw reply
* Re: [CFT][PATCH 09/10] sysfs: Create mountpoints with sysfs_create_empty_dir
From: Tejun Heo @ 2015-08-12 19:15 UTC (permalink / raw)
To: Eric W. Biederman
Cc: Linux Containers, LINUXFS-ML, Linux API, Serge E. Hallyn,
Andy Lutomirski, Richard Weinberger, Kenton Varda,
Michael Kerrisk-manpages, Stéphane Graber, Eric Windisch,
Greg Kroah-Hartman
In-Reply-To: <871tf9b19v.fsf-JOvCrm2gF+uungPnsOpG7nhyD016LWXt@public.gmane.org>
Hello, Eric.
On Tue, Aug 11, 2015 at 11:04:28PM -0500, Eric W. Biederman wrote:
> ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org (Eric W. Biederman) writes:
>
> > ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org (Eric W. Biederman) writes:
> >
> >> I just went and attempted to reproduce this, and on RHEL6 workstation
> >> (aka my work laptop), using the todays 4.2.0-rc6+ aka
> >> edf15b4d4b01b565cb5f4fd2e2d08940b9f92e2f and all of the mounts in
> >> /proc/self/mounts are the same between 4.2.0-rc6 and the RHEL6 stock
> >> 2.6.32-504.30.3.el6.x86_64, including the cgroups mounted on /cgroup.
> >
> > I built a few more kernels just to see if this was some weird backport
> > thing. The kernels 3.10.86, 3.14.58, 3.18.20, and 4.1.5 all boot and
> > mount their cgroup filesystems just fine. Granted I kept having to
> > smack the memory cgroup into being compiled in as the config options
> > kept changing but otherwise I have not seen any problems.
> >
> > So I am very surprised you are having problems.
>
> Although I guess I could have saved myself some time by noticing that
> 4.1.5 was the only one of the kernels with the change backported into
> it. *Shrug*
>
> I don't see the problem and I don't know where to look to see why you
> are having problems.
lol, this wasn't upstart but an internal tool which sets up a custom
cgroup hierarchy and the problem was the size of the directory inode
reported by stat(2). It's kinda hilarious but that's what the tool
was depending on to tell whether tmpfs is mounted on /sys/fs/cgroup or
not. A kernfs directory reports zero as its inode size while tmpfs
reports some non-zero number, so the tool did stat(2) on
/sys/fs/cgroup and mounted tmpfs iff size is zero to avoid mounting
tmpfs multiple times. Now, make_empty_dir_inode() sets i_size to 2
and the tool thinks that tmpfs is already mounted there.
It's an icky behavior but it'd be better to maintain the original
behavior. We should be able to set size to zero for empty dirs,
right?
Thanks.
--
tejun
^ permalink raw reply
* Re: [CFT][PATCH 09/10] sysfs: Create mountpoints with sysfs_create_empty_dir
From: Tejun Heo @ 2015-08-12 20:00 UTC (permalink / raw)
To: Eric W. Biederman
Cc: Andy Lutomirski, Linux Containers, Linux FS Devel, Linux API,
Serge E. Hallyn, Richard Weinberger, Kenton Varda,
Michael Kerrisk-manpages, Stéphane Graber, Eric Windisch,
Greg Kroah-Hartman
In-Reply-To: <87mvxxcogp.fsf-JOvCrm2gF+uungPnsOpG7nhyD016LWXt@public.gmane.org>
On Tue, Aug 11, 2015 at 07:58:14PM -0500, Eric W. Biederman wrote:
> Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org> writes:
>
> > On Tue, Aug 11, 2015 at 11:57 AM, Eric W. Biederman
> > <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org> wrote:
> >>
> >> *Boggle*
> >>
> >> The only time this should prevent anything is when in a container when
> >> you are not global root. And then only mounting sysfs should be
> >> affected.
> >
> > Before:
> >
> > open("/sys/kernel/debug/asdf", O_WRONLY|O_CREAT|O_NOCTTY|O_NONBLOCK,
> > 0666) = -1 EACCES (Permission denied)
> >
> >
> > After:
> >
> > open("/sys/kernel/debug/asdf", O_WRONLY|O_CREAT|O_NOCTTY|O_NONBLOCK,
> > 0666) = -1 ENOENT (No such file or directory)
> >
> > Something broke. I don't know whether CentOS cares about that change,
> > but there could be other odd side effects.
>
> Thanks for pointing this out. I don't know if broke is quite the right
> word for a change in error codes on lookup failure, but I agree it is a
> difference that could have affected something.
>
> The behavior of empty proc dirs actually return -ENOENT in this
> situation and so it is a little fuzzy about which is the best behavior
> to use.
>
> Creativing a negative dentry and and then letting vfs_create fail may be
> the better way to go.
>
> Negative dentries are weird enough that I would prefer not to have code
> that creates negative dentries. They could easily be a lurking trap
> for some filesystems dentry operations.
>
> The patch below is enough to change the error code if someone who can
> reproduce this wants to try this.
>
> Eric
>
> diff --gdiff --git a/fs/libfs.c b/fs/libfs.c
> index 102edfd39000..3a452a485cbf 100644
> --- a/fs/libfs.c
> +++ b/fs/libfs.c
> @@ -1109,7 +1109,7 @@ EXPORT_SYMBOL(simple_symlink_inode_operations);
> */
> static struct dentry *empty_dir_lookup(struct inode *dir, struct dentry *dentry, unsigned int flags)
> {
> - return ERR_PTR(-ENOENT);
> + return NULL;
And let's please restore this too. Sentiments about negative dentries
aside, It's outright wrong to report -ENOENT on creat.
Thanks.
--
tejun
^ permalink raw reply
* [PATCH] fs: Set the size of empty dirs to 0.
From: Eric W. Biederman @ 2015-08-12 20:07 UTC (permalink / raw)
To: Linux Containers
Cc: Linux API, Greg Kroah-Hartman, Andy Lutomirski, Kenton Varda,
Michael Kerrisk-manpages, Richard Weinberger, LINUXFS-ML,
Tejun Heo
In-Reply-To: <20150812191515.GA4496-qYNAdHglDFBN0TnZuCh8vA@public.gmane.org>
Before the make_empty_dir_inode calls were introduce into proc, sysfs,
and sysctl those directories when stated reported an i_size of 0.
make_empty_dir_inode started reporting an i_size of 2. At least one
userspace application depended on stat returning i_size of 0. So modify
make_empty_dir_inode to cause an i_size of 0 to be reported for these
directories.
Cc: stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Reproted-by: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Signed-off-by: "Eric W. Biederman" <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
---
I have tested this and will queue this up shortly.
fs/libfs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/libfs.c b/fs/libfs.c
index 102edfd39000..c7cbfb092e94 100644
--- a/fs/libfs.c
+++ b/fs/libfs.c
@@ -1185,7 +1185,7 @@ void make_empty_dir_inode(struct inode *inode)
inode->i_uid = GLOBAL_ROOT_UID;
inode->i_gid = GLOBAL_ROOT_GID;
inode->i_rdev = 0;
- inode->i_size = 2;
+ inode->i_size = 0;
inode->i_blkbits = PAGE_SHIFT;
inode->i_blocks = 0;
--
2.2.1
^ permalink raw reply related
* Re: [PATCH] fs: Set the size of empty dirs to 0.
From: Tejun Heo @ 2015-08-12 20:18 UTC (permalink / raw)
To: Eric W. Biederman
Cc: Linux API, Linux Containers, Greg Kroah-Hartman, Andy Lutomirski,
Kenton Varda, Michael Kerrisk-manpages, Richard Weinberger,
LINUXFS-ML
In-Reply-To: <87mvxw46fc.fsf_-_-JOvCrm2gF+uungPnsOpG7nhyD016LWXt@public.gmane.org>
On Wed, Aug 12, 2015 at 03:07:19PM -0500, Eric W. Biederman wrote:
>
> Before the make_empty_dir_inode calls were introduce into proc, sysfs,
> and sysctl those directories when stated reported an i_size of 0.
> make_empty_dir_inode started reporting an i_size of 2. At least one
> userspace application depended on stat returning i_size of 0. So modify
> make_empty_dir_inode to cause an i_size of 0 to be reported for these
> directories.
>
> Cc: stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Reproted-by: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
^^^
> Signed-off-by: "Eric W. Biederman" <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
Acked-by: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Thanks.
--
tejun
^ permalink raw reply
* Re: [CFT][PATCH 09/10] sysfs: Create mountpoints with sysfs_create_empty_dir
From: Eric W. Biederman @ 2015-08-12 20:27 UTC (permalink / raw)
To: Tejun Heo
Cc: Andy Lutomirski, Linux Containers, Linux FS Devel, Linux API,
Serge E. Hallyn, Richard Weinberger, Kenton Varda,
Michael Kerrisk-manpages, Stéphane Graber, Eric Windisch,
Greg Kroah-Hartman
In-Reply-To: <20150812200035.GB4496@mtj.duckdns.org>
Tejun Heo <tj@kernel.org> writes:
> On Tue, Aug 11, 2015 at 07:58:14PM -0500, Eric W. Biederman wrote:
>> Andy Lutomirski <luto@amacapital.net> writes:
>>
>> > On Tue, Aug 11, 2015 at 11:57 AM, Eric W. Biederman
>> > <ebiederm@xmission.com> wrote:
>> >>
>> >> *Boggle*
>> >>
>> >> The only time this should prevent anything is when in a container when
>> >> you are not global root. And then only mounting sysfs should be
>> >> affected.
>> >
>> > Before:
>> >
>> > open("/sys/kernel/debug/asdf", O_WRONLY|O_CREAT|O_NOCTTY|O_NONBLOCK,
>> > 0666) = -1 EACCES (Permission denied)
>> >
>> >
>> > After:
>> >
>> > open("/sys/kernel/debug/asdf", O_WRONLY|O_CREAT|O_NOCTTY|O_NONBLOCK,
>> > 0666) = -1 ENOENT (No such file or directory)
>> >
>> > Something broke. I don't know whether CentOS cares about that change,
>> > but there could be other odd side effects.
>>
>> Thanks for pointing this out. I don't know if broke is quite the right
>> word for a change in error codes on lookup failure, but I agree it is a
>> difference that could have affected something.
>>
>> The behavior of empty proc dirs actually return -ENOENT in this
>> situation and so it is a little fuzzy about which is the best behavior
>> to use.
>>
>> Creativing a negative dentry and and then letting vfs_create fail may be
>> the better way to go.
>>
>> Negative dentries are weird enough that I would prefer not to have code
>> that creates negative dentries. They could easily be a lurking trap
>> for some filesystems dentry operations.
>>
>> The patch below is enough to change the error code if someone who can
>> reproduce this wants to try this.
>>
>> Eric
>>
>> diff --gdiff --git a/fs/libfs.c b/fs/libfs.c
>> index 102edfd39000..3a452a485cbf 100644
>> --- a/fs/libfs.c
>> +++ b/fs/libfs.c
>> @@ -1109,7 +1109,7 @@ EXPORT_SYMBOL(simple_symlink_inode_operations);
>> */
>> static struct dentry *empty_dir_lookup(struct inode *dir, struct dentry *dentry, unsigned int flags)
>> {
>> - return ERR_PTR(-ENOENT);
>> + return NULL;
>
> And let's please restore this too. Sentiments about negative dentries
> aside, It's outright wrong to report -ENOENT on creat.
proc has always reported -ENOENT. sysfs is the odd one out.
I am not completely certain that trivial patch above, does not introduce
a leak, a NULL pointer dereference or something else nasty when the code
is hit.
So far it seems that no one cares. And since the change is brittle I am
not inclined to mess with it this week, as I have other demands on my
limited review bandwidth right now.
Eric
^ permalink raw reply
* Re: [CFT][PATCH 09/10] sysfs: Create mountpoints with sysfs_create_empty_dir
From: Tejun Heo @ 2015-08-12 21:05 UTC (permalink / raw)
To: Eric W. Biederman
Cc: Linux API, Linux Containers, Greg Kroah-Hartman, Andy Lutomirski,
Kenton Varda, Michael Kerrisk-manpages, Richard Weinberger,
Linux FS Devel
In-Reply-To: <87r3n82qxd.fsf-JOvCrm2gF+uungPnsOpG7nhyD016LWXt@public.gmane.org>
Hello,
On Wed, Aug 12, 2015 at 03:27:26PM -0500, Eric W. Biederman wrote:
> proc has always reported -ENOENT. sysfs is the odd one out.
Hmm... open(2) is clear about failure modes and ENOENT doesn't fit the
bill here. Maintaining the behavior for proc for backward
compatibility is fine but I don't think it's appropriate to change
behaviors on other filesystems which were behaving correctly
especially through changes which got routed through -stable.
ENOENT O_CREAT is not set and the named file does not exist. Or, a directory component in pathname does not exist or is a dangling symbolic link.
ENOENT pathname refers to a nonexistent directory, O_TMPFILE and one of O_WRONLY or O_RDWR were specified in flags, but this kernel version does not provide the O_TMPFILE
functionality.
> I am not completely certain that trivial patch above, does not introduce
> a leak, a NULL pointer dereference or something else nasty when the code
> is hit.
>
> So far it seems that no one cares. And since the change is brittle I am
> not inclined to mess with it this week, as I have other demands on my
> limited review bandwidth right now.
Sure, it isn't "today" urgent but let's please restore the original
behavior before the new behavior gets too widespread.
Thanks.
--
tejun
^ permalink raw reply
* [PATCHv4 0/3] fs: add O_BENEATH flag to openat(2)
From: David Drysdale @ 2015-08-13 9:32 UTC (permalink / raw)
To: linux-kernel-u79uwXL29TY76Z2rM5mHXA, Alexander Viro, Kees Cook,
Eric W. Biederman
Cc: Greg Kroah-Hartman, Meredydd Luff, Will Drewry,
Jorge Lucangeli Obes, Ricky Zhou, Lee Campbell, Julien Tinnes,
Mike Depinet, James Morris, Andy Lutomirski, Paolo Bonzini,
Paul Moore, Christoph Hellwig, Michael Kerrisk, Dave Chinner,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-arch-u79uwXL29TY76Z2rM5mHXA,
linux-security-module-u79uwXL29TY76Z2rM5mHXA,
linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
fstests-u79uwXL29TY76Z2rM5mHXA, David Drysdale
A couple of questions with this iteration:
- Should we create a new errno (say ENOTBENEATH) for this policing, to
make it easier to distinguish this case from other EPERM failures?
(The FreeBSD implementation is considering this approach.)
- Al, does the code look OK for (in particular) integrating with the
shiny new re-worked fs/namei.c code?
Thanks.
This change adds a new O_BENEATH flag for openat(2) which restricts the
provided path, rejecting (with -EPERM) paths that are not beneath
the provided dfd.
This functionality was originally implemented as part of the internals
of the Capsicum security framework, which is available in FreeBSD 10.x
and which has previously had a Linux kernel port proposed [1].
However, as this behaviour is potentially useful as an independent feature,
this change exposes it via an openat(2) flag. (This variant was not
originally exposed in FreeBSD, but is currently being proposed there
too [2].)
Various folks from Chrome[OS] have indicated an interest in having this
functionality -- when combined with a seccomp filter it allows a directory
to be more safely accessed by a sandboxed process. Other folk have also
expressed an interest [3].
[1] https://lkml.org/lkml/2014/7/25/426
[2] https://reviews.freebsd.org/D2808
[3] https://groups.google.com/d/msg/capnproto/sKpzanYNZmQ/T9IbJIB-rqQJ
Changes since v3:
- Merge up to v4.2-rc6
- Reinstate local selftests (I'll send xfstest changes separately
if and when this is merged)
- Pull in common selftests makefile
Changes since v2:
- Move tests into xfstests [Dave Chinner, with thanks for feedback
on initial version]
- Merge up to v4.0-rc3 & latest man-pages
Changes since v1:
- Don't needlessly duplicate flags [Al Viro]
- Use EPERM rather than EACCES as error code [Paolo Bonzini]
- Disallow nd_jump_link for O_BENEATH [Al Viro/Andy Lutomirski]
- Add test of a jumped symlink (/proc/self/root)
Changes since the version included in the Capsicum v2 patchset:
- Add tests of normal symlinks
- Fix man-page typo
- Update patch to 3.17
Changes from v1 to v2 of Capsicum patchset:
- renamed O_BENEATH_ONLY to O_BENEATH [Christoph Hellwig]
David Drysdale (2):
fs: add O_BENEATH flag to openat(2)
selftests: Add test of O_BENEATH & openat(2)
arch/alpha/include/uapi/asm/fcntl.h | 1 +
arch/parisc/include/uapi/asm/fcntl.h | 1 +
arch/sparc/include/uapi/asm/fcntl.h | 1 +
fs/fcntl.c | 4 +-
fs/namei.c | 12 +-
fs/open.c | 4 +-
fs/proc/base.c | 4 +-
fs/proc/namespaces.c | 8 +-
include/linux/namei.h | 3 +-
include/uapi/asm-generic/fcntl.h | 4 +
tools/testing/selftests/Makefile | 1 +
tools/testing/selftests/openat/.gitignore | 4 +
tools/testing/selftests/openat/Makefile | 29 ++++
tools/testing/selftests/openat/openat.c | 258 ++++++++++++++++++++++++++++++
14 files changed, 326 insertions(+), 8 deletions(-)
create mode 100644 tools/testing/selftests/openat/.gitignore
create mode 100644 tools/testing/selftests/openat/Makefile
create mode 100644 tools/testing/selftests/openat/openat.c
--
1.9.1
^ permalink raw reply
* [PATCHv4 1/3] fs: add O_BENEATH flag to openat(2)
From: David Drysdale @ 2015-08-13 9:32 UTC (permalink / raw)
To: linux-kernel-u79uwXL29TY76Z2rM5mHXA, Alexander Viro, Kees Cook,
Eric W. Biederman
Cc: Greg Kroah-Hartman, Meredydd Luff, Will Drewry,
Jorge Lucangeli Obes, Ricky Zhou, Lee Campbell, Julien Tinnes,
Mike Depinet, James Morris, Andy Lutomirski, Paolo Bonzini,
Paul Moore, Christoph Hellwig, Michael Kerrisk, Dave Chinner,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-arch-u79uwXL29TY76Z2rM5mHXA,
linux-security-module-u79uwXL29TY76Z2rM5mHXA,
linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
fstests-u79uwXL29TY76Z2rM5mHXA, David Drysdale
In-Reply-To: <1439458366-8223-1-git-send-email-drysdale-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
Add a new O_BENEATH flag for openat(2) which restricts the
provided path, rejecting (with -EPERM) paths that are not beneath
the provided dfd. In particular, reject:
- paths that contain .. components
- paths that begin with /
- symlinks that have paths as above.
Also disallow use of nd_jump_link() for following symlinks without
path expansion, when O_BENEATH is set.
Signed-off-by: David Drysdale <drysdale-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
---
arch/alpha/include/uapi/asm/fcntl.h | 1 +
arch/parisc/include/uapi/asm/fcntl.h | 1 +
arch/sparc/include/uapi/asm/fcntl.h | 1 +
fs/fcntl.c | 4 ++--
fs/namei.c | 12 +++++++++++-
fs/open.c | 4 +++-
fs/proc/base.c | 4 +++-
fs/proc/namespaces.c | 8 ++++++--
include/linux/namei.h | 3 ++-
include/uapi/asm-generic/fcntl.h | 4 ++++
10 files changed, 34 insertions(+), 8 deletions(-)
diff --git a/arch/alpha/include/uapi/asm/fcntl.h b/arch/alpha/include/uapi/asm/fcntl.h
index 09f49a6b87d1..76a87038d2c1 100644
--- a/arch/alpha/include/uapi/asm/fcntl.h
+++ b/arch/alpha/include/uapi/asm/fcntl.h
@@ -33,6 +33,7 @@
#define O_PATH 040000000
#define __O_TMPFILE 0100000000
+#define O_BENEATH 0200000000 /* no / or .. in openat path */
#define F_GETLK 7
#define F_SETLK 8
diff --git a/arch/parisc/include/uapi/asm/fcntl.h b/arch/parisc/include/uapi/asm/fcntl.h
index 34a46cbc76ed..3adadf72f929 100644
--- a/arch/parisc/include/uapi/asm/fcntl.h
+++ b/arch/parisc/include/uapi/asm/fcntl.h
@@ -21,6 +21,7 @@
#define O_PATH 020000000
#define __O_TMPFILE 040000000
+#define O_BENEATH 080000000 /* no / or .. in openat path */
#define F_GETLK64 8
#define F_SETLK64 9
diff --git a/arch/sparc/include/uapi/asm/fcntl.h b/arch/sparc/include/uapi/asm/fcntl.h
index 7e8ace5bf760..ea38f0bd6cec 100644
--- a/arch/sparc/include/uapi/asm/fcntl.h
+++ b/arch/sparc/include/uapi/asm/fcntl.h
@@ -36,6 +36,7 @@
#define O_PATH 0x1000000
#define __O_TMPFILE 0x2000000
+#define O_BENEATH 0x4000000 /* no / or .. in openat path */
#define F_GETOWN 5 /* for sockets. */
#define F_SETOWN 6 /* for sockets. */
diff --git a/fs/fcntl.c b/fs/fcntl.c
index ee85cd4e136a..3169693e9390 100644
--- a/fs/fcntl.c
+++ b/fs/fcntl.c
@@ -740,7 +740,7 @@ static int __init fcntl_init(void)
* Exceptions: O_NONBLOCK is a two bit define on parisc; O_NDELAY
* is defined as O_NONBLOCK on some platforms and not on others.
*/
- BUILD_BUG_ON(21 - 1 /* for O_RDONLY being 0 */ != HWEIGHT32(
+ BUILD_BUG_ON(22 - 1 /* for O_RDONLY being 0 */ != HWEIGHT32(
O_RDONLY | O_WRONLY | O_RDWR |
O_CREAT | O_EXCL | O_NOCTTY |
O_TRUNC | O_APPEND | /* O_NONBLOCK | */
@@ -748,7 +748,7 @@ static int __init fcntl_init(void)
O_DIRECT | O_LARGEFILE | O_DIRECTORY |
O_NOFOLLOW | O_NOATIME | O_CLOEXEC |
__FMODE_EXEC | O_PATH | __O_TMPFILE |
- __FMODE_NONOTIFY
+ __FMODE_NONOTIFY| O_BENEATH
));
fasync_cache = kmem_cache_create("fasync_cache",
diff --git a/fs/namei.c b/fs/namei.c
index fbbcf0993312..978f07d91a11 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -827,14 +827,18 @@ static inline void path_to_nameidata(const struct path *path,
* Helper to directly jump to a known parsed path from ->follow_link,
* caller must have taken a reference to path beforehand.
*/
-void nd_jump_link(struct path *path)
+int nd_jump_link(struct path *path)
{
struct nameidata *nd = current->nameidata;
+
+ if (nd->flags & LOOKUP_BENEATH)
+ return -EPERM;
path_put(&nd->path);
nd->path = *path;
nd->inode = nd->path.dentry->d_inode;
nd->flags |= LOOKUP_JUMPED;
+ return 0;
}
static inline void put_link(struct nameidata *nd)
@@ -1000,6 +1004,8 @@ const char *get_link(struct nameidata *nd)
}
}
if (*res == '/') {
+ if (nd->flags & LOOKUP_BENEATH)
+ return ERR_PTR(-EPERM);
if (nd->flags & LOOKUP_RCU) {
struct dentry *d;
if (!nd->root.mnt)
@@ -1888,6 +1894,8 @@ static int link_path_walk(const char *name, struct nameidata *nd)
if (name[0] == '.') switch (hashlen_len(hash_len)) {
case 2:
if (name[1] == '.') {
+ if (nd->flags & LOOKUP_BENEATH)
+ return -EPERM;
type = LAST_DOTDOT;
nd->flags |= LOOKUP_JUMPED;
}
@@ -2000,6 +2008,8 @@ static const char *path_init(struct nameidata *nd, unsigned flags)
nd->m_seq = read_seqbegin(&mount_lock);
if (*s == '/') {
+ if (flags & LOOKUP_BENEATH)
+ return ERR_PTR(-EPERM);
if (flags & LOOKUP_RCU) {
rcu_read_lock();
set_root_rcu(nd);
diff --git a/fs/open.c b/fs/open.c
index e33dab287fa0..29208cd307f7 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -917,7 +917,7 @@ static inline int build_open_flags(int flags, umode_t mode, struct open_flags *o
* If we have O_PATH in the open flag. Then we
* cannot have anything other than the below set of flags
*/
- flags &= O_DIRECTORY | O_NOFOLLOW | O_PATH;
+ flags &= O_DIRECTORY | O_NOFOLLOW | O_PATH | O_BENEATH;
acc_mode = 0;
} else {
acc_mode = MAY_OPEN | ACC_MODE(flags);
@@ -948,6 +948,8 @@ static inline int build_open_flags(int flags, umode_t mode, struct open_flags *o
lookup_flags |= LOOKUP_DIRECTORY;
if (!(flags & O_NOFOLLOW))
lookup_flags |= LOOKUP_FOLLOW;
+ if (flags & O_BENEATH)
+ lookup_flags |= LOOKUP_BENEATH;
op->lookup_flags = lookup_flags;
return 0;
}
diff --git a/fs/proc/base.c b/fs/proc/base.c
index aa50d1ac28fc..281f7a8d8060 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -1589,7 +1589,9 @@ static const char *proc_pid_follow_link(struct dentry *dentry, void **cookie)
if (error)
goto out;
- nd_jump_link(&path);
+ error = nd_jump_link(&path);
+ if (error)
+ goto out;
return NULL;
out:
return ERR_PTR(error);
diff --git a/fs/proc/namespaces.c b/fs/proc/namespaces.c
index f6e8354b8cea..beb5aa3ad38c 100644
--- a/fs/proc/namespaces.c
+++ b/fs/proc/namespaces.c
@@ -36,6 +36,7 @@ static const char *proc_ns_follow_link(struct dentry *dentry, void **cookie)
const struct proc_ns_operations *ns_ops = PROC_I(inode)->ns_ops;
struct task_struct *task;
struct path ns_path;
+ int err;
void *error = ERR_PTR(-EACCES);
task = get_proc_task(inode);
@@ -44,8 +45,11 @@ static const char *proc_ns_follow_link(struct dentry *dentry, void **cookie)
if (ptrace_may_access(task, PTRACE_MODE_READ)) {
error = ns_get_path(&ns_path, task, ns_ops);
- if (!error)
- nd_jump_link(&ns_path);
+ if (!error) {
+ err = nd_jump_link(&ns_path);
+ if (err)
+ error = ERR_PTR(err);
+ }
}
put_task_struct(task);
return error;
diff --git a/include/linux/namei.h b/include/linux/namei.h
index d8c6334cd150..a5a262c85e49 100644
--- a/include/linux/namei.h
+++ b/include/linux/namei.h
@@ -27,6 +27,7 @@ enum {LAST_NORM, LAST_ROOT, LAST_DOT, LAST_DOTDOT, LAST_BIND};
#define LOOKUP_FOLLOW 0x0001
#define LOOKUP_DIRECTORY 0x0002
#define LOOKUP_AUTOMOUNT 0x0004
+#define LOOKUP_BENEATH 0x0008
#define LOOKUP_PARENT 0x0010
#define LOOKUP_REVAL 0x0020
@@ -85,7 +86,7 @@ extern int follow_up(struct path *);
extern struct dentry *lock_rename(struct dentry *, struct dentry *);
extern void unlock_rename(struct dentry *, struct dentry *);
-extern void nd_jump_link(struct path *path);
+extern int nd_jump_link(struct path *path);
static inline void nd_terminate_link(void *name, size_t len, size_t maxlen)
{
diff --git a/include/uapi/asm-generic/fcntl.h b/include/uapi/asm-generic/fcntl.h
index e063effe0cc1..4542bc6a2950 100644
--- a/include/uapi/asm-generic/fcntl.h
+++ b/include/uapi/asm-generic/fcntl.h
@@ -92,6 +92,10 @@
#define O_TMPFILE (__O_TMPFILE | O_DIRECTORY)
#define O_TMPFILE_MASK (__O_TMPFILE | O_DIRECTORY | O_CREAT)
+#ifndef O_BENEATH
+#define O_BENEATH 040000000 /* no / or .. in openat path */
+#endif
+
#ifndef O_NDELAY
#define O_NDELAY O_NONBLOCK
#endif
--
1.9.1
^ permalink raw reply related
* [PATCHv4 2/3] selftests: Add test of O_BENEATH & openat(2)
From: David Drysdale @ 2015-08-13 9:32 UTC (permalink / raw)
To: linux-kernel, Alexander Viro, Kees Cook, Eric W. Biederman
Cc: Greg Kroah-Hartman, Meredydd Luff, Will Drewry,
Jorge Lucangeli Obes, Ricky Zhou, Lee Campbell, Julien Tinnes,
Mike Depinet, James Morris, Andy Lutomirski, Paolo Bonzini,
Paul Moore, Christoph Hellwig, Michael Kerrisk, Dave Chinner,
linux-api, linux-arch, linux-security-module, linux-fsdevel,
fstests, David Drysdale
In-Reply-To: <1439458366-8223-1-git-send-email-drysdale@google.com>
Add simple tests of openat(2) variations, including examples that
check the new O_BENEATH flag.
Signed-off-by: David Drysdale <drysdale@google.com>
---
tools/testing/selftests/Makefile | 1 +
tools/testing/selftests/openat/.gitignore | 4 +
tools/testing/selftests/openat/Makefile | 29 ++++
tools/testing/selftests/openat/openat.c | 258 ++++++++++++++++++++++++++++++
4 files changed, 292 insertions(+)
create mode 100644 tools/testing/selftests/openat/.gitignore
create mode 100644 tools/testing/selftests/openat/Makefile
create mode 100644 tools/testing/selftests/openat/openat.c
diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index 24ae9e829e9a..606f8df5c8aa 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -11,6 +11,7 @@ TARGETS += memory-hotplug
TARGETS += mount
TARGETS += mqueue
TARGETS += net
+TARGETS += openat
TARGETS += powerpc
TARGETS += ptrace
TARGETS += seccomp
diff --git a/tools/testing/selftests/openat/.gitignore b/tools/testing/selftests/openat/.gitignore
new file mode 100644
index 000000000000..835b2dcd8678
--- /dev/null
+++ b/tools/testing/selftests/openat/.gitignore
@@ -0,0 +1,4 @@
+openat
+subdir
+topfile
+symlinkdown
\ No newline at end of file
diff --git a/tools/testing/selftests/openat/Makefile b/tools/testing/selftests/openat/Makefile
new file mode 100644
index 000000000000..73f80428b6a5
--- /dev/null
+++ b/tools/testing/selftests/openat/Makefile
@@ -0,0 +1,29 @@
+CFLAGS = -Wall
+BINARIES = openat
+DEPS = subdir topfile symlinkdown subdir/bottomfile subdir/symlinkup subdir/symlinkout subdir/symlinkin
+all: $(BINARIES) $(DEPS)
+
+subdir:
+ mkdir -p subdir
+topfile:
+ echo 0123456789 > $@
+subdir/bottomfile: | subdir
+ echo 0123456789 > $@
+subdir/symlinkup: | subdir
+ ln -s ../topfile $@
+subdir/symlinkout: | subdir
+ ln -s /etc/passwd $@
+subdir/symlinkin: | subdir
+ ln -s bottomfile $@
+symlinkdown:
+ ln -s subdir/bottomfile $@
+%: %.c
+ $(CC) $(CFLAGS) -o $@ $^
+
+TEST_PROGS := openat
+TEST_FILES := $(DEPS)
+
+include ../lib.mk
+
+clean:
+ rm -rf $(BINARIES) $(DEPS)
diff --git a/tools/testing/selftests/openat/openat.c b/tools/testing/selftests/openat/openat.c
new file mode 100644
index 000000000000..bc2c4b02a091
--- /dev/null
+++ b/tools/testing/selftests/openat/openat.c
@@ -0,0 +1,258 @@
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <sys/syscall.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <string.h>
+#include <errno.h>
+
+#include <linux/fcntl.h>
+
+/* Bypass glibc */
+static int openat_(int dirfd, const char *pathname, int flags)
+{
+ return syscall(__NR_openat, dirfd, pathname, flags);
+}
+static int open_(const char *pathname, int flags)
+{
+ return syscall(__NR_open, pathname, flags);
+}
+
+static int openat_or_die(int dfd, const char *path, int flags)
+{
+ int fd = openat_(dfd, path, flags);
+
+ if (fd < 0) {
+ printf("Failed to openat(%d, '%s'); "
+ "check prerequisites are available\n", dfd, path);
+ exit(1);
+ }
+ return fd;
+}
+
+static int check_fd(int fd)
+{
+ int rc;
+ struct stat info;
+ char buffer[4];
+
+ if (fd < 0) {
+ printf("[FAIL]: openat() failed, rc=%d errno=%d (%s)\n",
+ fd, errno, strerror(errno));
+ return 1;
+ }
+ if (fstat(fd, &info) != 0) {
+ printf("[FAIL]: fstat() failed, rc=%d errno=%d (%s)\n",
+ fd, errno, strerror(errno));
+ return 1;
+ }
+ if (!S_ISDIR(info.st_mode)) {
+ errno = 0;
+ rc = read(fd, buffer, sizeof(buffer));
+ if (rc < 0) {
+ printf("[FAIL]: read() failed, rc=%d errno=%d (%s)\n",
+ rc, errno, strerror(errno));
+ return 1;
+ }
+ }
+ close(fd);
+ printf("[OK]\n");
+ return 0;
+}
+
+static int check_openat(int dfd, const char *path, int flags)
+{
+ int fd;
+
+ errno = 0;
+ printf("Check success of openat(%d, '%s', %x)... ",
+ dfd, path?:"(null)", flags);
+ fd = openat_(dfd, path, flags);
+ return check_fd(fd);
+}
+
+static int check_open(const char *path, int flags)
+{
+ int fd;
+
+ errno = 0;
+ printf("Check success of open('%s', %x)... ", path?:"(null)", flags);
+ fd = open_(path, flags);
+ return check_fd(fd);
+}
+
+static int check_fail(int rc, int expected_errno, const char *errno_str)
+{
+ if (rc > 0) {
+ printf("[FAIL] (unexpected success from open operation)\n");
+ close(rc);
+ 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;
+}
+
+#define check_openat_fail(dfd, path, flags, errno) \
+ _check_openat_fail(dfd, path, flags, errno, #errno)
+static int _check_openat_fail(int dfd, const char *path, int flags,
+ int expected_errno, const char *errno_str)
+{
+ int rc;
+
+ printf("Check failure of openat(%d, '%s', %x) with %s... ",
+ dfd, path?:"(null)", flags, errno_str);
+ errno = 0;
+ rc = openat_(dfd, path, flags);
+ return check_fail(rc, expected_errno, errno_str);
+}
+
+#define check_open_fail(path, flags, errno) \
+ _check_open_fail(path, flags, errno, #errno)
+static int _check_open_fail(const char *path, int flags,
+ int expected_errno, const char *errno_str)
+{
+ int rc;
+
+ printf("Check failure of open('%s', %x) with %s... ",
+ path?:"(null)", flags, errno_str);
+ errno = 0;
+ rc = open_(path, flags);
+ return check_fail(rc, expected_errno, errno_str);
+}
+
+int check_proc(void)
+{
+ int root_dfd = openat_(AT_FDCWD, "/", O_RDONLY);
+ int proc_dfd = openat_(AT_FDCWD, "/proc/self", O_RDONLY);
+ int fail = 0;
+
+ if (proc_dfd < 0) {
+ printf("'/proc/self' unavailable (errno=%d '%s'), skipping\n",
+ errno, strerror(errno));
+ return 0;
+ }
+ fail += check_openat(proc_dfd, "root/etc/passwd", O_RDONLY);
+ fail += check_openat(root_dfd, "proc/self/root/etc/passwd", O_RDONLY);
+#ifdef O_BENEATH
+ fail += check_openat_fail(proc_dfd, "root/etc/passwd",
+ O_RDONLY|O_BENEATH, EPERM);
+ fail += check_openat_fail(root_dfd, "proc/self/root/etc/passwd",
+ O_RDONLY|O_BENEATH, EPERM);
+#endif
+ return fail;
+}
+
+int main(int argc, char *argv[])
+{
+ int fail = 0;
+ int dot_dfd = openat_or_die(AT_FDCWD, ".", O_RDONLY);
+ int subdir_dfd = openat_or_die(AT_FDCWD, "subdir", O_RDONLY);
+ int file_fd = openat_or_die(AT_FDCWD, "topfile", O_RDONLY);
+
+ /* Sanity check normal behavior */
+ fail += check_open("topfile", O_RDONLY);
+ fail += check_open("subdir/bottomfile", O_RDONLY);
+ fail += check_openat(AT_FDCWD, "topfile", O_RDONLY);
+ fail += check_openat(AT_FDCWD, "subdir/bottomfile", O_RDONLY);
+
+ fail += check_openat(dot_dfd, "topfile", O_RDONLY);
+ fail += check_openat(dot_dfd, "subdir/bottomfile", O_RDONLY);
+ fail += check_openat(dot_dfd, "subdir/../topfile", O_RDONLY);
+ fail += check_open("subdir/../topfile", O_RDONLY);
+
+ fail += check_openat(subdir_dfd, "../topfile", O_RDONLY);
+ fail += check_openat(subdir_dfd, "bottomfile", O_RDONLY);
+ fail += check_openat(subdir_dfd, "../subdir/bottomfile", O_RDONLY);
+ fail += check_openat(subdir_dfd, "symlinkup", O_RDONLY);
+ fail += check_openat(subdir_dfd, "symlinkout", O_RDONLY);
+
+ fail += check_open("/etc/passwd", O_RDONLY);
+ fail += check_openat(AT_FDCWD, "/etc/passwd", O_RDONLY);
+ fail += check_openat(dot_dfd, "/etc/passwd", O_RDONLY);
+ fail += check_openat(subdir_dfd, "/etc/passwd", O_RDONLY);
+
+ fail += check_openat_fail(AT_FDCWD, "bogus", O_RDONLY, ENOENT);
+ fail += check_openat_fail(dot_dfd, "bogus", O_RDONLY, ENOENT);
+ fail += check_openat_fail(999, "bogus", O_RDONLY, EBADF);
+ fail += check_openat_fail(file_fd, "bogus", O_RDONLY, ENOTDIR);
+
+#ifdef O_BENEATH
+ /* Test out O_BENEATH */
+ fail += check_open("topfile", O_RDONLY|O_BENEATH);
+ fail += check_open("subdir/bottomfile", O_RDONLY|O_BENEATH);
+ fail += check_openat(AT_FDCWD, "topfile", O_RDONLY|O_BENEATH);
+ fail += check_openat(AT_FDCWD, "subdir/bottomfile",
+ O_RDONLY|O_BENEATH);
+
+ fail += check_openat(dot_dfd, "topfile", O_RDONLY|O_BENEATH);
+ fail += check_openat(dot_dfd, "subdir/bottomfile",
+ O_RDONLY|O_BENEATH);
+ fail += check_openat(dot_dfd, "subdir///bottomfile",
+ O_RDONLY|O_BENEATH);
+ fail += check_openat(subdir_dfd, "bottomfile", O_RDONLY|O_BENEATH);
+ fail += check_openat(subdir_dfd, "./bottomfile", O_RDONLY|O_BENEATH);
+ fail += check_openat(subdir_dfd, ".", O_RDONLY|O_BENEATH);
+
+ /* Symlinks without .. or leading / are OK */
+ fail += check_open("symlinkdown", O_RDONLY|O_BENEATH);
+ fail += check_open("subdir/symlinkin", O_RDONLY|O_BENEATH);
+ fail += check_openat(dot_dfd, "symlinkdown", O_RDONLY|O_BENEATH);
+ fail += check_openat(dot_dfd, "subdir/symlinkin", O_RDONLY|O_BENEATH);
+ fail += check_openat(subdir_dfd, "symlinkin", O_RDONLY|O_BENEATH);
+ /* ... unless of course we specify O_NOFOLLOW */
+ fail += check_open_fail("symlinkdown",
+ O_RDONLY|O_BENEATH|O_NOFOLLOW, ELOOP);
+ fail += check_open_fail("subdir/symlinkin",
+ O_RDONLY|O_BENEATH|O_NOFOLLOW, ELOOP);
+ fail += check_openat_fail(dot_dfd, "symlinkdown",
+ O_RDONLY|O_BENEATH|O_NOFOLLOW, ELOOP);
+ fail += check_openat_fail(dot_dfd, "subdir/symlinkin",
+ O_RDONLY|O_BENEATH|O_NOFOLLOW, ELOOP);
+ fail += check_openat_fail(subdir_dfd, "symlinkin",
+ O_RDONLY|O_BENEATH|O_NOFOLLOW, ELOOP);
+
+ /* Can't open paths with ".." in them */
+ fail += check_open_fail("subdir/../topfile",
+ O_RDONLY|O_BENEATH, EPERM);
+ fail += check_openat_fail(dot_dfd, "subdir/../topfile",
+ O_RDONLY|O_BENEATH, EPERM);
+ fail += check_openat_fail(subdir_dfd, "../topfile",
+ O_RDONLY|O_BENEATH, EPERM);
+ fail += check_openat_fail(subdir_dfd, "../subdir/bottomfile",
+ O_RDONLY|O_BENEATH, EPERM);
+ fail += check_openat_fail(subdir_dfd, "..", O_RDONLY|O_BENEATH, EPERM);
+
+ /* Can't open paths starting with "/" */
+ fail += check_open_fail("/etc/passwd", O_RDONLY|O_BENEATH, EPERM);
+ fail += check_openat_fail(AT_FDCWD, "/etc/passwd",
+ O_RDONLY|O_BENEATH, EPERM);
+ fail += check_openat_fail(dot_dfd, "/etc/passwd",
+ O_RDONLY|O_BENEATH, EPERM);
+ fail += check_openat_fail(subdir_dfd, "/etc/passwd",
+ O_RDONLY|O_BENEATH, EPERM);
+ /* Can't sneak around constraints with symlinks */
+ fail += check_openat_fail(subdir_dfd, "symlinkup",
+ O_RDONLY|O_BENEATH, EPERM);
+ fail += check_openat_fail(subdir_dfd, "symlinkout",
+ O_RDONLY|O_BENEATH, EPERM);
+ fail += check_openat_fail(subdir_dfd, "../symlinkdown",
+ O_RDONLY|O_BENEATH, EPERM);
+ fail += check_openat_fail(dot_dfd, "subdir/symlinkup",
+ O_RDONLY|O_BENEATH, EPERM);
+ fail += check_open_fail("subdir/symlinkup", O_RDONLY|O_BENEATH, EPERM);
+#else
+ printf("Skipping O_BENEATH tests due to missing #define\n");
+#endif
+ fail += check_proc();
+
+ if (fail > 0)
+ printf("%d tests failed\n", fail);
+ return fail;
+}
--
1.9.1
^ permalink raw reply related
* [PATCHv4 man-pages 3/3] open.2: describe O_BENEATH flag
From: David Drysdale @ 2015-08-13 9:32 UTC (permalink / raw)
To: linux-kernel, Alexander Viro, Kees Cook, Eric W. Biederman
Cc: Greg Kroah-Hartman, Meredydd Luff, Will Drewry,
Jorge Lucangeli Obes, Ricky Zhou, Lee Campbell, Julien Tinnes,
Mike Depinet, James Morris, Andy Lutomirski, Paolo Bonzini,
Paul Moore, Christoph Hellwig, Michael Kerrisk, Dave Chinner,
linux-api, linux-arch, linux-security-module, linux-fsdevel,
fstests, David Drysdale
In-Reply-To: <1439458366-8223-1-git-send-email-drysdale@google.com>
Signed-off-by: David Drysdale <drysdale@google.com>
---
man2/open.2 | 44 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 44 insertions(+)
diff --git a/man2/open.2 b/man2/open.2
index f49ab3042161..d09511f9ffb0 100644
--- a/man2/open.2
+++ b/man2/open.2
@@ -201,6 +201,43 @@ See
for further details.
See also BUGS, below.
.TP
+.B O_BENEATH " (since Linux 4.??)"
+Ensure that the
+.I pathname
+is beneath the current working directory (for
+.BR open (2))
+or the
+.I dirfd
+(for
+.BR openat (2)).
+If the
+.I pathname
+is absolute or contains a path component of "..", the
+.BR open ()
+fails with the error
+.BR EPERM.
+This occurs even if ".." path component would not actually
+escape the original directory; for example, a
+.I pathname
+of "subdir/../filename" would be rejected.
+Path components that are symbolic links to absolute paths, or that are
+relative paths containing a ".." component, will also cause the
+.BR open ()
+operation to fail with the error
+.BR EPERM.
+
+This feature allows applications to be sure that the opened file is
+within the specified directory, regardless of the original source of the
+.I pathname
+argument.
+Some security-conscious programs may further ensure
+this by imposing a system call filter (with
+.BR seccomp (2))
+that requires this flag for all
+.BR open ()
+operations, so that the program cannot open files outside of
+specified directories even if subverted.
+.TP
.BR O_CLOEXEC " (since Linux 2.6.23)"
.\" NOTE! several other man pages refer to this text
Enable the close-on-exec flag for the new file descriptor.
@@ -1015,6 +1052,13 @@ did not match the owner of the file and the caller was not privileged
The operation was prevented by a file seal; see
.BR fcntl (2).
.TP
+.B EPERM
+The
+.B O_BENEATH
+flag was specified and the
+.I pathname
+was not beneath the relevant directory.
+.TP
.B EROFS
.I pathname
refers to a file on a read-only filesystem and write access was
--
2.5.0.rc2.392.g76e840b
^ permalink raw reply related
* Re: [PATCHv4 man-pages 3/3] open.2: describe O_BENEATH flag
From: Andy Lutomirski @ 2015-08-13 17:38 UTC (permalink / raw)
To: David Drysdale
Cc: linux-kernel@vger.kernel.org, Alexander Viro, Kees Cook,
Eric W. Biederman, Greg Kroah-Hartman, Meredydd Luff, Will Drewry,
Jorge Lucangeli Obes, Ricky Zhou, Lee Campbell, Julien Tinnes,
Mike Depinet, James Morris, Paolo Bonzini, Paul Moore,
Christoph Hellwig, Michael Kerrisk, Dave Chinner, Linux API,
linux-arch, LSM List, Linux FS Devel <linux-fsdevel@
In-Reply-To: <1439458366-8223-4-git-send-email-drysdale@google.com>
On Thu, Aug 13, 2015 at 2:32 AM, David Drysdale <drysdale@google.com> wrote:
> Signed-off-by: David Drysdale <drysdale@google.com>
What's the behavior wrt fcntl(F_GETFL, etc)?
--Andy
^ permalink raw reply
* [RFC][PATCH 0/2] usb: gadget: add MTP function and Uevent userspace notifications
From: Amit Pundir @ 2015-08-13 19:12 UTC (permalink / raw)
To: linux-usb, linux-kernel, linux-doc, linux-api
Cc: Mike Lockwood, Benoit Goby, Colin Cross, Arve Hjønnevåg,
Peter Oh, Greg Hackmann, Badhri Jagan Sridharan,
Android Kernel Team, Greg Kroah-Hartman, Jonathan Corbet,
Felipe Balbi, Andrzej Pietrasiewicz, Laurent Pinchart,
Yegor Yefremov, Philippe Reynes, John Stultz, Sumit Semwal
RFC on Android's ConfigFS based MTP function implementation and usage
of Uevents to notify the userspace about the USB state changes.
The MTP function is based on years of work originally done in the
Android kernel tree by:
Mike Lockwood <lockwood@android.com>
Benoit Goby <benoit@android.com>
Colin Cross <ccross@android.com>
Arve Hjønnevåg <arve@android.com>
Peter Oh <poh@broadcom.com>
Greg Hackmann <ghackmann@google.com>
Badhri Jagan Sridharan <Badhri@google.com>
The Uevent notification patch is more of an RFC than an actual
submission. It is based on Android patchset originaly authored by
Badhri to send uevent notifications to Android userpace for USB
state changes.
I've folded the series up to make it easier to review, provided a
coherent patch description and modified it enough that I don't want
them to be blamed for any mistakes I've made condensing their patches
down.
Thoughts and feedback would be appreciated.
Thanks,
Amit Pundir
Cc: Mike Lockwood <lockwood@android.com>
Cc: Benoit Goby <benoit@android.com>
Cc: Colin Cross <ccross@android.com>
Cc: Arve Hjønnevåg <arve@android.com>
Cc: Peter Oh <poh@broadcom.com>
Cc: Greg Hackmann <ghackmann@google.com>
Cc: Badhri Jagan Sridharan <Badhri@google.com>
Cc: Android Kernel Team <kernel-team@android.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Felipe Balbi <balbi@ti.com>
Cc: Andrzej Pietrasiewicz <andrzej.p@samsung.com>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Yegor Yefremov <yegorslists@googlemail.com>
Cc: Philippe Reynes <tremyfr@gmail.com>
Cc: John Stultz <john.stultz@linaro.org>
Cc: Sumit Semwal <sumit.semwal@linaro.org>
Amit Pundir (2):
usb: gadget: configfs: add MTP function
usb: gadget: configfs: notify userspace of usb state changes
Documentation/ABI/testing/configfs-usb-gadget-mtp | 7 +
Documentation/usb/gadget-testing.txt | 24 +
drivers/usb/gadget/Kconfig | 21 +
drivers/usb/gadget/configfs.c | 201 ++-
drivers/usb/gadget/function/Makefile | 2 +
drivers/usb/gadget/function/f_mtp.c | 1365 +++++++++++++++++++++
include/linux/usb/f_mtp.h | 23 +
include/uapi/linux/usb/f_mtp.h | 61 +
8 files changed, 1702 insertions(+), 2 deletions(-)
create mode 100644 Documentation/ABI/testing/configfs-usb-gadget-mtp
create mode 100644 drivers/usb/gadget/function/f_mtp.c
create mode 100644 include/linux/usb/f_mtp.h
create mode 100644 include/uapi/linux/usb/f_mtp.h
--
1.9.1
^ permalink raw reply
* [RFC][PATCH 1/2] usb: gadget: configfs: add MTP function
From: Amit Pundir @ 2015-08-13 19:12 UTC (permalink / raw)
To: linux-usb, linux-kernel, linux-doc, linux-api
Cc: Mike Lockwood, Benoit Goby, Colin Cross, Arve Hjønnevåg,
Peter Oh, Greg Hackmann, Badhri Jagan Sridharan,
Android Kernel Team, Greg Kroah-Hartman, Jonathan Corbet,
Felipe Balbi, Andrzej Pietrasiewicz, Laurent Pinchart,
Yegor Yefremov, Philippe Reynes, John Stultz, Sumit Semwal
In-Reply-To: <1439493140-22207-1-git-send-email-amit.pundir@linaro.org>
This MTP function is based on years of work originally done in the
Android kernel tree by:
Mike Lockwood <lockwood@android.com>
Benoit Goby <benoit@android.com>
Colin Cross <ccross@android.com>
Arve Hjønnevåg <arve@android.com>
Peter Oh <poh@broadcom.com>
Greg Hackmann <ghackmann@google.com>
Badhri Jagan Sridharan <Badhri@google.com>
I've folded the series up to make it easier to review, and to provide
a coherent patch description.
Post Gingerbread (Android v2.3), Android dropped USB Mass Storage
in favor of Media Transfer Protocal (MTP), which is widely used for
transferring media files to digital music players and similar
applications. This USB gadget function implements MTP functionalty.
Historically this function has been a part of Android composite
gadget driver. Android composite driver was Android's solution
for dynamic gadget function switching prior to the ConfigFS gadget
being merged. There were failed few attempts in past
http://marc.info/?l=linux-usb&m=132451695808552 to upstream Android
composite driver as well. Now this Android MTP gadget function has been
re-implemented so as to be used as a generic ConfigFS function instead.
Again, many thanks to Mike, Benoit, Colin, Arve, Peter, Greg and Badhri,
as they are the real authors of this work. However, I've folded their
patches together and modified it enough that I don't want them to be
blamed for any mistakes I've made condensing their patches down.
Cc: Mike Lockwood <lockwood@android.com>
Cc: Benoit Goby <benoit@android.com>
Cc: Colin Cross <ccross@android.com>
Cc: Arve Hjønnevåg <arve@android.com>
Cc: Peter Oh <poh@broadcom.com>
Cc: Greg Hackmann <ghackmann@google.com>
Cc: Badhri Jagan Sridharan <Badhri@google.com>
Cc: Android Kernel Team <kernel-team@android.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Felipe Balbi <balbi@ti.com>
Cc: Andrzej Pietrasiewicz <andrzej.p@samsung.com>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Yegor Yefremov <yegorslists@googlemail.com>
Cc: Philippe Reynes <tremyfr@gmail.com>
Cc: John Stultz <john.stultz@linaro.org>
Cc: Sumit Semwal <sumit.semwal@linaro.org>
Signed-off-by: Amit Pundir <amit.pundir@linaro.org>
---
Documentation/ABI/testing/configfs-usb-gadget-mtp | 7 +
Documentation/usb/gadget-testing.txt | 24 +
drivers/usb/gadget/Kconfig | 13 +
drivers/usb/gadget/function/Makefile | 2 +
drivers/usb/gadget/function/f_mtp.c | 1365 +++++++++++++++++++++
include/linux/usb/f_mtp.h | 23 +
include/uapi/linux/usb/f_mtp.h | 61 +
7 files changed, 1495 insertions(+)
create mode 100644 Documentation/ABI/testing/configfs-usb-gadget-mtp
create mode 100644 drivers/usb/gadget/function/f_mtp.c
create mode 100644 include/linux/usb/f_mtp.h
create mode 100644 include/uapi/linux/usb/f_mtp.h
diff --git a/Documentation/ABI/testing/configfs-usb-gadget-mtp b/Documentation/ABI/testing/configfs-usb-gadget-mtp
new file mode 100644
index 0000000..6738bee
--- /dev/null
+++ b/Documentation/ABI/testing/configfs-usb-gadget-mtp
@@ -0,0 +1,7 @@
+What: /config/usb-gadget/gadget/functions/mtp.name
+Date: Aug 2015
+KernelVersion: 4.2
+Description: The purpose of this directory is to create and remove it.
+
+ A corresponding USB function instance is created/removed.
+ There are no attributes here.
diff --git a/Documentation/usb/gadget-testing.txt b/Documentation/usb/gadget-testing.txt
index 5926780..7fb8494d4 100644
--- a/Documentation/usb/gadget-testing.txt
+++ b/Documentation/usb/gadget-testing.txt
@@ -20,6 +20,7 @@ provided by gadgets.
17. UAC2 function
18. UVC function
19. PRINTER function
+20. MTP function
1. ACM function
@@ -771,3 +772,26 @@ host:
More advanced testing can be done with the prn_example
described in Documentation/usb/gadget-printer.txt.
+
+20. MTP function
+===============
+
+The function is provided by usb_f_mtp.ko module.
+
+Function-specific configfs interface
+------------------------------------
+
+The function name to use when creating the function directory is "mtp".
+The function directory is intentionally empty and has no attributes as such.
+
+After creating the mtp function directory, link mtp function with the gadget
+configuration by creating symbolic link, enable the gadget by writing a
+suitable string to usb_gadget/<gadget>/UDC and start the mtp userspace daemon.
+
+Testing the mtp function
+------------------------
+
+On the device: enable the gadget, and start the mtp userspace daemon.
+On the host: if configured correctly the media storage device should auto-mount
+itself, or use Linux mtp-tools package to browse/transfer the media content
+instead.
diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
index bcf83c0..65d110d 100644
--- a/drivers/usb/gadget/Kconfig
+++ b/drivers/usb/gadget/Kconfig
@@ -199,6 +199,9 @@ config USB_F_HID
config USB_F_PRINTER
tristate
+config USB_F_MTP
+ tristate
+
choice
tristate "USB Gadget Drivers"
default USB_ETH
@@ -451,6 +454,16 @@ config USB_CONFIGFS_F_PRINTER
For more information, see Documentation/usb/gadget_printer.txt
which includes sample code for accessing the device file.
+config USB_CONFIGFS_F_MTP
+ bool "MTP gadget"
+ depends on USB_CONFIGFS
+ select USB_F_MTP
+ help
+ The Media Transfer Protocol (MTP) function mounts USB gadget as
+ a media device but unlike Mass Storage Gadget, MTP operates at
+ the file level. Thus exposing the relevant content but hiding
+ the system/restricted files.
+
source "drivers/usb/gadget/legacy/Kconfig"
endchoice
diff --git a/drivers/usb/gadget/function/Makefile b/drivers/usb/gadget/function/Makefile
index bd7def5..11ef10f 100644
--- a/drivers/usb/gadget/function/Makefile
+++ b/drivers/usb/gadget/function/Makefile
@@ -44,3 +44,5 @@ usb_f_hid-y := f_hid.o
obj-$(CONFIG_USB_F_HID) += usb_f_hid.o
usb_f_printer-y := f_printer.o
obj-$(CONFIG_USB_F_PRINTER) += usb_f_printer.o
+usb_f_mtp-y := f_mtp.o
+obj-$(CONFIG_USB_F_MTP) += usb_f_mtp.o
diff --git a/drivers/usb/gadget/function/f_mtp.c b/drivers/usb/gadget/function/f_mtp.c
new file mode 100644
index 0000000..3eedece
--- /dev/null
+++ b/drivers/usb/gadget/function/f_mtp.c
@@ -0,0 +1,1365 @@
+/*
+ * Gadget Function Driver for MTP
+ *
+ * Copyright (C) 2010 Google, Inc.
+ * Author: Mike Lockwood <lockwood@android.com>
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * 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.
+ *
+ */
+
+/* #define DEBUG */
+/* #define VERBOSE_DEBUG */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/poll.h>
+#include <linux/delay.h>
+#include <linux/wait.h>
+#include <linux/err.h>
+#include <linux/interrupt.h>
+
+#include <linux/types.h>
+#include <linux/file.h>
+#include <linux/device.h>
+#include <linux/miscdevice.h>
+
+#include <linux/usb.h>
+#include <linux/usb_usual.h>
+#include <linux/usb/ch9.h>
+#include <linux/usb/f_mtp.h>
+#include <linux/configfs.h>
+#include <linux/usb/composite.h>
+
+#include "configfs.h"
+
+#define MTP_BULK_BUFFER_SIZE 16384
+#define INTR_BUFFER_SIZE 28
+#define MAX_INST_NAME_LEN 40
+
+/* String IDs */
+#define INTERFACE_STRING_INDEX 0
+
+/* values for mtp_dev.state */
+#define STATE_OFFLINE 0 /* initial state, disconnected */
+#define STATE_READY 1 /* ready for userspace calls */
+#define STATE_BUSY 2 /* processing userspace calls */
+#define STATE_CANCELED 3 /* transaction canceled by host */
+#define STATE_ERROR 4 /* error from completion routine */
+
+/* number of tx and rx requests to allocate */
+#define TX_REQ_MAX 4
+#define RX_REQ_MAX 2
+#define INTR_REQ_MAX 5
+
+/* ID for Microsoft MTP OS String */
+#define MTP_OS_STRING_ID 0xEE
+
+/* MTP class reqeusts */
+#define MTP_REQ_CANCEL 0x64
+#define MTP_REQ_GET_EXT_EVENT_DATA 0x65
+#define MTP_REQ_RESET 0x66
+#define MTP_REQ_GET_DEVICE_STATUS 0x67
+
+/* constants for device status */
+#define MTP_RESPONSE_OK 0x2001
+#define MTP_RESPONSE_DEVICE_BUSY 0x2019
+#define DRIVER_NAME "mtp"
+
+static const char mtp_shortname[] = DRIVER_NAME "_usb";
+
+struct mtp_dev {
+ struct usb_function function;
+ struct usb_composite_dev *cdev;
+ spinlock_t lock;
+
+ struct usb_ep *ep_in;
+ struct usb_ep *ep_out;
+ struct usb_ep *ep_intr;
+
+ int state;
+
+ /* synchronize access to our device file */
+ atomic_t open_excl;
+ /* to enforce only one ioctl at a time */
+ atomic_t ioctl_excl;
+
+ struct list_head tx_idle;
+ struct list_head intr_idle;
+
+ wait_queue_head_t read_wq;
+ wait_queue_head_t write_wq;
+ wait_queue_head_t intr_wq;
+ struct usb_request *rx_req[RX_REQ_MAX];
+ int rx_done;
+
+ /* for processing MTP_SEND_FILE, MTP_RECEIVE_FILE and
+ * MTP_SEND_FILE_WITH_HEADER ioctls on a work queue
+ */
+ struct workqueue_struct *wq;
+ struct work_struct send_file_work;
+ struct work_struct receive_file_work;
+ struct file *xfer_file;
+ loff_t xfer_file_offset;
+ int64_t xfer_file_length;
+ unsigned xfer_send_header;
+ uint16_t xfer_command;
+ uint32_t xfer_transaction_id;
+ int xfer_result;
+};
+
+static struct usb_interface_descriptor mtp_interface_desc = {
+ .bLength = USB_DT_INTERFACE_SIZE,
+ .bDescriptorType = USB_DT_INTERFACE,
+ .bInterfaceNumber = 0,
+ .bNumEndpoints = 3,
+ .bInterfaceClass = USB_CLASS_VENDOR_SPEC,
+ .bInterfaceSubClass = USB_SUBCLASS_VENDOR_SPEC,
+ .bInterfaceProtocol = 0,
+};
+
+static struct usb_endpoint_descriptor mtp_highspeed_in_desc = {
+ .bLength = USB_DT_ENDPOINT_SIZE,
+ .bDescriptorType = USB_DT_ENDPOINT,
+ .bEndpointAddress = USB_DIR_IN,
+ .bmAttributes = USB_ENDPOINT_XFER_BULK,
+ .wMaxPacketSize = cpu_to_le16(512),
+};
+
+static struct usb_endpoint_descriptor mtp_highspeed_out_desc = {
+ .bLength = USB_DT_ENDPOINT_SIZE,
+ .bDescriptorType = USB_DT_ENDPOINT,
+ .bEndpointAddress = USB_DIR_OUT,
+ .bmAttributes = USB_ENDPOINT_XFER_BULK,
+ .wMaxPacketSize = cpu_to_le16(512),
+};
+
+static struct usb_endpoint_descriptor mtp_fullspeed_in_desc = {
+ .bLength = USB_DT_ENDPOINT_SIZE,
+ .bDescriptorType = USB_DT_ENDPOINT,
+ .bEndpointAddress = USB_DIR_IN,
+ .bmAttributes = USB_ENDPOINT_XFER_BULK,
+};
+
+static struct usb_endpoint_descriptor mtp_fullspeed_out_desc = {
+ .bLength = USB_DT_ENDPOINT_SIZE,
+ .bDescriptorType = USB_DT_ENDPOINT,
+ .bEndpointAddress = USB_DIR_OUT,
+ .bmAttributes = USB_ENDPOINT_XFER_BULK,
+};
+
+static struct usb_endpoint_descriptor mtp_intr_desc = {
+ .bLength = USB_DT_ENDPOINT_SIZE,
+ .bDescriptorType = USB_DT_ENDPOINT,
+ .bEndpointAddress = USB_DIR_IN,
+ .bmAttributes = USB_ENDPOINT_XFER_INT,
+ .wMaxPacketSize = cpu_to_le16(INTR_BUFFER_SIZE),
+ .bInterval = 6,
+};
+
+static struct usb_descriptor_header *fs_mtp_descs[] = {
+ (struct usb_descriptor_header *) &mtp_interface_desc,
+ (struct usb_descriptor_header *) &mtp_fullspeed_in_desc,
+ (struct usb_descriptor_header *) &mtp_fullspeed_out_desc,
+ (struct usb_descriptor_header *) &mtp_intr_desc,
+ NULL,
+};
+
+static struct usb_descriptor_header *hs_mtp_descs[] = {
+ (struct usb_descriptor_header *) &mtp_interface_desc,
+ (struct usb_descriptor_header *) &mtp_highspeed_in_desc,
+ (struct usb_descriptor_header *) &mtp_highspeed_out_desc,
+ (struct usb_descriptor_header *) &mtp_intr_desc,
+ NULL,
+};
+
+static struct usb_string mtp_string_defs[] = {
+ /* Naming interface "MTP" so libmtp will recognize us */
+ [INTERFACE_STRING_INDEX].s = "MTP",
+ { }, /* end of list */
+};
+
+static struct usb_gadget_strings mtp_string_table = {
+ .language = 0x0409, /* en-US */
+ .strings = mtp_string_defs,
+};
+
+static struct usb_gadget_strings *mtp_strings[] = {
+ &mtp_string_table,
+ NULL,
+};
+
+/* Microsoft MTP OS String */
+static u8 mtp_os_string[] = {
+ 18, /* sizeof(mtp_os_string) */
+ USB_DT_STRING,
+ /* Signature field: "MSFT100" */
+ 'M', 0, 'S', 0, 'F', 0, 'T', 0, '1', 0, '0', 0, '0', 0,
+ /* vendor code */
+ 1,
+ /* padding */
+ 0
+};
+
+/* Microsoft Extended Configuration Descriptor Header Section */
+struct mtp_ext_config_desc_header {
+ __le32 dwLength;
+ __u16 bcdVersion;
+ __le16 wIndex;
+ __u8 bCount;
+ __u8 reserved[7];
+};
+
+/* Microsoft Extended Configuration Descriptor Function Section */
+struct mtp_ext_config_desc_function {
+ __u8 bFirstInterfaceNumber;
+ __u8 bInterfaceCount;
+ __u8 compatibleID[8];
+ __u8 subCompatibleID[8];
+ __u8 reserved[6];
+};
+
+/* MTP Extended Configuration Descriptor */
+struct {
+ struct mtp_ext_config_desc_header header;
+ struct mtp_ext_config_desc_function function;
+} mtp_ext_config_desc = {
+ .header = {
+ .dwLength = cpu_to_le32(sizeof(mtp_ext_config_desc)),
+ .bcdVersion = cpu_to_le16(0x0100),
+ .wIndex = cpu_to_le16(4),
+ .bCount = cpu_to_le16(1),
+ },
+ .function = {
+ .bFirstInterfaceNumber = 0,
+ .bInterfaceCount = 1,
+ .compatibleID = { 'M', 'T', 'P' },
+ },
+};
+
+struct mtp_device_status {
+ __le16 wLength;
+ __le16 wCode;
+};
+
+struct mtp_data_header {
+ /* length of packet, including this header */
+ __le32 length;
+ /* container type (2 for data packet) */
+ __le16 type;
+ /* MTP command code */
+ __le16 command;
+ /* MTP transaction ID */
+ __le32 transaction_id;
+};
+
+struct mtp_instance {
+ struct usb_function_instance func_inst;
+ const char *name;
+ struct mtp_dev *dev;
+};
+
+/* temporary variable used between mtp_open() and mtp_gadget_bind() */
+static struct mtp_dev *_mtp_dev;
+
+static inline struct mtp_dev *func_to_mtp(struct usb_function *f)
+{
+ return container_of(f, struct mtp_dev, function);
+}
+
+static struct usb_request *mtp_request_new(struct usb_ep *ep, int buffer_size)
+{
+ struct usb_request *req = usb_ep_alloc_request(ep, GFP_KERNEL);
+
+ if (!req)
+ return NULL;
+ /* now allocate buffers for the requests */
+ req->buf = kmalloc(buffer_size, GFP_KERNEL);
+ if (!req->buf) {
+ usb_ep_free_request(ep, req);
+ return NULL;
+ }
+
+ return req;
+}
+
+static void mtp_request_free(struct usb_request *req, struct usb_ep *ep)
+{
+ if (req) {
+ kfree(req->buf);
+ usb_ep_free_request(ep, req);
+ }
+}
+
+static inline int mtp_lock(atomic_t *excl)
+{
+ if (atomic_inc_return(excl) == 1) {
+ return 0;
+ } else {
+ atomic_dec(excl);
+ return -1;
+ }
+}
+
+static inline void mtp_unlock(atomic_t *excl)
+{
+ atomic_dec(excl);
+}
+
+/* add a request to the tail of a list */
+static void mtp_req_put(struct mtp_dev *dev, struct list_head *head,
+ struct usb_request *req)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(&dev->lock, flags);
+ list_add_tail(&req->list, head);
+ spin_unlock_irqrestore(&dev->lock, flags);
+}
+
+/* remove a request from the head of a list */
+static struct usb_request
+*mtp_req_get(struct mtp_dev *dev, struct list_head *head)
+{
+ unsigned long flags;
+ struct usb_request *req;
+
+ spin_lock_irqsave(&dev->lock, flags);
+ if (list_empty(head)) {
+ req = 0;
+ } else {
+ req = list_first_entry(head, struct usb_request, list);
+ list_del(&req->list);
+ }
+ spin_unlock_irqrestore(&dev->lock, flags);
+ return req;
+}
+
+static void mtp_complete_in(struct usb_ep *ep, struct usb_request *req)
+{
+ struct mtp_dev *dev = _mtp_dev;
+
+ if (req->status != 0)
+ dev->state = STATE_ERROR;
+
+ mtp_req_put(dev, &dev->tx_idle, req);
+
+ wake_up(&dev->write_wq);
+}
+
+static void mtp_complete_out(struct usb_ep *ep, struct usb_request *req)
+{
+ struct mtp_dev *dev = _mtp_dev;
+
+ dev->rx_done = 1;
+ if (req->status != 0)
+ dev->state = STATE_ERROR;
+
+ wake_up(&dev->read_wq);
+}
+
+static void mtp_complete_intr(struct usb_ep *ep, struct usb_request *req)
+{
+ struct mtp_dev *dev = _mtp_dev;
+
+ if (req->status != 0)
+ dev->state = STATE_ERROR;
+
+ mtp_req_put(dev, &dev->intr_idle, req);
+
+ wake_up(&dev->intr_wq);
+}
+
+static int mtp_create_bulk_endpoints(struct mtp_dev *dev,
+ struct usb_endpoint_descriptor *in_desc,
+ struct usb_endpoint_descriptor *out_desc,
+ struct usb_endpoint_descriptor *intr_desc)
+{
+ struct usb_composite_dev *cdev = dev->cdev;
+ struct usb_request *req;
+ struct usb_ep *ep;
+ int i;
+
+ DBG(cdev, "create_bulk_endpoints dev: %p\n", dev);
+
+ ep = usb_ep_autoconfig(cdev->gadget, in_desc);
+ if (!ep) {
+ DBG(cdev, "usb_ep_autoconfig for ep_in failed\n");
+ return -ENODEV;
+ }
+ DBG(cdev, "usb_ep_autoconfig for ep_in got %s\n", ep->name);
+ ep->driver_data = dev; /* claim the endpoint */
+ dev->ep_in = ep;
+
+ ep = usb_ep_autoconfig(cdev->gadget, out_desc);
+ if (!ep) {
+ DBG(cdev, "usb_ep_autoconfig for ep_out failed\n");
+ return -ENODEV;
+ }
+ DBG(cdev, "usb_ep_autoconfig for mtp ep_out got %s\n", ep->name);
+ ep->driver_data = dev; /* claim the endpoint */
+ dev->ep_out = ep;
+
+ ep = usb_ep_autoconfig(cdev->gadget, intr_desc);
+ if (!ep) {
+ DBG(cdev, "usb_ep_autoconfig for ep_intr failed\n");
+ return -ENODEV;
+ }
+ DBG(cdev, "usb_ep_autoconfig for mtp ep_intr got %s\n", ep->name);
+ ep->driver_data = dev; /* claim the endpoint */
+ dev->ep_intr = ep;
+
+ /* now allocate requests for our endpoints */
+ for (i = 0; i < TX_REQ_MAX; i++) {
+ req = mtp_request_new(dev->ep_in, MTP_BULK_BUFFER_SIZE);
+ if (!req)
+ goto fail;
+ req->complete = mtp_complete_in;
+ mtp_req_put(dev, &dev->tx_idle, req);
+ }
+ for (i = 0; i < RX_REQ_MAX; i++) {
+ req = mtp_request_new(dev->ep_out, MTP_BULK_BUFFER_SIZE);
+ if (!req)
+ goto fail;
+ req->complete = mtp_complete_out;
+ dev->rx_req[i] = req;
+ }
+ for (i = 0; i < INTR_REQ_MAX; i++) {
+ req = mtp_request_new(dev->ep_intr, INTR_BUFFER_SIZE);
+ if (!req)
+ goto fail;
+ req->complete = mtp_complete_intr;
+ mtp_req_put(dev, &dev->intr_idle, req);
+ }
+
+ return 0;
+
+fail:
+ pr_err("mtp_bind() could not allocate requests\n");
+ return -1;
+}
+
+static ssize_t mtp_read(struct file *fp, char __user *buf,
+ size_t count, loff_t *pos)
+{
+ struct mtp_dev *dev = fp->private_data;
+ struct usb_composite_dev *cdev = dev->cdev;
+ struct usb_request *req;
+ ssize_t r = count;
+ unsigned xfer;
+ int ret = 0;
+
+ DBG(cdev, "mtp_read(%zu)\n", count);
+
+ if (count > MTP_BULK_BUFFER_SIZE)
+ return -EINVAL;
+
+ /* we will block until we're online */
+ DBG(cdev, "mtp_read: waiting for online state\n");
+ ret = wait_event_interruptible(dev->read_wq,
+ dev->state != STATE_OFFLINE);
+ if (ret < 0) {
+ r = ret;
+ goto done;
+ }
+ spin_lock_irq(&dev->lock);
+ if (dev->state == STATE_CANCELED) {
+ /* report cancellation to userspace */
+ dev->state = STATE_READY;
+ spin_unlock_irq(&dev->lock);
+ return -ECANCELED;
+ }
+ dev->state = STATE_BUSY;
+ spin_unlock_irq(&dev->lock);
+
+requeue_req:
+ /* queue a request */
+ req = dev->rx_req[0];
+ req->length = count;
+ dev->rx_done = 0;
+ ret = usb_ep_queue(dev->ep_out, req, GFP_KERNEL);
+ if (ret < 0) {
+ r = -EIO;
+ goto done;
+ } else {
+ DBG(cdev, "rx %p queue\n", req);
+ }
+
+ /* wait for a request to complete */
+ ret = wait_event_interruptible(dev->read_wq, dev->rx_done);
+ if (ret < 0) {
+ r = ret;
+ usb_ep_dequeue(dev->ep_out, req);
+ goto done;
+ }
+ if (dev->state == STATE_BUSY) {
+ /* If we got a 0-len packet, throw it back and try again. */
+ if (req->actual == 0)
+ goto requeue_req;
+
+ DBG(cdev, "rx %p %d\n", req, req->actual);
+ xfer = (req->actual < count) ? req->actual : count;
+ r = xfer;
+ if (copy_to_user(buf, req->buf, xfer))
+ r = -EFAULT;
+ } else
+ r = -EIO;
+
+done:
+ spin_lock_irq(&dev->lock);
+ if (dev->state == STATE_CANCELED)
+ r = -ECANCELED;
+ else if (dev->state != STATE_OFFLINE)
+ dev->state = STATE_READY;
+ spin_unlock_irq(&dev->lock);
+
+ DBG(cdev, "mtp_read returning %zd\n", r);
+ return r;
+}
+
+static ssize_t mtp_write(struct file *fp, const char __user *buf,
+ size_t count, loff_t *pos)
+{
+ struct mtp_dev *dev = fp->private_data;
+ struct usb_composite_dev *cdev = dev->cdev;
+ struct usb_request *req = 0;
+ ssize_t r = count;
+ unsigned xfer;
+ int sendZLP = 0;
+ int ret;
+
+ DBG(cdev, "mtp_write(%zu)\n", count);
+
+ spin_lock_irq(&dev->lock);
+ if (dev->state == STATE_CANCELED) {
+ /* report cancellation to userspace */
+ dev->state = STATE_READY;
+ spin_unlock_irq(&dev->lock);
+ return -ECANCELED;
+ }
+ if (dev->state == STATE_OFFLINE) {
+ spin_unlock_irq(&dev->lock);
+ return -ENODEV;
+ }
+ dev->state = STATE_BUSY;
+ spin_unlock_irq(&dev->lock);
+
+ /* we need to send a zero length packet to signal the end of transfer
+ * if the transfer size is aligned to a packet boundary.
+ */
+ if ((count & (dev->ep_in->maxpacket - 1)) == 0)
+ sendZLP = 1;
+
+ while (count > 0 || sendZLP) {
+ /* so we exit after sending ZLP */
+ if (count == 0)
+ sendZLP = 0;
+
+ if (dev->state != STATE_BUSY) {
+ DBG(cdev, "mtp_write dev->error\n");
+ r = -EIO;
+ break;
+ }
+
+ /* get an idle tx request to use */
+ req = 0;
+ ret = wait_event_interruptible(dev->write_wq,
+ ((req = mtp_req_get(dev, &dev->tx_idle))
+ || dev->state != STATE_BUSY));
+ if (!req) {
+ r = ret;
+ break;
+ }
+
+ if (count > MTP_BULK_BUFFER_SIZE)
+ xfer = MTP_BULK_BUFFER_SIZE;
+ else
+ xfer = count;
+ if (xfer && copy_from_user(req->buf, buf, xfer)) {
+ r = -EFAULT;
+ break;
+ }
+
+ req->length = xfer;
+ ret = usb_ep_queue(dev->ep_in, req, GFP_KERNEL);
+ if (ret < 0) {
+ DBG(cdev, "mtp_write: xfer error %d\n", ret);
+ r = -EIO;
+ break;
+ }
+
+ buf += xfer;
+ count -= xfer;
+
+ /* zero this so we don't try to free it on error exit */
+ req = 0;
+ }
+
+ if (req)
+ mtp_req_put(dev, &dev->tx_idle, req);
+
+ spin_lock_irq(&dev->lock);
+ if (dev->state == STATE_CANCELED)
+ r = -ECANCELED;
+ else if (dev->state != STATE_OFFLINE)
+ dev->state = STATE_READY;
+ spin_unlock_irq(&dev->lock);
+
+ DBG(cdev, "mtp_write returning %zd\n", r);
+ return r;
+}
+
+/* read from a local file and write to USB */
+static void send_file_work(struct work_struct *data)
+{
+ struct mtp_dev *dev = container_of(data, struct mtp_dev,
+ send_file_work);
+ struct usb_composite_dev *cdev = dev->cdev;
+ struct usb_request *req = 0;
+ struct mtp_data_header *header;
+ struct file *filp;
+ loff_t offset;
+ int64_t count;
+ int xfer, ret, hdr_size;
+ int r = 0;
+ int sendZLP = 0;
+
+ /* read our parameters */
+ smp_rmb();
+ filp = dev->xfer_file;
+ offset = dev->xfer_file_offset;
+ count = dev->xfer_file_length;
+
+ DBG(cdev, "send_file_work(%lld %lld)\n", offset, count);
+
+ if (dev->xfer_send_header) {
+ hdr_size = sizeof(struct mtp_data_header);
+ count += hdr_size;
+ } else {
+ hdr_size = 0;
+ }
+
+ /* we need to send a zero length packet to signal the end of transfer
+ * if the transfer size is aligned to a packet boundary.
+ */
+ if ((count & (dev->ep_in->maxpacket - 1)) == 0)
+ sendZLP = 1;
+
+ while (count > 0 || sendZLP) {
+ /* so we exit after sending ZLP */
+ if (count == 0)
+ sendZLP = 0;
+
+ /* get an idle tx request to use */
+ req = 0;
+ ret = wait_event_interruptible(dev->write_wq,
+ (req = mtp_req_get(dev, &dev->tx_idle))
+ || dev->state != STATE_BUSY);
+ if (dev->state == STATE_CANCELED) {
+ r = -ECANCELED;
+ break;
+ }
+ if (!req) {
+ r = ret;
+ break;
+ }
+
+ if (count > MTP_BULK_BUFFER_SIZE)
+ xfer = MTP_BULK_BUFFER_SIZE;
+ else
+ xfer = count;
+
+ if (hdr_size) {
+ /* prepend MTP data header */
+ header = (struct mtp_data_header *)req->buf;
+ header->length = __cpu_to_le32(count);
+ header->type = __cpu_to_le16(2); /* data packet */
+ header->command = __cpu_to_le16(dev->xfer_command);
+ header->transaction_id =
+ __cpu_to_le32(dev->xfer_transaction_id);
+ }
+
+ ret = vfs_read(filp, req->buf + hdr_size, xfer - hdr_size,
+ &offset);
+ if (ret < 0) {
+ r = ret;
+ break;
+ }
+ xfer = ret + hdr_size;
+ hdr_size = 0;
+
+ req->length = xfer;
+ ret = usb_ep_queue(dev->ep_in, req, GFP_KERNEL);
+ if (ret < 0) {
+ DBG(cdev, "send_file_work: xfer error %d\n", ret);
+ dev->state = STATE_ERROR;
+ r = -EIO;
+ break;
+ }
+
+ count -= xfer;
+
+ /* zero this so we don't try to free it on error exit */
+ req = 0;
+ }
+
+ if (req)
+ mtp_req_put(dev, &dev->tx_idle, req);
+
+ DBG(cdev, "send_file_work returning %d\n", r);
+ /* write the result */
+ dev->xfer_result = r;
+ smp_wmb();
+}
+
+/* read from USB and write to a local file */
+static void receive_file_work(struct work_struct *data)
+{
+ struct mtp_dev *dev = container_of(data, struct mtp_dev,
+ receive_file_work);
+ struct usb_composite_dev *cdev = dev->cdev;
+ struct usb_request *read_req = NULL, *write_req = NULL;
+ struct file *filp;
+ loff_t offset;
+ int64_t count;
+ int ret, cur_buf = 0;
+ int r = 0;
+
+ /* read our parameters */
+ smp_rmb();
+ filp = dev->xfer_file;
+ offset = dev->xfer_file_offset;
+ count = dev->xfer_file_length;
+
+ DBG(cdev, "receive_file_work(%lld)\n", count);
+
+ while (count > 0 || write_req) {
+ if (count > 0) {
+ /* queue a request */
+ read_req = dev->rx_req[cur_buf];
+ cur_buf = (cur_buf + 1) % RX_REQ_MAX;
+
+ read_req->length = (count > MTP_BULK_BUFFER_SIZE
+ ? MTP_BULK_BUFFER_SIZE : count);
+ dev->rx_done = 0;
+ ret = usb_ep_queue(dev->ep_out, read_req, GFP_KERNEL);
+ if (ret < 0) {
+ r = -EIO;
+ dev->state = STATE_ERROR;
+ break;
+ }
+ }
+
+ if (write_req) {
+ DBG(cdev, "rx %p %d\n", write_req, write_req->actual);
+ ret = vfs_write(filp, write_req->buf, write_req->actual,
+ &offset);
+ DBG(cdev, "vfs_write %d\n", ret);
+ if (ret != write_req->actual) {
+ r = -EIO;
+ dev->state = STATE_ERROR;
+ break;
+ }
+ write_req = NULL;
+ }
+
+ if (read_req) {
+ /* wait for our last read to complete */
+ ret = wait_event_interruptible(dev->read_wq,
+ dev->rx_done || dev->state != STATE_BUSY);
+ if (dev->state == STATE_CANCELED) {
+ r = -ECANCELED;
+ if (!dev->rx_done)
+ usb_ep_dequeue(dev->ep_out, read_req);
+ break;
+ }
+ /* if xfer_file_length is 0xFFFFFFFF, then we read until
+ * we get a zero length packet
+ */
+ if (count != 0xFFFFFFFF)
+ count -= read_req->actual;
+ if (read_req->actual < read_req->length) {
+ /*
+ * short packet is used to signal EOF for
+ * sizes > 4 gig
+ */
+ DBG(cdev, "got short packet\n");
+ count = 0;
+ }
+
+ write_req = read_req;
+ read_req = NULL;
+ }
+ }
+
+ DBG(cdev, "receive_file_work returning %d\n", r);
+ /* write the result */
+ dev->xfer_result = r;
+ smp_wmb();
+}
+
+static int mtp_send_event(struct mtp_dev *dev, struct mtp_event *event)
+{
+ struct usb_request *req = NULL;
+ int ret;
+ int length = event->length;
+
+ DBG(dev->cdev, "mtp_send_event(%zu)\n", event->length);
+
+ if (length < 0 || length > INTR_BUFFER_SIZE)
+ return -EINVAL;
+ if (dev->state == STATE_OFFLINE)
+ return -ENODEV;
+
+ ret = wait_event_interruptible_timeout(dev->intr_wq,
+ (req = mtp_req_get(dev, &dev->intr_idle)),
+ msecs_to_jiffies(1000));
+ if (!req)
+ return -ETIME;
+
+ if (copy_from_user(req->buf, (void __user *)event->data, length)) {
+ mtp_req_put(dev, &dev->intr_idle, req);
+ return -EFAULT;
+ }
+ req->length = length;
+ ret = usb_ep_queue(dev->ep_intr, req, GFP_KERNEL);
+ if (ret)
+ mtp_req_put(dev, &dev->intr_idle, req);
+
+ return ret;
+}
+
+static long mtp_ioctl(struct file *fp, unsigned code, unsigned long value)
+{
+ struct mtp_dev *dev = fp->private_data;
+ struct file *filp = NULL;
+ int ret = -EINVAL;
+
+ if (mtp_lock(&dev->ioctl_excl))
+ return -EBUSY;
+
+ switch (code) {
+ case MTP_SEND_FILE:
+ case MTP_RECEIVE_FILE:
+ case MTP_SEND_FILE_WITH_HEADER:
+ {
+ struct mtp_file_range mfr;
+ struct work_struct *work;
+
+ spin_lock_irq(&dev->lock);
+ if (dev->state == STATE_CANCELED) {
+ /* report cancellation to userspace */
+ dev->state = STATE_READY;
+ spin_unlock_irq(&dev->lock);
+ ret = -ECANCELED;
+ goto out;
+ }
+ if (dev->state == STATE_OFFLINE) {
+ spin_unlock_irq(&dev->lock);
+ ret = -ENODEV;
+ goto out;
+ }
+ dev->state = STATE_BUSY;
+ spin_unlock_irq(&dev->lock);
+
+ if (copy_from_user(&mfr, (void __user *)value, sizeof(mfr))) {
+ ret = -EFAULT;
+ goto fail;
+ }
+ /* hold a reference to the file while we are working with it */
+ filp = fget(mfr.fd);
+ if (!filp) {
+ ret = -EBADF;
+ goto fail;
+ }
+
+ /* write the parameters */
+ dev->xfer_file = filp;
+ dev->xfer_file_offset = mfr.offset;
+ dev->xfer_file_length = mfr.length;
+ smp_wmb();
+
+ if (code == MTP_SEND_FILE_WITH_HEADER) {
+ work = &dev->send_file_work;
+ dev->xfer_send_header = 1;
+ dev->xfer_command = mfr.command;
+ dev->xfer_transaction_id = mfr.transaction_id;
+ } else if (code == MTP_SEND_FILE) {
+ work = &dev->send_file_work;
+ dev->xfer_send_header = 0;
+ } else {
+ work = &dev->receive_file_work;
+ }
+
+ /* We do the file transfer on a work queue so it will run
+ * in kernel context, which is necessary for vfs_read and
+ * vfs_write to use our buffers in the kernel address space.
+ */
+ queue_work(dev->wq, work);
+ /* wait for operation to complete */
+ flush_workqueue(dev->wq);
+ fput(filp);
+
+ /* read the result */
+ smp_rmb();
+ ret = dev->xfer_result;
+ break;
+ }
+ case MTP_SEND_EVENT:
+ {
+ struct mtp_event event;
+ /* return here so we don't change dev->state below,
+ * which would interfere with bulk transfer state.
+ */
+ if (copy_from_user(&event, (void __user *)value, sizeof(event)))
+ ret = -EFAULT;
+ else
+ ret = mtp_send_event(dev, &event);
+ goto out;
+ }
+ }
+
+fail:
+ spin_lock_irq(&dev->lock);
+ if (dev->state == STATE_CANCELED)
+ ret = -ECANCELED;
+ else if (dev->state != STATE_OFFLINE)
+ dev->state = STATE_READY;
+ spin_unlock_irq(&dev->lock);
+out:
+ mtp_unlock(&dev->ioctl_excl);
+ DBG(dev->cdev, "ioctl returning %d\n", ret);
+ return ret;
+}
+
+static int mtp_open(struct inode *ip, struct file *fp)
+{
+ pr_info("mtp_open\n");
+ if (mtp_lock(&_mtp_dev->open_excl))
+ return -EBUSY;
+
+ /* clear any error condition */
+ if (_mtp_dev->state != STATE_OFFLINE)
+ _mtp_dev->state = STATE_READY;
+
+ fp->private_data = _mtp_dev;
+ return 0;
+}
+
+static int mtp_release(struct inode *ip, struct file *fp)
+{
+ pr_info("mtp_release\n");
+
+ mtp_unlock(&_mtp_dev->open_excl);
+ return 0;
+}
+
+/* file operations for /dev/mtp_usb */
+static const struct file_operations mtp_fops = {
+ .owner = THIS_MODULE,
+ .read = mtp_read,
+ .write = mtp_write,
+ .unlocked_ioctl = mtp_ioctl,
+ .open = mtp_open,
+ .release = mtp_release,
+};
+
+static struct miscdevice mtp_device = {
+ .minor = MISC_DYNAMIC_MINOR,
+ .name = mtp_shortname,
+ .fops = &mtp_fops,
+};
+
+static int mtp_ctrlrequest(struct usb_composite_dev *cdev,
+ const struct usb_ctrlrequest *ctrl)
+{
+ struct mtp_dev *dev = _mtp_dev;
+ int value = -EOPNOTSUPP;
+ u16 w_index = le16_to_cpu(ctrl->wIndex);
+ u16 w_value = le16_to_cpu(ctrl->wValue);
+ u16 w_length = le16_to_cpu(ctrl->wLength);
+ unsigned long flags;
+
+ VDBG(cdev, "mtp_ctrlrequest %02x.%02x v%04x i%04x l%u\n",
+ ctrl->bRequestType, ctrl->bRequest,
+ w_value, w_index, w_length);
+
+ /* Handle MTP OS string */
+ if (ctrl->bRequestType ==
+ (USB_DIR_IN | USB_TYPE_STANDARD | USB_RECIP_DEVICE)
+ && ctrl->bRequest == USB_REQ_GET_DESCRIPTOR
+ && (w_value >> 8) == USB_DT_STRING
+ && (w_value & 0xFF) == MTP_OS_STRING_ID) {
+ value = (w_length < sizeof(mtp_os_string)
+ ? w_length : sizeof(mtp_os_string));
+ memcpy(cdev->req->buf, mtp_os_string, value);
+ } else if ((ctrl->bRequestType & USB_TYPE_MASK) == USB_TYPE_VENDOR) {
+ /* Handle MTP OS descriptor */
+ DBG(cdev, "vendor request: %d index: %d value: %d length: %d\n",
+ ctrl->bRequest, w_index, w_value, w_length);
+
+ if (ctrl->bRequest == 1
+ && (ctrl->bRequestType & USB_DIR_IN)
+ && (w_index == 4 || w_index == 5)) {
+ value = (w_length < sizeof(mtp_ext_config_desc) ?
+ w_length : sizeof(mtp_ext_config_desc));
+ memcpy(cdev->req->buf, &mtp_ext_config_desc, value);
+ }
+ } else if ((ctrl->bRequestType & USB_TYPE_MASK) == USB_TYPE_CLASS) {
+ DBG(cdev, "class request: %d index: %d value: %d length: %d\n",
+ ctrl->bRequest, w_index, w_value, w_length);
+
+ if (ctrl->bRequest == MTP_REQ_CANCEL && w_index == 0
+ && w_value == 0) {
+ DBG(cdev, "MTP_REQ_CANCEL\n");
+
+ spin_lock_irqsave(&dev->lock, flags);
+ if (dev->state == STATE_BUSY) {
+ dev->state = STATE_CANCELED;
+ wake_up(&dev->read_wq);
+ wake_up(&dev->write_wq);
+ }
+ spin_unlock_irqrestore(&dev->lock, flags);
+
+ /* We need to queue a request to read the remaining
+ * bytes, but we don't actually need to look at
+ * the contents.
+ */
+ value = w_length;
+ } else if (ctrl->bRequest == MTP_REQ_GET_DEVICE_STATUS
+ && w_index == 0 && w_value == 0) {
+ struct mtp_device_status *status = cdev->req->buf;
+
+ status->wLength =
+ cpu_to_le16(sizeof(*status));
+ DBG(cdev, "MTP_REQ_GET_DEVICE_STATUS\n");
+ spin_lock_irqsave(&dev->lock, flags);
+ /* device status is "busy" until we report
+ * the cancellation to userspace
+ */
+ if (dev->state == STATE_CANCELED)
+ status->wCode =
+ __cpu_to_le16(MTP_RESPONSE_DEVICE_BUSY);
+ else
+ status->wCode =
+ __cpu_to_le16(MTP_RESPONSE_OK);
+ spin_unlock_irqrestore(&dev->lock, flags);
+ value = sizeof(*status);
+ }
+ }
+
+ /* respond with data transfer or status phase? */
+ if (value >= 0) {
+ int rc;
+
+ cdev->req->zero = value < w_length;
+ cdev->req->length = value;
+ rc = usb_ep_queue(cdev->gadget->ep0, cdev->req, GFP_ATOMIC);
+ if (rc < 0)
+ ERROR(cdev, "%s: response queue error\n", __func__);
+ }
+ return value;
+}
+
+static int
+mtp_function_bind(struct usb_configuration *c, struct usb_function *f)
+{
+ struct usb_composite_dev *cdev = c->cdev;
+ struct mtp_dev *dev = func_to_mtp(f);
+ int id;
+ int ret;
+
+ dev->cdev = cdev;
+ DBG(cdev, "mtp_function_bind dev: %p\n", dev);
+
+ /* allocate interface ID(s) */
+ id = usb_interface_id(c, f);
+ if (id < 0)
+ return id;
+ mtp_interface_desc.bInterfaceNumber = id;
+
+ if (mtp_string_defs[INTERFACE_STRING_INDEX].id == 0) {
+ ret = usb_string_id(c->cdev);
+ if (ret < 0)
+ return ret;
+ mtp_string_defs[INTERFACE_STRING_INDEX].id = ret;
+ mtp_interface_desc.iInterface = ret;
+ }
+ /* allocate endpoints */
+ ret = mtp_create_bulk_endpoints(dev, &mtp_fullspeed_in_desc,
+ &mtp_fullspeed_out_desc, &mtp_intr_desc);
+ if (ret)
+ return ret;
+
+ /* support high speed hardware */
+ if (gadget_is_dualspeed(c->cdev->gadget)) {
+ mtp_highspeed_in_desc.bEndpointAddress =
+ mtp_fullspeed_in_desc.bEndpointAddress;
+ mtp_highspeed_out_desc.bEndpointAddress =
+ mtp_fullspeed_out_desc.bEndpointAddress;
+ }
+
+ DBG(cdev, "%s speed %s: IN/%s, OUT/%s\n",
+ gadget_is_dualspeed(c->cdev->gadget) ? "dual" : "full",
+ f->name, dev->ep_in->name, dev->ep_out->name);
+ return 0;
+}
+
+static void
+mtp_function_unbind(struct usb_configuration *c, struct usb_function *f)
+{
+ struct mtp_dev *dev = func_to_mtp(f);
+ struct usb_request *req;
+ int i;
+
+ mtp_string_defs[INTERFACE_STRING_INDEX].id = 0;
+ while ((req = mtp_req_get(dev, &dev->tx_idle)))
+ mtp_request_free(req, dev->ep_in);
+ for (i = 0; i < RX_REQ_MAX; i++)
+ mtp_request_free(dev->rx_req[i], dev->ep_out);
+ while ((req = mtp_req_get(dev, &dev->intr_idle)))
+ mtp_request_free(req, dev->ep_intr);
+ dev->state = STATE_OFFLINE;
+}
+
+static int mtp_function_set_alt(struct usb_function *f,
+ unsigned intf, unsigned alt)
+{
+ struct mtp_dev *dev = func_to_mtp(f);
+ struct usb_composite_dev *cdev = f->config->cdev;
+ int ret;
+
+ DBG(cdev, "mtp_function_set_alt intf: %d alt: %d\n", intf, alt);
+
+ ret = config_ep_by_speed(cdev->gadget, f, dev->ep_in);
+ if (ret)
+ return ret;
+
+ ret = usb_ep_enable(dev->ep_in);
+ if (ret)
+ return ret;
+
+ ret = config_ep_by_speed(cdev->gadget, f, dev->ep_out);
+ if (ret)
+ return ret;
+
+ ret = usb_ep_enable(dev->ep_out);
+ if (ret) {
+ usb_ep_disable(dev->ep_in);
+ return ret;
+ }
+
+ ret = config_ep_by_speed(cdev->gadget, f, dev->ep_intr);
+ if (ret)
+ return ret;
+
+ ret = usb_ep_enable(dev->ep_intr);
+ if (ret) {
+ usb_ep_disable(dev->ep_out);
+ usb_ep_disable(dev->ep_in);
+ return ret;
+ }
+ dev->state = STATE_READY;
+
+ /* readers may be blocked waiting for us to go online */
+ wake_up(&dev->read_wq);
+ return 0;
+}
+
+static void mtp_function_disable(struct usb_function *f)
+{
+ struct mtp_dev *dev = func_to_mtp(f);
+ struct usb_composite_dev *cdev = dev->cdev;
+
+ DBG(cdev, "mtp_function_disable\n");
+ dev->state = STATE_OFFLINE;
+ usb_ep_disable(dev->ep_in);
+ usb_ep_disable(dev->ep_out);
+ usb_ep_disable(dev->ep_intr);
+
+ /* readers may be blocked waiting for us to go online */
+ wake_up(&dev->read_wq);
+
+ VDBG(cdev, "%s disabled\n", dev->function.name);
+}
+
+static int mtp_setup(struct mtp_instance *fi_mtp)
+{
+ struct mtp_dev *dev;
+ int ret;
+
+ dev = kzalloc(sizeof(*dev), GFP_KERNEL);
+
+ if (fi_mtp != NULL)
+ fi_mtp->dev = dev;
+
+ if (!dev)
+ return -ENOMEM;
+
+ spin_lock_init(&dev->lock);
+ init_waitqueue_head(&dev->read_wq);
+ init_waitqueue_head(&dev->write_wq);
+ init_waitqueue_head(&dev->intr_wq);
+ atomic_set(&dev->open_excl, 0);
+ atomic_set(&dev->ioctl_excl, 0);
+ INIT_LIST_HEAD(&dev->tx_idle);
+ INIT_LIST_HEAD(&dev->intr_idle);
+
+ dev->wq = create_singlethread_workqueue("f_mtp");
+ if (!dev->wq) {
+ ret = -ENOMEM;
+ goto err1;
+ }
+ INIT_WORK(&dev->send_file_work, send_file_work);
+ INIT_WORK(&dev->receive_file_work, receive_file_work);
+
+ _mtp_dev = dev;
+
+ ret = misc_register(&mtp_device);
+ if (ret)
+ goto err2;
+
+ return 0;
+
+err2:
+ destroy_workqueue(dev->wq);
+err1:
+ _mtp_dev = NULL;
+ kfree(dev);
+ pr_err("mtp gadget driver failed to initialize\n");
+ return ret;
+}
+
+static void mtp_cleanup(void)
+{
+ struct mtp_dev *dev = _mtp_dev;
+
+ if (!dev)
+ return;
+
+ misc_deregister(&mtp_device);
+ destroy_workqueue(dev->wq);
+ _mtp_dev = NULL;
+ kfree(dev);
+}
+
+static struct mtp_instance *to_mtp_instance(struct config_item *item)
+{
+ return container_of(to_config_group(item), struct mtp_instance,
+ func_inst.group);
+}
+
+static void mtp_attr_release(struct config_item *item)
+{
+ struct mtp_instance *fi_mtp = to_mtp_instance(item);
+
+ usb_put_function_instance(&fi_mtp->func_inst);
+}
+
+static struct configfs_item_operations mtp_item_ops = {
+ .release = mtp_attr_release,
+};
+
+static struct config_item_type mtp_func_type = {
+ .ct_item_ops = &mtp_item_ops,
+ .ct_owner = THIS_MODULE,
+};
+
+static struct mtp_instance *to_fi_mtp(struct usb_function_instance *fi)
+{
+ return container_of(fi, struct mtp_instance, func_inst);
+}
+
+static int mtp_set_inst_name(struct usb_function_instance *fi, const char *name)
+{
+ struct mtp_instance *fi_mtp;
+ char *ptr;
+ int name_len;
+
+ name_len = strlen(name) + 1;
+ if (name_len > MAX_INST_NAME_LEN)
+ return -ENAMETOOLONG;
+
+ ptr = kstrndup(name, name_len, GFP_KERNEL);
+ if (!ptr)
+ return -ENOMEM;
+
+ fi_mtp = to_fi_mtp(fi);
+ fi_mtp->name = ptr;
+
+ return 0;
+}
+
+static void mtp_free_inst(struct usb_function_instance *fi)
+{
+ struct mtp_instance *fi_mtp;
+
+ fi_mtp = to_fi_mtp(fi);
+ kfree(fi_mtp->name);
+ mtp_cleanup();
+ kfree(fi_mtp);
+}
+
+static struct usb_function_instance *mtp_alloc_inst(void)
+{
+ struct mtp_instance *fi_mtp;
+ int ret = 0;
+
+ fi_mtp = kzalloc(sizeof(*fi_mtp), GFP_KERNEL);
+ if (!fi_mtp)
+ return ERR_PTR(-ENOMEM);
+ fi_mtp->func_inst.set_inst_name = mtp_set_inst_name;
+ fi_mtp->func_inst.free_func_inst = mtp_free_inst;
+
+ ret = mtp_setup(fi_mtp);
+ if (ret) {
+ kfree(fi_mtp);
+ pr_err("Error setting MTP\n");
+ return ERR_PTR(ret);
+ }
+
+ config_group_init_type_name(&fi_mtp->func_inst.group,
+ "", &mtp_func_type);
+
+ return &fi_mtp->func_inst;
+}
+
+static int mtp_ctrlreq_configfs(struct usb_function *f,
+ const struct usb_ctrlrequest *ctrl)
+{
+ return mtp_ctrlrequest(f->config->cdev, ctrl);
+}
+
+static void mtp_free(struct usb_function *f)
+{
+ /*NO-OP: no function specific resource allocation in mtp_alloc*/
+}
+
+static struct usb_function *mtp_alloc(struct usb_function_instance *fi)
+{
+ struct mtp_instance *fi_mtp = to_fi_mtp(fi);
+ struct mtp_dev *dev = fi_mtp->dev;
+
+ dev->function.name = DRIVER_NAME;
+ dev->function.strings = mtp_strings;
+ dev->function.fs_descriptors = fs_mtp_descs;
+ dev->function.hs_descriptors = hs_mtp_descs;
+ dev->function.bind = mtp_function_bind;
+ dev->function.unbind = mtp_function_unbind;
+ dev->function.set_alt = mtp_function_set_alt;
+ dev->function.disable = mtp_function_disable;
+ dev->function.setup = mtp_ctrlreq_configfs;
+ dev->function.free_func = mtp_free;
+
+ return &dev->function;
+}
+
+DECLARE_USB_FUNCTION_INIT(mtp, mtp_alloc_inst, mtp_alloc);
+MODULE_LICENSE("GPL");
diff --git a/include/linux/usb/f_mtp.h b/include/linux/usb/f_mtp.h
new file mode 100644
index 0000000..4e84177
--- /dev/null
+++ b/include/linux/usb/f_mtp.h
@@ -0,0 +1,23 @@
+/*
+ * Gadget Function Driver for MTP
+ *
+ * Copyright (C) 2010 Google, Inc.
+ * Author: Mike Lockwood <lockwood@android.com>
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * 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.
+ *
+ */
+
+#ifndef __LINUX_USB_F_MTP_H
+#define __LINUX_USB_F_MTP_H
+
+#include <uapi/linux/usb/f_mtp.h>
+
+#endif /* __LINUX_USB_F_MTP_H */
diff --git a/include/uapi/linux/usb/f_mtp.h b/include/uapi/linux/usb/f_mtp.h
new file mode 100644
index 0000000..6baa90d
--- /dev/null
+++ b/include/uapi/linux/usb/f_mtp.h
@@ -0,0 +1,61 @@
+/*
+ * MTP Userspace Interface
+ *
+ * Copyright (C) 2010 Google, Inc.
+ * Author: Mike Lockwood <lockwood@android.com>
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * 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.
+ *
+ */
+
+#ifndef _UAPI_LINUX_USB_F_MTP_H
+#define _UAPI_LINUX_USB_F_MTP_H
+
+#include <linux/ioctl.h>
+#include <linux/types.h>
+
+struct mtp_file_range {
+ /* file descriptor for file to transfer */
+ int fd;
+ /* offset in file for start of transfer */
+ loff_t offset;
+ /* number of bytes to transfer */
+ int64_t length;
+ /* MTP command ID for data header,
+ * used only for MTP_SEND_FILE_WITH_HEADER
+ */
+ uint16_t command;
+ /* MTP transaction ID for data header,
+ * used only for MTP_SEND_FILE_WITH_HEADER
+ */
+ uint32_t transaction_id;
+};
+
+struct mtp_event {
+ /* size of the event */
+ size_t length;
+ /* event data to send */
+ void *data;
+};
+
+/* Sends the specified file range to the host */
+#define MTP_SEND_FILE _IOW('M', 0, struct mtp_file_range)
+/* Receives data from the host and writes it to a file.
+ * The file is created if it does not exist.
+ */
+#define MTP_RECEIVE_FILE _IOW('M', 1, struct mtp_file_range)
+/* Sends an event to the host via the interrupt endpoint */
+#define MTP_SEND_EVENT _IOW('M', 3, struct mtp_event)
+/* Sends the specified file range to the host,
+ * with a 12 byte MTP data packet header at the beginning.
+ */
+#define MTP_SEND_FILE_WITH_HEADER _IOW('M', 4, struct mtp_file_range)
+
+#endif /* _UAPI_LINUX_USB_F_MTP_H */
--
1.9.1
^ permalink raw reply related
* [RFC][PATCH 2/2] usb: gadget: configfs: notify userspace of usb state changes
From: Amit Pundir @ 2015-08-13 19:12 UTC (permalink / raw)
To: linux-usb, linux-kernel, linux-doc, linux-api
Cc: Mike Lockwood, Benoit Goby, Colin Cross, Arve Hjønnevåg,
Peter Oh, Greg Hackmann, Badhri Jagan Sridharan,
Android Kernel Team, Greg Kroah-Hartman, Jonathan Corbet,
Felipe Balbi, Andrzej Pietrasiewicz, Laurent Pinchart,
Yegor Yefremov, Philippe Reynes, John Stultz, Sumit Semwal
In-Reply-To: <1439493140-22207-1-git-send-email-amit.pundir@linaro.org>
This is more of an RFC than an actual submission. There are few
scattered #ifdefs..#endifs here and there which still need to be
taken care of before going for actual submission.
Currently there is no way with the upstream ConfigFS gadget to
communicate state changes (connected, disconnected, configured), at
the gadget level. Instead such state changes are handled function by
function independently I presume. This is problematic, because some
coordination between the functions, across the state changes, may be
desired at the userspace level. Thus to address this issue, this
patch send uevents to allow userspace to be notified of these usb
state changes, allowing userspace to respond and configure the
configfs gadget appropriately.
This patch is based on an Android patchset originaly authored by
Badhri Jagan Sridharan <Badhri@google.com> to send uevent notifications
to Android userpace for USB state changes. I've folded his patches
together and modified it enough that I don't want him to be blamed for
any mistakes I've made condensing his patches down.
This patch introduces USB_CONFIGFS_UEVENT Kconfig to handle userspace
notifications of usb state changes, and add setup and disconnect
functions to intercept the setup requests from the usb_core. It also
creates a sysfs device class entry and a device attribute (state) to
read and respond to gadget's current state from userspace. As of now
this sysfs device class (/sys/class/android_usb) and gadget device
(/sys/class/android_usb/android0) with state attribute
(/sys/class/android_usb/android0/state) are strictly tied up to
facilitate Android userspace requests. But going forward we may want
to bring all function devices (hid, printer etc) under a unified usb
gadget device class e.g. /sys/class/usb_gadget/g_{func0,func1} etc..
Also I think it make sense to add this state attribute to the configfs
usb gadget itself i.e. have something like /config/usb_gadget/g1/state
to read USB gadget's current state. Since it is going to be consistent
throughout all the functions tied up to that gadget.
Again this is just an initial RFC, thoughts and feedback would be
greatly appreciated.
Cc: Mike Lockwood <lockwood@android.com>
Cc: Benoit Goby <benoit@android.com>
Cc: Colin Cross <ccross@android.com>
Cc: Arve Hjønnevåg <arve@android.com>
Cc: Peter Oh <poh@broadcom.com>
Cc: Greg Hackmann <ghackmann@google.com>
Cc: Badhri Jagan Sridharan <Badhri@google.com>
Cc: Android Kernel Team <kernel-team@android.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Felipe Balbi <balbi@ti.com>
Cc: Andrzej Pietrasiewicz <andrzej.p@samsung.com>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Yegor Yefremov <yegorslists@googlemail.com>
Cc: Philippe Reynes <tremyfr@gmail.com>
Cc: John Stultz <john.stultz@linaro.org>
Cc: Sumit Semwal <sumit.semwal@linaro.org>
Signed-off-by: Amit Pundir <amit.pundir@linaro.org>
---
drivers/usb/gadget/Kconfig | 8 ++
drivers/usb/gadget/configfs.c | 201 +++++++++++++++++++++++++++++++++++++++++-
2 files changed, 207 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
index 65d110d..e1d1fc1 100644
--- a/drivers/usb/gadget/Kconfig
+++ b/drivers/usb/gadget/Kconfig
@@ -464,6 +464,14 @@ config USB_CONFIGFS_F_MTP
the file level. Thus exposing the relevant content but hiding
the system/restricted files.
+config USB_CONFIGFS_UEVENT
+ bool "Uevent notification of Gadget state"
+ depends on USB_CONFIGFS
+ help
+ Enable uevent notifications to userspace when the gadget
+ state changes. The gadget can be in any of the following
+ three states: "CONNECTED/DISCONNECTED/CONFIGURED"
+
source "drivers/usb/gadget/legacy/Kconfig"
endchoice
diff --git a/drivers/usb/gadget/configfs.c b/drivers/usb/gadget/configfs.c
index 289e201..1575343 100644
--- a/drivers/usb/gadget/configfs.c
+++ b/drivers/usb/gadget/configfs.c
@@ -9,6 +9,15 @@
#include "u_f.h"
#include "u_os_desc.h"
+#ifdef CONFIG_USB_CONFIGFS_UEVENT
+#include <linux/platform_device.h>
+#include <linux/kdev_t.h>
+#include <linux/usb/ch9.h>
+
+static struct class *usb_gadget_class;
+static struct device *usb_gadget_device;
+#endif
+
int check_user_usb_string(const char *name,
struct usb_gadget_strings *stringtab_dev)
{
@@ -63,6 +72,12 @@ struct gadget_info {
bool use_os_desc;
char b_vendor_code;
char qw_sign[OS_STRING_QW_SIGN_LEN];
+#ifdef CONFIG_USB_CONFIGFS_UEVENT
+ bool connected;
+ bool sw_connected;
+ struct work_struct work;
+ struct device *dev;
+#endif
};
struct config_usb_cfg {
@@ -1444,13 +1459,143 @@ static void configfs_composite_unbind(struct usb_gadget *gadget)
set_gadget_data(gadget, NULL);
}
+#ifdef CONFIG_USB_CONFIGFS_UEVENT
+static ssize_t state_show(struct device *pdev, struct device_attribute *attr,
+ char *buf)
+{
+ struct gadget_info *dev = dev_get_drvdata(pdev);
+ struct usb_composite_dev *cdev;
+ char *state = "DISCONNECTED";
+ unsigned long flags;
+
+ if (!dev)
+ goto out;
+
+ cdev = &dev->cdev;
+
+ if (!cdev)
+ goto out;
+
+ spin_lock_irqsave(&cdev->lock, flags);
+ if (cdev->config)
+ state = "CONFIGURED";
+ else if (dev->connected)
+ state = "CONNECTED";
+ spin_unlock_irqrestore(&cdev->lock, flags);
+out:
+ return sprintf(buf, "%s\n", state);
+}
+
+static DEVICE_ATTR(state, S_IRUGO, state_show, NULL);
+
+static struct device_attribute *cfs_usb_attributes[] = {
+ &dev_attr_state,
+ NULL
+};
+
+static void cfs_uevent_work(struct work_struct *data)
+{
+ struct gadget_info *gi = container_of(data, struct gadget_info, work);
+ struct usb_composite_dev *cdev = &gi->cdev;
+ char *disconnected[2] = { "USB_STATE=DISCONNECTED", NULL };
+ char *connected[2] = { "USB_STATE=CONNECTED", NULL };
+ char *configured[2] = { "USB_STATE=CONFIGURED", NULL };
+ /* 0-connected 1-configured 2-disconnected*/
+ bool status[3] = { false, false, false };
+ unsigned long flags;
+ bool uevent_sent = false;
+
+ spin_lock_irqsave(&cdev->lock, flags);
+ if (cdev->config)
+ status[1] = true;
+
+ if (gi->connected != gi->sw_connected) {
+ if (gi->connected)
+ status[0] = true;
+ else
+ status[2] = true;
+ gi->sw_connected = gi->connected;
+ }
+ spin_unlock_irqrestore(&cdev->lock, flags);
+
+ if (status[0]) {
+ kobject_uevent_env(&usb_gadget_device->kobj,
+ KOBJ_CHANGE, connected);
+ pr_info("%s: sent uevent %s\n", __func__, connected[0]);
+ uevent_sent = true;
+ }
+
+ if (status[1]) {
+ kobject_uevent_env(&usb_gadget_device->kobj,
+ KOBJ_CHANGE, configured);
+ pr_info("%s: sent uevent %s\n", __func__, configured[0]);
+ uevent_sent = true;
+ }
+
+ if (status[2]) {
+ kobject_uevent_env(&usb_gadget_device->kobj,
+ KOBJ_CHANGE, disconnected);
+ pr_info("%s: sent uevent %s\n", __func__, disconnected[0]);
+ uevent_sent = true;
+ }
+
+ if (!uevent_sent) {
+ pr_info("%s: did not send uevent (%d %d %p)\n", __func__,
+ gi->connected, gi->sw_connected, cdev->config);
+ }
+}
+
+static int cfs_uevent_setup(struct usb_gadget *gadget,
+ const struct usb_ctrlrequest *c)
+{
+ struct usb_composite_dev *cdev = get_gadget_data(gadget);
+ unsigned long flags;
+ struct gadget_info *gi = container_of(cdev, struct gadget_info, cdev);
+ int value = -EOPNOTSUPP;
+
+ spin_lock_irqsave(&cdev->lock, flags);
+ if (!gi->connected) {
+ gi->connected = 1;
+ schedule_work(&gi->work);
+ }
+ spin_unlock_irqrestore(&cdev->lock, flags);
+
+ value = composite_setup(gadget, c);
+
+ spin_lock_irqsave(&cdev->lock, flags);
+ if (c->bRequest == USB_REQ_SET_CONFIGURATION &&
+ cdev->config) {
+ schedule_work(&gi->work);
+ }
+ spin_unlock_irqrestore(&cdev->lock, flags);
+
+ return value;
+}
+
+static void cfs_uevent_disconnect(struct usb_gadget *gadget)
+{
+ struct usb_composite_dev *cdev = get_gadget_data(gadget);
+ struct gadget_info *gi = container_of(cdev, struct gadget_info, cdev);
+
+ gi->connected = 0;
+ schedule_work(&gi->work);
+ composite_disconnect(gadget);
+}
+#endif
+
static const struct usb_gadget_driver configfs_driver_template = {
.bind = configfs_composite_bind,
.unbind = configfs_composite_unbind,
+#ifdef CONFIG_USB_CONFIGFS_UEVENT
+ .setup = cfs_uevent_setup,
+ .reset = cfs_uevent_disconnect,
+ .disconnect = cfs_uevent_disconnect,
+#else
.setup = composite_setup,
.reset = composite_disconnect,
.disconnect = composite_disconnect,
+#endif
.suspend = composite_suspend,
.resume = composite_resume,
@@ -1462,16 +1607,21 @@ static const struct usb_gadget_driver configfs_driver_template = {
},
};
+
static struct config_group *gadgets_make(
struct config_group *group,
const char *name)
{
struct gadget_info *gi;
+#ifdef CONFIG_USB_CONFIGFS_UEVENT
+ struct device_attribute **attrs;
+ struct device_attribute *attr;
+ int err;
+#endif
gi = kzalloc(sizeof(*gi), GFP_KERNEL);
if (!gi)
return ERR_PTR(-ENOMEM);
-
gi->group.default_groups = gi->default_groups;
gi->group.default_groups[0] = &gi->functions_group;
gi->group.default_groups[1] = &gi->configs_group;
@@ -1507,9 +1657,26 @@ static struct config_group *gadgets_make(
gi->composite.gadget_driver.function = kstrdup(name, GFP_KERNEL);
gi->composite.name = gi->composite.gadget_driver.function;
- if (!gi->composite.gadget_driver.function)
+#ifdef CONFIG_USB_CONFIGFS_UEVENT
+ INIT_WORK(&gi->work, cfs_uevent_work);
+ usb_gadget_device = device_create(usb_gadget_class, NULL,
+ MKDEV(0, 0), NULL, "android0");
+ if (IS_ERR(usb_gadget_device))
goto err;
+ dev_set_drvdata(usb_gadget_device, gi);
+
+ attrs = cfs_usb_attributes;
+ while ((attr = *attrs++)) {
+ err = device_create_file(usb_gadget_device, attr);
+ if (err)
+ goto err1;
+ }
+#endif
+
+ if (!gi->composite.gadget_driver.function)
+ goto err1;
+
#ifdef CONFIG_USB_OTG
gi->otg.bLength = sizeof(struct usb_otg_descriptor);
gi->otg.bDescriptorType = USB_DT_OTG;
@@ -1519,13 +1686,31 @@ static struct config_group *gadgets_make(
config_group_init_type_name(&gi->group, name,
&gadget_root_type);
return &gi->group;
+
+err1:
+#ifdef CONFIG_USB_CONFIGFS_UEVENT
+ attrs = cfs_usb_attributes;
+ while ((attr = *attrs++))
+ device_remove_file(usb_gadget_device, attr);
+ device_destroy(usb_gadget_device->class, usb_gadget_device->devt);
err:
+#endif
kfree(gi);
return ERR_PTR(-ENOMEM);
}
static void gadgets_drop(struct config_group *group, struct config_item *item)
{
+#ifdef CONFIG_USB_CONFIGFS_UEVENT
+ struct device_attribute **attrs;
+ struct device_attribute *attr;
+
+ attrs = cfs_usb_attributes;
+ while ((attr = *attrs++))
+ device_remove_file(usb_gadget_device, attr);
+ device_destroy(usb_gadget_device->class, usb_gadget_device->devt);
+#endif
+
config_item_put(item);
}
@@ -1564,6 +1749,13 @@ static int __init gadget_cfs_init(void)
config_group_init(&gadget_subsys.su_group);
ret = configfs_register_subsystem(&gadget_subsys);
+
+#ifdef CONFIG_USB_CONFIGFS_UEVENT
+ usb_gadget_class = class_create(THIS_MODULE, "android_usb");
+ if (IS_ERR(usb_gadget_class))
+ return PTR_ERR(usb_gadget_class);
+#endif
+
return ret;
}
module_init(gadget_cfs_init);
@@ -1571,5 +1763,10 @@ module_init(gadget_cfs_init);
static void __exit gadget_cfs_exit(void)
{
configfs_unregister_subsystem(&gadget_subsys);
+#ifdef CONFIG_USB_CONFIGFS_UEVENT
+ if (!IS_ERR(usb_gadget_class))
+ class_destroy(usb_gadget_class);
+#endif
+
}
module_exit(gadget_cfs_exit);
--
1.9.1
^ permalink raw reply related
* Re: [RFC][PATCH 1/2] usb: gadget: configfs: add MTP function
From: Krzysztof Opasiak @ 2015-08-13 19:34 UTC (permalink / raw)
To: Amit Pundir, linux-usb-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-doc-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA
Cc: Mike Lockwood, Benoit Goby, Colin Cross, Arve Hjønnevåg,
Peter Oh, Greg Hackmann, Badhri Jagan Sridharan,
Android Kernel Team, Greg Kroah-Hartman, Jonathan Corbet,
Felipe Balbi, Andrzej Pietrasiewicz, Laurent Pinchart,
Yegor Yefremov, Philippe Reynes, John Stultz, Sumit Semwal
In-Reply-To: <1439493140-22207-2-git-send-email-amit.pundir-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Hello,
On 08/13/2015 09:12 PM, Amit Pundir wrote:
> his MTP function is based on years of work originally done in the
> Android kernel tree by:
> Mike Lockwood<lockwood-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org>
> Benoit Goby<benoit-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org>
> Colin Cross<ccross-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org>
> Arve Hjønnevåg<arve-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org>
> Peter Oh<poh-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
> Greg Hackmann<ghackmann-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
> Badhri Jagan Sridharan<Badhri-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
> I've folded the series up to make it easier to review, and to provide
> a coherent patch description.
>
> Post Gingerbread (Android v2.3), Android dropped USB Mass Storage
> in favor of Media Transfer Protocal (MTP), which is widely used for
> transferring media files to digital music players and similar
> applications. This USB gadget function implements MTP functionalty.
>
> Historically this function has been a part of Android composite
> gadget driver. Android composite driver was Android's solution
> for dynamic gadget function switching prior to the ConfigFS gadget
> being merged. There were failed few attempts in past
> http://marc.info/?l=linux-usb&m=132451695808552 to upstream Android
> composite driver as well. Now this Android MTP gadget function has been
> re-implemented so as to be used as a generic ConfigFS function instead.
>
> Again, many thanks to Mike, Benoit, Colin, Arve, Peter, Greg and Badhri,
> as they are the real authors of this work. However, I've folded their
> patches together and modified it enough that I don't want them to be
> blamed for any mistakes I've made condensing their patches down.
>
> Cc: Mike Lockwood<lockwood-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org>
> Cc: Benoit Goby<benoit-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org>
> Cc: Colin Cross<ccross-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org>
> Cc: Arve Hjønnevåg<arve-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org>
> Cc: Peter Oh<poh-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
> Cc: Greg Hackmann<ghackmann-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
> Cc: Badhri Jagan Sridharan<Badhri-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
> Cc: Android Kernel Team<kernel-team-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org>
> Cc: Greg Kroah-Hartman<gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>
> Cc: Jonathan Corbet<corbet-T1hC0tSOHrs@public.gmane.org>
> Cc: Felipe Balbi<balbi-l0cyMroinI0@public.gmane.org>
> Cc: Andrzej Pietrasiewicz<andrzej.p-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> Cc: Laurent Pinchart<laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
> Cc: Yegor Yefremov<yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
> Cc: Philippe Reynes<tremyfr-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Cc: John Stultz<john.stultz-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Cc: Sumit Semwal<sumit.semwal-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Signed-off-by: Amit Pundir<amit.pundir-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
In my humble opinion adding such function to Linux kernel doesn't make
any sense. By design, MTP is a protocol which requires access to
userspace features esp. file system. It is very important to run MTP
daemon with suitable user and LSM label and many many other issues which
should be handled by userspace access policy.
Moreover this is not a fully functional USB function but only some
interface which can be used by mtp-responder (mtp-daemon - call it as
you like) to communicate with host. As we have FunctionFS which allows
to implement any USB function in as a userspace service. As MTP nature
is more related to userspace I think that porting MTP daemon to use this
is a right way to go. This should be much more reasonable than adding
new function which also requires daemon for proper working. So why add
another interface while we can use a generic one?
Best regards,
--
Krzysztof Opasiak
Samsung R&D Institute Poland
Samsung Electronics
^ permalink raw reply
* Re: [RFC][PATCH 2/2] usb: gadget: configfs: notify userspace of usb state changes
From: Krzysztof Opasiak @ 2015-08-13 19:42 UTC (permalink / raw)
To: Amit Pundir, linux-usb, linux-kernel, linux-doc, linux-api
Cc: Mike Lockwood, Benoit Goby, Colin Cross, Arve Hjønnevåg,
Peter Oh, Greg Hackmann, Badhri Jagan Sridharan,
Android Kernel Team, Greg Kroah-Hartman, Jonathan Corbet,
Felipe Balbi, Andrzej Pietrasiewicz, Laurent Pinchart,
Yegor Yefremov, Philippe Reynes, John Stultz, Sumit Semwal
In-Reply-To: <1439493140-22207-3-git-send-email-amit.pundir@linaro.org>
Hello,
On 08/13/2015 09:12 PM, Amit Pundir wrote:
> This is more of an RFC than an actual submission. There are few
> scattered #ifdefs..#endifs here and there which still need to be
> taken care of before going for actual submission.
>
> Currently there is no way with the upstream ConfigFS gadget to
> communicate state changes (connected, disconnected, configured), at
> the gadget level. Instead such state changes are handled function by
> function independently I presume. This is problematic, because some
> coordination between the functions, across the state changes, may be
> desired at the userspace level. Thus to address this issue, this
> patch send uevents to allow userspace to be notified of these usb
> state changes, allowing userspace to respond and configure the
> configfs gadget appropriately.
>
> This patch is based on an Android patchset originaly authored by
> Badhri Jagan Sridharan<Badhri@google.com> to send uevent notifications
> to Android userpace for USB state changes. I've folded his patches
> together and modified it enough that I don't want him to be blamed for
> any mistakes I've made condensing his patches down.
>
> This patch introduces USB_CONFIGFS_UEVENT Kconfig to handle userspace
> notifications of usb state changes, and add setup and disconnect
> functions to intercept the setup requests from the usb_core. It also
> creates a sysfs device class entry and a device attribute (state) to
> read and respond to gadget's current state from userspace. As of now
> this sysfs device class (/sys/class/android_usb) and gadget device
> (/sys/class/android_usb/android0) with state attribute
> (/sys/class/android_usb/android0/state) are strictly tied up to
> facilitate Android userspace requests. But going forward we may want
> to bring all function devices (hid, printer etc) under a unified usb
> gadget device class e.g. /sys/class/usb_gadget/g_{func0,func1} etc..
>
> Also I think it make sense to add this state attribute to the configfs
> usb gadget itself i.e. have something like /config/usb_gadget/g1/state
> to read USB gadget's current state. Since it is going to be consistent
> throughout all the functions tied up to that gadget.
>
> Again this is just an initial RFC, thoughts and feedback would be
> greatly appreciated.
>
> Cc: Mike Lockwood<lockwood@android.com>
> Cc: Benoit Goby<benoit@android.com>
> Cc: Colin Cross<ccross@android.com>
> Cc: Arve Hjønnevåg<arve@android.com>
> Cc: Peter Oh<poh@broadcom.com>
> Cc: Greg Hackmann<ghackmann@google.com>
> Cc: Badhri Jagan Sridharan<Badhri@google.com>
> Cc: Android Kernel Team<kernel-team@android.com>
> Cc: Greg Kroah-Hartman<gregkh@linuxfoundation.org>
> Cc: Jonathan Corbet<corbet@lwn.net>
> Cc: Felipe Balbi<balbi@ti.com>
> Cc: Andrzej Pietrasiewicz<andrzej.p@samsung.com>
> Cc: Laurent Pinchart<laurent.pinchart@ideasonboard.com>
> Cc: Yegor Yefremov<yegorslists@googlemail.com>
> Cc: Philippe Reynes<tremyfr@gmail.com>
> Cc: John Stultz<john.stultz@linaro.org>
> Cc: Sumit Semwal<sumit.semwal@linaro.org>
> Signed-off-by: Amit Pundir<amit.pundir@linaro.org>
Generally I agree that there should be some way of notifying userspace
about gadget state but I'm not sure if this is proper way to go. In my
opinion gadget-bus which has been discussed some time ago on linux-usb
makes much more sense than this.
Maybe I will be wrong but I guess that you are adding this feature to
make android mtp-responder working properly in Linux (not android).
Again if we use FFS instead of adding mtp function we don't need this
notification as mtp-responder can get all the required informations
about gadget (function) via ep0 using functionfs events.
Best regards,
--
Krzysztof Opasiak
Samsung R&D Institute Poland
Samsung Electronics
^ permalink raw reply
* Re: [RFC][PATCH 1/2] usb: gadget: configfs: add MTP function
From: Greg Kroah-Hartman @ 2015-08-13 19:57 UTC (permalink / raw)
To: Krzysztof Opasiak
Cc: Amit Pundir, linux-usb, linux-kernel, linux-doc, linux-api,
Mike Lockwood, Benoit Goby, Colin Cross, Arve Hjønnevåg,
Peter Oh, Greg Hackmann, Badhri Jagan Sridharan,
Android Kernel Team, Jonathan Corbet, Felipe Balbi,
Andrzej Pietrasiewicz, Laurent Pinchart, Yegor Yefremov,
Philippe Reynes, John Stultz, Sumit Semwal
In-Reply-To: <55CCF156.8010302@samsung.com>
On Thu, Aug 13, 2015 at 09:34:46PM +0200, Krzysztof Opasiak wrote:
> Hello,
>
> On 08/13/2015 09:12 PM, Amit Pundir wrote:
> >his MTP function is based on years of work originally done in the
> >Android kernel tree by:
> > Mike Lockwood<lockwood@android.com>
> > Benoit Goby<benoit@android.com>
> > Colin Cross<ccross@android.com>
> > Arve Hjønnevåg<arve@android.com>
> > Peter Oh<poh@broadcom.com>
> > Greg Hackmann<ghackmann@google.com>
> > Badhri Jagan Sridharan<Badhri@google.com>
> >I've folded the series up to make it easier to review, and to provide
> >a coherent patch description.
> >
> >Post Gingerbread (Android v2.3), Android dropped USB Mass Storage
> >in favor of Media Transfer Protocal (MTP), which is widely used for
> >transferring media files to digital music players and similar
> >applications. This USB gadget function implements MTP functionalty.
> >
> >Historically this function has been a part of Android composite
> >gadget driver. Android composite driver was Android's solution
> >for dynamic gadget function switching prior to the ConfigFS gadget
> >being merged. There were failed few attempts in past
> >http://marc.info/?l=linux-usb&m=132451695808552 to upstream Android
> >composite driver as well. Now this Android MTP gadget function has been
> >re-implemented so as to be used as a generic ConfigFS function instead.
> >
> >Again, many thanks to Mike, Benoit, Colin, Arve, Peter, Greg and Badhri,
> >as they are the real authors of this work. However, I've folded their
> >patches together and modified it enough that I don't want them to be
> >blamed for any mistakes I've made condensing their patches down.
> >
> >Cc: Mike Lockwood<lockwood@android.com>
> >Cc: Benoit Goby<benoit@android.com>
> >Cc: Colin Cross<ccross@android.com>
> >Cc: Arve Hjønnevåg<arve@android.com>
> >Cc: Peter Oh<poh@broadcom.com>
> >Cc: Greg Hackmann<ghackmann@google.com>
> >Cc: Badhri Jagan Sridharan<Badhri@google.com>
> >Cc: Android Kernel Team<kernel-team@android.com>
> >Cc: Greg Kroah-Hartman<gregkh@linuxfoundation.org>
> >Cc: Jonathan Corbet<corbet@lwn.net>
> >Cc: Felipe Balbi<balbi@ti.com>
> >Cc: Andrzej Pietrasiewicz<andrzej.p@samsung.com>
> >Cc: Laurent Pinchart<laurent.pinchart@ideasonboard.com>
> >Cc: Yegor Yefremov<yegorslists@googlemail.com>
> >Cc: Philippe Reynes<tremyfr@gmail.com>
> >Cc: John Stultz<john.stultz@linaro.org>
> >Cc: Sumit Semwal<sumit.semwal@linaro.org>
> >Signed-off-by: Amit Pundir<amit.pundir@linaro.org>
>
> In my humble opinion adding such function to Linux kernel doesn't make any
> sense. By design, MTP is a protocol which requires access to userspace
> features esp. file system. It is very important to run MTP daemon with
> suitable user and LSM label and many many other issues which should be
> handled by userspace access policy.
>
> Moreover this is not a fully functional USB function but only some interface
> which can be used by mtp-responder (mtp-daemon - call it as you like) to
> communicate with host. As we have FunctionFS which allows to implement any
> USB function in as a userspace service. As MTP nature is more related to
> userspace I think that porting MTP daemon to use this is a right way to go.
> This should be much more reasonable than adding new function which also
> requires daemon for proper working. So why add another interface while we
> can use a generic one?
Isn't there already a userspace MTP daemon that uses the existing
functionfs for usb gadgets? I thought I remember seeing that
somewhere...
thanks,
greg k-h
^ permalink raw reply
* Re: [RFC][PATCH 1/2] usb: gadget: configfs: add MTP function
From: Krzysztof Opasiak @ 2015-08-13 20:23 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Amit Pundir, linux-usb-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-doc-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA, Mike Lockwood, Benoit Goby,
Colin Cross, Arve Hjønnevåg, Peter Oh, Greg Hackmann,
Badhri Jagan Sridharan, Android Kernel Team, Jonathan Corbet,
Felipe Balbi, Andrzej Pietrasiewicz, Laurent Pinchart,
Yegor Yefremov, Philippe Reynes, John Stultz, Sumit Semwal
In-Reply-To: <20150813195748.GB30092-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org>
On 08/13/2015 09:57 PM, Greg Kroah-Hartman wrote:
> On Thu, Aug 13, 2015 at 09:34:46PM +0200, Krzysztof Opasiak wrote:
>> Hello,
>>
>> On 08/13/2015 09:12 PM, Amit Pundir wrote:
>>> his MTP function is based on years of work originally done in the
>>> Android kernel tree by:
>>> Mike Lockwood<lockwood-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org>
>>> Benoit Goby<benoit-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org>
>>> Colin Cross<ccross-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org>
>>> Arve Hjønnevåg<arve-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org>
>>> Peter Oh<poh-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
>>> Greg Hackmann<ghackmann-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
>>> Badhri Jagan Sridharan<Badhri-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
>>> I've folded the series up to make it easier to review, and to provide
>>> a coherent patch description.
>>>
>>> Post Gingerbread (Android v2.3), Android dropped USB Mass Storage
>>> in favor of Media Transfer Protocal (MTP), which is widely used for
>>> transferring media files to digital music players and similar
>>> applications. This USB gadget function implements MTP functionalty.
>>>
>>> Historically this function has been a part of Android composite
>>> gadget driver. Android composite driver was Android's solution
>>> for dynamic gadget function switching prior to the ConfigFS gadget
>>> being merged. There were failed few attempts in past
>>> http://marc.info/?l=linux-usb&m=132451695808552 to upstream Android
>>> composite driver as well. Now this Android MTP gadget function has been
>>> re-implemented so as to be used as a generic ConfigFS function instead.
>>>
>>> Again, many thanks to Mike, Benoit, Colin, Arve, Peter, Greg and Badhri,
>>> as they are the real authors of this work. However, I've folded their
>>> patches together and modified it enough that I don't want them to be
>>> blamed for any mistakes I've made condensing their patches down.
>>>
>>> Cc: Mike Lockwood<lockwood-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org>
>>> Cc: Benoit Goby<benoit-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org>
>>> Cc: Colin Cross<ccross-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org>
>>> Cc: Arve Hjønnevåg<arve-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org>
>>> Cc: Peter Oh<poh-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
>>> Cc: Greg Hackmann<ghackmann-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
>>> Cc: Badhri Jagan Sridharan<Badhri-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
>>> Cc: Android Kernel Team<kernel-team-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org>
>>> Cc: Greg Kroah-Hartman<gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>
>>> Cc: Jonathan Corbet<corbet-T1hC0tSOHrs@public.gmane.org>
>>> Cc: Felipe Balbi<balbi-l0cyMroinI0@public.gmane.org>
>>> Cc: Andrzej Pietrasiewicz<andrzej.p-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
>>> Cc: Laurent Pinchart<laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
>>> Cc: Yegor Yefremov<yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
>>> Cc: Philippe Reynes<tremyfr-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>> Cc: John Stultz<john.stultz-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>>> Cc: Sumit Semwal<sumit.semwal-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>>> Signed-off-by: Amit Pundir<amit.pundir-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>>
>> In my humble opinion adding such function to Linux kernel doesn't make any
>> sense. By design, MTP is a protocol which requires access to userspace
>> features esp. file system. It is very important to run MTP daemon with
>> suitable user and LSM label and many many other issues which should be
>> handled by userspace access policy.
>>
>> Moreover this is not a fully functional USB function but only some interface
>> which can be used by mtp-responder (mtp-daemon - call it as you like) to
>> communicate with host. As we have FunctionFS which allows to implement any
>> USB function in as a userspace service. As MTP nature is more related to
>> userspace I think that porting MTP daemon to use this is a right way to go.
>> This should be much more reasonable than adding new function which also
>> requires daemon for proper working. So why add another interface while we
>> can use a generic one?
>
> Isn't there already a userspace MTP daemon that uses the existing
> functionfs for usb gadgets? I thought I remember seeing that
> somewhere...
>
I know for sure that ADB and SDB has been ported to use functionfs. I
can even see ADB with ffs backend working on my nexus 9 with 3.10 kernel.
I've seen such mtp-responder implementation but that time it has not
been published to open source. I don't know what is the current state...
Best regards,
--
Krzysztof Opasiak
Samsung R&D Institute Poland
Samsung Electronics
^ permalink raw reply
* Re: [RFC][PATCH 1/2] usb: gadget: configfs: add MTP function
From: Krzysztof Opasiak @ 2015-08-13 20:41 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Amit Pundir, linux-usb, linux-kernel, linux-doc, linux-api,
Mike Lockwood, Benoit Goby, Colin Cross, Arve Hjønnevåg,
Peter Oh, Greg Hackmann, Badhri Jagan Sridharan,
Android Kernel Team, Jonathan Corbet, Felipe Balbi,
Andrzej Pietrasiewicz, Laurent Pinchart, Yegor Yefremov,
Philippe Reynes, John Stultz, Sumit Semwal
In-Reply-To: <20150813195748.GB30092@kroah.com>
On 08/13/2015 09:57 PM, Greg Kroah-Hartman wrote:
> On Thu, Aug 13, 2015 at 09:34:46PM +0200, Krzysztof Opasiak wrote:
>> Hello,
>>
>> On 08/13/2015 09:12 PM, Amit Pundir wrote:
>>> his MTP function is based on years of work originally done in the
>>> Android kernel tree by:
>>> Mike Lockwood<lockwood@android.com>
>>> Benoit Goby<benoit@android.com>
>>> Colin Cross<ccross@android.com>
>>> Arve Hjønnevåg<arve@android.com>
>>> Peter Oh<poh@broadcom.com>
>>> Greg Hackmann<ghackmann@google.com>
>>> Badhri Jagan Sridharan<Badhri@google.com>
>>> I've folded the series up to make it easier to review, and to provide
>>> a coherent patch description.
>>>
>>> Post Gingerbread (Android v2.3), Android dropped USB Mass Storage
>>> in favor of Media Transfer Protocal (MTP), which is widely used for
>>> transferring media files to digital music players and similar
>>> applications. This USB gadget function implements MTP functionalty.
>>>
>>> Historically this function has been a part of Android composite
>>> gadget driver. Android composite driver was Android's solution
>>> for dynamic gadget function switching prior to the ConfigFS gadget
>>> being merged. There were failed few attempts in past
>>> http://marc.info/?l=linux-usb&m=132451695808552 to upstream Android
>>> composite driver as well. Now this Android MTP gadget function has been
>>> re-implemented so as to be used as a generic ConfigFS function instead.
>>>
>>> Again, many thanks to Mike, Benoit, Colin, Arve, Peter, Greg and Badhri,
>>> as they are the real authors of this work. However, I've folded their
>>> patches together and modified it enough that I don't want them to be
>>> blamed for any mistakes I've made condensing their patches down.
>>>
>>> Cc: Mike Lockwood<lockwood@android.com>
>>> Cc: Benoit Goby<benoit@android.com>
>>> Cc: Colin Cross<ccross@android.com>
>>> Cc: Arve Hjønnevåg<arve@android.com>
>>> Cc: Peter Oh<poh@broadcom.com>
>>> Cc: Greg Hackmann<ghackmann@google.com>
>>> Cc: Badhri Jagan Sridharan<Badhri@google.com>
>>> Cc: Android Kernel Team<kernel-team@android.com>
>>> Cc: Greg Kroah-Hartman<gregkh@linuxfoundation.org>
>>> Cc: Jonathan Corbet<corbet@lwn.net>
>>> Cc: Felipe Balbi<balbi@ti.com>
>>> Cc: Andrzej Pietrasiewicz<andrzej.p@samsung.com>
>>> Cc: Laurent Pinchart<laurent.pinchart@ideasonboard.com>
>>> Cc: Yegor Yefremov<yegorslists@googlemail.com>
>>> Cc: Philippe Reynes<tremyfr@gmail.com>
>>> Cc: John Stultz<john.stultz@linaro.org>
>>> Cc: Sumit Semwal<sumit.semwal@linaro.org>
>>> Signed-off-by: Amit Pundir<amit.pundir@linaro.org>
>>
>> In my humble opinion adding such function to Linux kernel doesn't make any
>> sense. By design, MTP is a protocol which requires access to userspace
>> features esp. file system. It is very important to run MTP daemon with
>> suitable user and LSM label and many many other issues which should be
>> handled by userspace access policy.
>>
>> Moreover this is not a fully functional USB function but only some interface
>> which can be used by mtp-responder (mtp-daemon - call it as you like) to
>> communicate with host. As we have FunctionFS which allows to implement any
>> USB function in as a userspace service. As MTP nature is more related to
>> userspace I think that porting MTP daemon to use this is a right way to go.
>> This should be much more reasonable than adding new function which also
>> requires daemon for proper working. So why add another interface while we
>> can use a generic one?
>
> Isn't there already a userspace MTP daemon that uses the existing
> functionfs for usb gadgets? I thought I remember seeing that
> somewhere...
>
I've found some interesting link[2] which may mean that Sailfish OS guys
has some mtp implementation with functionfs backend:
<<<<< cite
- /dev/mtp
mtp functionfs rw,relatime
>>>>> cite
Started digging and got it!
This looks like mtp with ffs backend:
https://github.com/nemomobile/buteo-mtp
Didn't tested, even didn't try to compile, no guarantee;)
Footnotes:
1 -
http://reviewjolla.blogspot.com/2014/06/techspecs-android-on-jolla-phone.html
Best regards,
--
Krzysztof Opasiak
Samsung R&D Institute Poland
Samsung Electronics
^ 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