* Re: [PATCHv2 2/2] userns: control capabilities of some user namespaces
From: Serge E. Hallyn @ 2017-11-28 23:04 UTC (permalink / raw)
To: Mahesh Bandewar (महेश बंडेवार)
Cc: Serge E. Hallyn, Mahesh Bandewar, LKML, Netdev, Kernel-hardening,
Linux API, Kees Cook, Eric W . Biederman, Eric Dumazet,
David Miller
In-Reply-To: <CAF2d9jhiZFiFODNXUY13F+4Pm+59+rZW=CnLPX1VUT3BAJdeeA@mail.gmail.com>
Quoting Mahesh Bandewar (महेश बंडेवार) (maheshb@google.com):
...
> >> diff --git a/security/commoncap.c b/security/commoncap.c
> >> index fc46f5b85251..89103f16ac37 100644
> >> --- a/security/commoncap.c
> >> +++ b/security/commoncap.c
> >> @@ -73,6 +73,14 @@ int cap_capable(const struct cred *cred, struct user_namespace *targ_ns,
> >> {
> >> struct user_namespace *ns = targ_ns;
> >>
> >> + /* If the capability is controlled and user-ns that process
> >> + * belongs-to is 'controlled' then return EPERM and no need
> >> + * to check the user-ns hierarchy.
> >> + */
> >> + if (is_user_ns_controlled(cred->user_ns) &&
> >> + is_capability_controlled(cap))
> >> + return -EPERM;
> >
> > I'd be curious to see the performance impact on this on a regular
> > workload (kernel build?) in a controlled ns.
> >
> Should it affect? If at all, it should be +ve since, the recursive
> user-ns hierarchy lookup is avoided with the above check if the
> capability is controlled.
Yes but I expect that to be the rare case for normal lxc installs
(which are of course what I am interested in)
> The additional cost otherwise is this check
> per cap_capable() call.
And pipeline refetching?
Capability calls also shouldn't be all that frequent, but still I'm
left wondering...
^ permalink raw reply
* Re: bpf trees integration for linux-next
From: Stephen Rothwell @ 2017-11-28 22:52 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: Alexei Starovoitov, David Miller, Networking
In-Reply-To: <b07014c8-3c58-119b-e814-98486c0fbf69@iogearbox.net>
Hi Daniel,
On Tue, 28 Nov 2017 16:33:00 +0100 Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> Alexei (Cc) and myself are maintaining the bpf and bpf-next tree for
> upstream:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf.git/
> https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git/
>
> Is it possible that you could integrate the two into linux-next as
> well, that would help us a lot with regards to being aware of conflicts
> with any other trees. The BPF trees are usually pulled into net and
> net-next by David S. Miller.
>
> Patches are coming in via netdev@vger.kernel.org list as well, so it
> would be great if possible if notifications could Cc netdev@vger.kernel.org
> and Alexei and myself.
Added from today. I used the master branch in each case. You might
consider adding your trees to the MAINTAINERS file.
Thanks for adding your subsystem tree as a participant of linux-next. As
you may know, this is not a judgement of your code. The purpose of
linux-next is for integration testing and to lower the impact of
conflicts between subsystems in the next merge window.
You will need to ensure that the patches/commits in your tree/series have
been:
* submitted under GPL v2 (or later) and include the Contributor's
Signed-off-by,
* posted to the relevant mailing list,
* reviewed by you (or another maintainer of your subsystem tree),
* successfully unit tested, and
* destined for the current or next Linux merge window.
Basically, this should be just what you would send to Linus (or ask him
to fetch). It is allowed to be rebased if you deem it necessary.
--
Cheers,
Stephen Rothwell
sfr@canb.auug.org.au
^ permalink raw reply
* Re: [PATCH v5 next 1/5] modules:capabilities: add request_module_cap()
From: Djalal Harouni @ 2017-11-28 22:52 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: Kees Cook, Andy Lutomirski, Andrew Morton, James Morris,
Ben Hutchings, Solar Designer, Serge Hallyn, Jessica Yu,
Rusty Russell, LKML, linux-security-module, kernel-hardening,
Jonathan Corbet, Ingo Molnar, David S. Miller,
Network Development, Peter Zijlstra, Linus Torvalds
In-Reply-To: <20171128221856.GS729@wotan.suse.de>
On Tue, Nov 28, 2017 at 11:18 PM, Luis R. Rodriguez <mcgrof@kernel.org> wrote:
> On Tue, Nov 28, 2017 at 10:33:27PM +0100, Djalal Harouni wrote:
>> On Tue, Nov 28, 2017 at 10:16 PM, Luis R. Rodriguez <mcgrof@kernel.org> wrote:
>> > On Tue, Nov 28, 2017 at 12:11:34PM -0800, Kees Cook wrote:
>> >> On Tue, Nov 28, 2017 at 11:14 AM, Luis R. Rodriguez <mcgrof@kernel.org> wrote:
>> >> > kmod is just a helper to poke userpsace to load a module, that's it.
>> >> >
>> >> > The old init_module() and newer finit_module() do the real handy work or
>> >> > module loading, and both currently only use may_init_module():
>> >> >
>> >> > static int may_init_module(void)
>> >> > {
>> >> > if (!capable(CAP_SYS_MODULE) || modules_disabled)
>> >> > return -EPERM;
>> >> >
>> >> > return 0;
>> >> > }
>> >> >
>> >> > This begs the question:
>> >> >
>> >> > o If userspace just tries to just use raw finit_module() do we want similar
>> >> > checks?
>> >> >
>> >> > Otherwise, correct me if I'm wrong this all seems pointless.
>> >>
>> >> Hm? That's direct-loading, not auto-loading. This series is only about
>> >> auto-loading.
>> >
>> > And *all* auto-loading uses aliases? What's the difference between auto-loading
>> > and direct-loading?
>>
>> Not all auto-loading uses aliases, auto-loading is when kernel code
>> calls request_module() to loads the feature that was not present,
>
> It seems the actual interest here is system call implicated request_module()
> calls? Because there are uses of request_module() which may be module hacks,
> and not implicated via system calls.
Indeed.
>> and direct-loading in this thread is the direct syscalls like
>> finit_module().
>
> OK.
>
>> >> We already have a global sysctl for blocking direct-loading (modules_disabled).
>> >
>> > My point was that even if you have a CAP_NET_ADMIN check on request_module(),
>> > finit_module() will not check for it, so a crafty userspace could still try
>> > to just finit_module() directly, and completely then bypass the CAP_NET_ADMIN
>> > check.
>>
>> The finit_module() uses CAP_SYS_MODULE which should allow all modules
>> and in this context it should be more privileged than CAP_NET_ADMIN
>> which is only for "netdev-%s" (to not load arbitrary modules with it).
>>
>> finit_module() coming from request_module() always has the
>> CAP_NET_ADMIN, hence the check is done before.
>
> But since CAP_SYS_MODULE is more restrictive, what's the point in checking
> for CAP_NET_ADMIN?
For backward compatibility with 'netdev' modules since it is for those.
>> > So unless I'm missing something, I see no point in adding extra checks for
>> > request_module() but nothing for the respective load_module().
>>
>> I see, request_module() is called from kernel context which runs in
>> init namespace will full capabilities, the spawned userspace modprobe
>> will get CAP_SYS_MODULE and all other caps, then after comes modprobe
>> and load_module().
>
> Right, so defining the gains of adding this extra check is not very clear
> yet. It would seem a benefit exists, what is it?
it will able to filter if the request_module() should continue loading
the module or deny it which prevents spawning the *privileged*
usermode helper. This is all based on are we allowed to load new
features or not, or IOW I don't want to allow new features or modules
autoloading from now and on, as stated in the cover letter for various
benefit including security, reduce the amount of kernel code running,
but also do not allow new features for anyone like tunneling, etc.
>> Btw as suggested by Linus I will update with request_module_cap() and > I can
>> offer my help maintaining these bits too.
>
> Can you start by extending lib/test_module.c and
> tools/testing/selftests/kmod/kmod.sh with a proof of concept of the gains here,
> as well as ensuring things work as expected ?
Alright Luis, thanks for the hint, yes I will make sure to cover these.
For gains, kees already answered in the other email, and please check
the DCCP exploit and others linked in the cover letter.
Thank you!
> Luis
--
tixxdz
^ permalink raw reply
* Re: [PATCH] leds: trigger: Introduce a NETDEV trigger
From: Andrew Lunn @ 2017-11-28 22:50 UTC (permalink / raw)
To: Randy Dunlap
Cc: Ben Whitten, rpurdie, jacek.anaszewski, pavel, linux-leds,
linux-kernel, netdev
In-Reply-To: <f6edbdbb-2b9d-5ee6-7a74-0567ff7d9ded@infradead.org>
> What LEDs do LED triggers manipulate? I.e., where are they?
Any LEDs which the LED subsystem controls.
This is emulating what PHYs can do with there LEDs, but using generic
LEDs.
Andrew
^ permalink raw reply
* Re: [PATCH v5 next 1/5] modules:capabilities: add request_module_cap()
From: Luis R. Rodriguez @ 2017-11-28 22:48 UTC (permalink / raw)
To: Kees Cook
Cc: Luis R. Rodriguez, Djalal Harouni, Andy Lutomirski, Andrew Morton,
James Morris, Ben Hutchings, Solar Designer, Serge Hallyn,
Jessica Yu, Rusty Russell, LKML, linux-security-module,
kernel-hardening, Jonathan Corbet, Ingo Molnar, David S. Miller,
Network Development, Peter Zijlstra, Linus Torvalds
In-Reply-To: <CAGXu5jKqMeBUB4iArqw9_iyKqm3NjpCrU_byy90kZrAHDb0c9w@mail.gmail.com>
On Tue, Nov 28, 2017 at 02:18:18PM -0800, Kees Cook wrote:
> On Tue, Nov 28, 2017 at 2:12 PM, Luis R. Rodriguez <mcgrof@kernel.org> wrote:
> > On Tue, Nov 28, 2017 at 01:39:58PM -0800, Kees Cook wrote:
> >> On Tue, Nov 28, 2017 at 1:16 PM, Luis R. Rodriguez <mcgrof@kernel.org> wrote:
> >> > And *all* auto-loading uses aliases? What's the difference between auto-loading
> >> > and direct-loading?
> >>
> >> The difference is the process privileges. Unprivilged autoloading
> >> (e.g. int n_hdlc = N_HDLC; ioctl(fd,
> >> TIOCSETD, &n_hdlc)), triggers a privileged call to finit_module()
> >> under CAP_SYS_MODULE.
> >
> > Ah, so system call implicated request_module() calls.
>
> Yup. Unprivileged user does something that ultimately hits a
> request_module() in the kernel. Then the kernel calls out with the
> usermode helper (which has CAP_SYS_MODULE) and calls finit_module().
Thanks, using this terminology is much better to understand than auto-loading,
given it does make it clear an unprivileged call was one that initiated the
request_module() call, there are many uses of request_module() which *are*
privileged.
> > OK and since CAP_SYS_MODULE is much more restrictive one could argue, what's the
> > point here?
>
> The goal is to block an unprivileged user from being able to trigger a
> module load without blocking root from loading modules directly.
I see now. Do we have an audit of all system calls which implicate a
request_module() call? Networking is a good example for sure to start
off with but I was curious if we have a grasp of how wide spread this
could be.
I'll go review the patches again now with all this in mind.
Luis
^ permalink raw reply
* Re: [PATCH] leds: trigger: Introduce a NETDEV trigger
From: Randy Dunlap @ 2017-11-28 22:41 UTC (permalink / raw)
To: Ben Whitten, rpurdie, jacek.anaszewski, pavel
Cc: linux-leds, linux-kernel, netdev
In-Reply-To: <1511906058-30649-2-git-send-email-ben.whitten@gmail.com>
On 11/28/2017 01:54 PM, Ben Whitten wrote:
> diff --git a/drivers/leds/trigger/Kconfig b/drivers/leds/trigger/Kconfig
> index 3f9ddb9..4ec1853 100644
> --- a/drivers/leds/trigger/Kconfig
> +++ b/drivers/leds/trigger/Kconfig
> @@ -126,4 +126,11 @@ config LEDS_TRIGGER_PANIC
> a different trigger.
> If unsure, say Y.
>
> +config LEDS_TRIGGER_NETDEV
> + tristate "LED Netdev Trigger"
> + depends on NET && LEDS_TRIGGERS
> + help
> + This allows LEDs to be controlled by network device activity.
> + If unsure, say Y.
> +
> endif # LEDS_TRIGGERS
What LEDs do LED triggers manipulate? I.e., where are they?
--
~confuzed
^ permalink raw reply
* Re: [PATCH] [RFC v3] packet: experimental support for 64-bit timestamps
From: Willem de Bruijn @ 2017-11-28 22:28 UTC (permalink / raw)
To: Arnd Bergmann
Cc: David S. Miller, Willem de Bruijn, Björn Töpel,
Richard Cochran, Thomas Gleixner, Mike Maloney, Eric Dumazet,
Kees Cook, Hans Liljestrand, Andrey Konovalov, Rosen, Rami,
Reshetova, Elena, Sowmini Varadhan, Network Development, LKML
In-Reply-To: <20171128203346.1582725-1-arnd@arndb.de>
On Tue, Nov 28, 2017 at 3:32 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> As I noticed in my previous patch to remove the 'timespec' usage in
> the packet socket, the timestamps in the packet socket are slightly
> inefficient as they convert a nanosecond value into seconds/nanoseconds
> or seconds/microseconds.
>
> This adds two new socket options for the timestamp to resolve that:
>
> PACKET_SKIPTIMESTAMP sets a flag to indicate whether to generate
> timestamps at all. When this is set, all timestamps are hardcoded to
> zero, which saves a few cycles for the conversion and the access of
> the hardware clocksource. The idea was taken from pktgen, which has an
> F_NO_TIMESTAMP option for the same purpose.
>
> PACKET_TIMESTAMP_NS64 changes the interpretation of the time stamp fields:
> instead of having 32 bits for seconds plus 32 bits for nanoseconds or
> microseconds, we now always send down 64 bits worth of nanoseconds when
> this flag is set.
>
> Link: https://patchwork.kernel.org/patch/10077199/
> Suggested-by: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
This works. Another option would be to add a PACKET_TIMESTAMP_EX
with the semantics we discussed previously + fail hard when any undefined
bits are set. I don't feel strong either way, we don't intend to extend further.
If taking this approach, it might be good to split into separate patches, one
for each flag?
> -static __u32 tpacket_get_timestamp(struct sk_buff *skb, struct timespec64 *ts,
> +static __u32 tpacket_get_timestamp(struct sk_buff *skb, __u32 *hi, __u32 *lo,
> unsigned int flags)
Argument flags is no longer used.
> {
> + struct packet_sock *po = pkt_sk(skb->sk);
> struct skb_shared_hwtstamps *shhwtstamps = skb_hwtstamps(skb);
> + ktime_t stamp;
> + u32 type;
> +
> + if (po->tp_skiptstamp)
> + return 0;
>
> if (shhwtstamps &&
> - (flags & SOF_TIMESTAMPING_RAW_HARDWARE) &&
> - ktime_to_timespec64_cond(shhwtstamps->hwtstamp, ts))
> - return TP_STATUS_TS_RAW_HARDWARE;
> + (po->tp_tstamp & SOF_TIMESTAMPING_RAW_HARDWARE) &&
> + shhwtstamps->hwtstamp) {
> + stamp = shhwtstamps->hwtstamp;
> + type = TP_STATUS_TS_RAW_HARDWARE;
> + } else if (skb->tstamp) {
> + stamp = skb->tstamp;
> + type = TP_STATUS_TS_SOFTWARE;
> + } else {
> + return 0;
> + }
>
> - if (ktime_to_timespec64_cond(skb->tstamp, ts))
> - return TP_STATUS_TS_SOFTWARE;
> + if (po->tp_tstamp_ns64) {
> + __u64 ns = ktime_to_ns(stamp);
>
> - return 0;
> + *hi = upper_32_bits(ns);
> + *lo = lower_32_bits(ns);
> + } else {
> + struct timespec64 ts = ktime_to_timespec64(stamp);
> +
> + *hi = ts.tv_sec;
> + if (po->tp_version == TPACKET_V1)
Very minor: may want to invert test to make newer the protocols the
likely branch.
> static __u32 __packet_set_timestamp(struct packet_sock *po, void *frame,
> struct sk_buff *skb)
> {
> union tpacket_uhdr h;
> - struct timespec64 ts;
> - __u32 ts_status;
> + __u32 ts_status, hi, lo;
>
> - if (!(ts_status = tpacket_get_timestamp(skb, &ts, po->tp_tstamp)))
> + if (!(ts_status = tpacket_get_timestamp(skb, &hi, &lo, po->tp_tstamp)))
> return 0;
>
> h.raw = frame;
> - /*
> - * versions 1 through 3 overflow the timestamps in y2106, since they
> - * all store the seconds in a 32-bit unsigned integer.
> - * If we create a version 4, that should have a 64-bit timestamp,
> - * either 64-bit seconds + 32-bit nanoseconds, or just 64-bit
> - * nanoseconds.
> - */
Probably no need to introduce this in patch 1/2 when removing it in 2/2.
> @@ -2191,8 +2226,8 @@ static int tpacket_rcv(struct sk_buff *skb, struct net_device *dev,
> unsigned long status = TP_STATUS_USER;
> unsigned short macoff, netoff, hdrlen;
> struct sk_buff *copy_skb = NULL;
> - struct timespec64 ts;
> __u32 ts_status;
> + __u32 hi, lo;
since this function is not time-specific, the context of hi and lo is not
immediately obvious here. tstamp_hi, tstamp_lo? Or even __u32
tstamp[2] and have tpacket_get_timestamp and packet_get_time take
one fewer argument.
^ permalink raw reply
* Re: [PATCH v5 next 1/5] modules:capabilities: add request_module_cap()
From: Luis R. Rodriguez @ 2017-11-28 22:18 UTC (permalink / raw)
To: Djalal Harouni
Cc: Luis R. Rodriguez, Kees Cook, Andy Lutomirski, Andrew Morton,
James Morris, Ben Hutchings, Solar Designer, Serge Hallyn,
Jessica Yu, Rusty Russell, LKML, linux-security-module,
kernel-hardening, Jonathan Corbet, Ingo Molnar, David S. Miller,
Network Development, Peter Zijlstra, Linus Torvalds
In-Reply-To: <CAEiveUf3Zdw=+MPTHRSRsNq3o9M0XOgnXBVszdsGAPvhyS5wLw@mail.gmail.com>
On Tue, Nov 28, 2017 at 10:33:27PM +0100, Djalal Harouni wrote:
> On Tue, Nov 28, 2017 at 10:16 PM, Luis R. Rodriguez <mcgrof@kernel.org> wrote:
> > On Tue, Nov 28, 2017 at 12:11:34PM -0800, Kees Cook wrote:
> >> On Tue, Nov 28, 2017 at 11:14 AM, Luis R. Rodriguez <mcgrof@kernel.org> wrote:
> >> > kmod is just a helper to poke userpsace to load a module, that's it.
> >> >
> >> > The old init_module() and newer finit_module() do the real handy work or
> >> > module loading, and both currently only use may_init_module():
> >> >
> >> > static int may_init_module(void)
> >> > {
> >> > if (!capable(CAP_SYS_MODULE) || modules_disabled)
> >> > return -EPERM;
> >> >
> >> > return 0;
> >> > }
> >> >
> >> > This begs the question:
> >> >
> >> > o If userspace just tries to just use raw finit_module() do we want similar
> >> > checks?
> >> >
> >> > Otherwise, correct me if I'm wrong this all seems pointless.
> >>
> >> Hm? That's direct-loading, not auto-loading. This series is only about
> >> auto-loading.
> >
> > And *all* auto-loading uses aliases? What's the difference between auto-loading
> > and direct-loading?
>
> Not all auto-loading uses aliases, auto-loading is when kernel code
> calls request_module() to loads the feature that was not present,
It seems the actual interest here is system call implicated request_module()
calls? Because there are uses of request_module() which may be module hacks,
and not implicated via system calls.
> and direct-loading in this thread is the direct syscalls like
> finit_module().
OK.
> >> We already have a global sysctl for blocking direct-loading (modules_disabled).
> >
> > My point was that even if you have a CAP_NET_ADMIN check on request_module(),
> > finit_module() will not check for it, so a crafty userspace could still try
> > to just finit_module() directly, and completely then bypass the CAP_NET_ADMIN
> > check.
>
> The finit_module() uses CAP_SYS_MODULE which should allow all modules
> and in this context it should be more privileged than CAP_NET_ADMIN
> which is only for "netdev-%s" (to not load arbitrary modules with it).
>
> finit_module() coming from request_module() always has the
> CAP_NET_ADMIN, hence the check is done before.
But since CAP_SYS_MODULE is more restrictive, what's the point in checking
for CAP_NET_ADMIN?
> > So unless I'm missing something, I see no point in adding extra checks for
> > request_module() but nothing for the respective load_module().
>
> I see, request_module() is called from kernel context which runs in
> init namespace will full capabilities, the spawned userspace modprobe
> will get CAP_SYS_MODULE and all other caps, then after comes modprobe
> and load_module().
Right, so defining the gains of adding this extra check is not very clear
yet. It would seem a benefit exists, what is it?
> Btw as suggested by Linus I will update with request_module_cap() and > I can
> offer my help maintaining these bits too.
Can you start by extending lib/test_module.c and
tools/testing/selftests/kmod/kmod.sh with a proof of concept of the gains here,
as well as ensuring things work as expected ?
Luis
^ permalink raw reply
* Re: [PATCH v5 next 1/5] modules:capabilities: add request_module_cap()
From: Kees Cook @ 2017-11-28 22:18 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: Djalal Harouni, Andy Lutomirski, Andrew Morton, James Morris,
Ben Hutchings, Solar Designer, Serge Hallyn, Jessica Yu,
Rusty Russell, LKML, linux-security-module, kernel-hardening,
Jonathan Corbet, Ingo Molnar, David S. Miller,
Network Development, Peter Zijlstra, Linus Torvalds
In-Reply-To: <20171128221237.GR729@wotan.suse.de>
On Tue, Nov 28, 2017 at 2:12 PM, Luis R. Rodriguez <mcgrof@kernel.org> wrote:
> On Tue, Nov 28, 2017 at 01:39:58PM -0800, Kees Cook wrote:
>> On Tue, Nov 28, 2017 at 1:16 PM, Luis R. Rodriguez <mcgrof@kernel.org> wrote:
>> > And *all* auto-loading uses aliases? What's the difference between auto-loading
>> > and direct-loading?
>>
>> The difference is the process privileges. Unprivilged autoloading
>> (e.g. int n_hdlc = N_HDLC; ioctl(fd,
>> TIOCSETD, &n_hdlc)), triggers a privileged call to finit_module()
>> under CAP_SYS_MODULE.
>
> Ah, so system call implicated request_module() calls.
Yup. Unprivileged user does something that ultimately hits a
request_module() in the kernel. Then the kernel calls out with the
usermode helper (which has CAP_SYS_MODULE) and calls finit_module().
> OK and since CAP_SYS_MODULE is much more restrictive one could argue, what's the
> point here?
The goal is to block an unprivileged user from being able to trigger a
module load without blocking root from loading modules directly.
-Kees
--
Kees Cook
Pixel Security
^ permalink raw reply
* Re: [PATCH v5 next 1/5] modules:capabilities: add request_module_cap()
From: Luis R. Rodriguez @ 2017-11-28 22:12 UTC (permalink / raw)
To: Kees Cook
Cc: Luis R. Rodriguez, Djalal Harouni, Andy Lutomirski, Andrew Morton,
James Morris, Ben Hutchings, Solar Designer, Serge Hallyn,
Jessica Yu, Rusty Russell, LKML, linux-security-module,
kernel-hardening, Jonathan Corbet, Ingo Molnar, David S. Miller,
Network Development, Peter Zijlstra, Linus Torvalds
In-Reply-To: <CAGXu5j+JaFCo+pZKvet==Kh7GQgNXi30P50SMF4cpOGoUw-8ug@mail.gmail.com>
On Tue, Nov 28, 2017 at 01:39:58PM -0800, Kees Cook wrote:
> On Tue, Nov 28, 2017 at 1:16 PM, Luis R. Rodriguez <mcgrof@kernel.org> wrote:
> > And *all* auto-loading uses aliases? What's the difference between auto-loading
> > and direct-loading?
>
> The difference is the process privileges. Unprivilged autoloading
> (e.g. int n_hdlc = N_HDLC; ioctl(fd,
> TIOCSETD, &n_hdlc)), triggers a privileged call to finit_module()
> under CAP_SYS_MODULE.
Ah, so system call implicated request_module() calls.
> >> We already have a global sysctl for blocking direct-loading (modules_disabled).
> >
> > My point was that even if you have a CAP_NET_ADMIN check on request_module(),
> > finit_module() will not check for it, so a crafty userspace could still try
> > to just finit_module() directly, and completely then bypass the CAP_NET_ADMIN
> > check.
>
> You need CAP_SYS_MODULE to run finit_module().
OK and since CAP_SYS_MODULE is much more restrictive one could argue, what's the
point here?
Luis
^ permalink raw reply
* [PATCH] leds: trigger: Introduce a NETDEV trigger
From: Ben Whitten @ 2017-11-28 21:54 UTC (permalink / raw)
To: rpurdie, jacek.anaszewski, pavel
Cc: linux-leds, linux-kernel, netdev, Ben Whitten
In-Reply-To: <1511906058-30649-1-git-send-email-ben.whitten@gmail.com>
This commit introduces a NETDEV trigger for named device
activity. Available triggers are link, rx, and tx.
Signed-off-by: Ben Whitten <ben.whitten@gmail.com>
---
.../ABI/testing/sysfs-class-led-trigger-netdev | 45 +++
drivers/leds/trigger/Kconfig | 7 +
drivers/leds/trigger/Makefile | 1 +
drivers/leds/trigger/ledtrig-netdev.c | 428 +++++++++++++++++++++
4 files changed, 481 insertions(+)
create mode 100644 Documentation/ABI/testing/sysfs-class-led-trigger-netdev
create mode 100644 drivers/leds/trigger/ledtrig-netdev.c
diff --git a/Documentation/ABI/testing/sysfs-class-led-trigger-netdev b/Documentation/ABI/testing/sysfs-class-led-trigger-netdev
new file mode 100644
index 0000000..2c917df
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-class-led-trigger-netdev
@@ -0,0 +1,45 @@
+What: /sys/class/leds/<led>/device_name
+Date: Nov 2017
+KernelVersion: 4.15
+Contact: linux-leds@vger.kernel.org
+Description:
+ Specifies the network device name to monitor.
+
+What: /sys/class/leds/<led>/interval
+Date: Nov 2017
+KernelVersion: 4.15
+Contact: linux-leds@vger.kernel.org
+Description:
+ Specifies the duration of the LED blink in milliseconds.
+ Defaults to 50 ms.
+
+What: /sys/class/leds/<led>/link
+Date: Nov 2017
+KernelVersion: 4.15
+Contact: linux-leds@vger.kernel.org
+Description:
+ Signal the link state of the named network device.
+ If set to 0 (default), the LED's normal state is off.
+ If set to 1, the LED's normal state reflects the link state
+ of the named network device.
+ Setting this value also immediately changes the LED state.
+
+What: /sys/class/leds/<led>/tx
+Date: Nov 2017
+KernelVersion: 4.15
+Contact: linux-leds@vger.kernel.org
+Description:
+ Signal transmission of data on the named network device.
+ If set to 0 (default), the LED will not blink on transmission.
+ If set to 1, the LED will blink for the milliseconds specified
+ in interval to signal transmission.
+
+What: /sys/class/leds/<led>/rx
+Date: Nov 2017
+KernelVersion: 4.15
+Contact: linux-leds@vger.kernel.org
+Description:
+ Signal reception of data on the named network device.
+ If set to 0 (default), the LED will not blink on reception.
+ If set to 1, the LED will blink for the milliseconds specified
+ in interval to signal reception.
diff --git a/drivers/leds/trigger/Kconfig b/drivers/leds/trigger/Kconfig
index 3f9ddb9..4ec1853 100644
--- a/drivers/leds/trigger/Kconfig
+++ b/drivers/leds/trigger/Kconfig
@@ -126,4 +126,11 @@ config LEDS_TRIGGER_PANIC
a different trigger.
If unsure, say Y.
+config LEDS_TRIGGER_NETDEV
+ tristate "LED Netdev Trigger"
+ depends on NET && LEDS_TRIGGERS
+ help
+ This allows LEDs to be controlled by network device activity.
+ If unsure, say Y.
+
endif # LEDS_TRIGGERS
diff --git a/drivers/leds/trigger/Makefile b/drivers/leds/trigger/Makefile
index 9f2e868..59e163d 100644
--- a/drivers/leds/trigger/Makefile
+++ b/drivers/leds/trigger/Makefile
@@ -11,3 +11,4 @@ obj-$(CONFIG_LEDS_TRIGGER_DEFAULT_ON) += ledtrig-default-on.o
obj-$(CONFIG_LEDS_TRIGGER_TRANSIENT) += ledtrig-transient.o
obj-$(CONFIG_LEDS_TRIGGER_CAMERA) += ledtrig-camera.o
obj-$(CONFIG_LEDS_TRIGGER_PANIC) += ledtrig-panic.o
+obj-$(CONFIG_LEDS_TRIGGER_NETDEV) += ledtrig-netdev.o
diff --git a/drivers/leds/trigger/ledtrig-netdev.c b/drivers/leds/trigger/ledtrig-netdev.c
new file mode 100644
index 0000000..2953b41
--- /dev/null
+++ b/drivers/leds/trigger/ledtrig-netdev.c
@@ -0,0 +1,428 @@
+/*
+ * LED Kernel Netdev Trigger
+ *
+ * Toggles the LED to reflect the link and traffic state of a named net device
+ *
+ * Copyright 2017 Ben Whitten <ben.whitten@gmail.com>
+ *
+ * Copyright 2007 Oliver Jowett <oliver@opencloud.com>
+ *
+ * Derived from ledtrig-timer.c which is:
+ * Copyright 2005-2006 Openedhand Ltd.
+ * Author: Richard Purdie <rpurdie@openedhand.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/jiffies.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/list.h>
+#include <linux/spinlock.h>
+#include <linux/device.h>
+#include <linux/netdevice.h>
+#include <linux/timer.h>
+#include <linux/ctype.h>
+#include <linux/atomic.h>
+#include <linux/leds.h>
+
+/*
+ * Configurable sysfs attributes:
+ *
+ * device_name - network device name to monitor
+ *
+ * interval - duration of LED blink, in milliseconds
+ *
+ * link - LED's normal state reflects whether the link is up
+ * (has carrier) or not
+ * tx - LED blinks on transmitted data
+ * rx - LED blinks on receive data
+ *
+ */
+
+struct led_netdev_data {
+ spinlock_t lock;
+
+ struct delayed_work work;
+ struct notifier_block notifier;
+
+ struct led_classdev *led_cdev;
+ struct net_device *net_dev;
+
+ char device_name[IFNAMSIZ];
+ atomic_t interval;
+ unsigned int last_activity;
+
+ unsigned long mode;
+#define LED_BLINK_link 0
+#define LED_BLINK_tx 1
+#define LED_BLINK_rx 2
+#define LED_MODE_LINKUP 3
+};
+
+static void set_baseline_state(struct led_netdev_data *trigger_data)
+{
+ if (!test_bit(LED_MODE_LINKUP, &trigger_data->mode))
+ led_set_brightness(trigger_data->led_cdev, LED_OFF);
+ else {
+ if (test_bit(LED_BLINK_link, &trigger_data->mode))
+ led_set_brightness(trigger_data->led_cdev, LED_FULL);
+
+ if (test_bit(LED_BLINK_tx, &trigger_data->mode) ||
+ test_bit(LED_BLINK_rx, &trigger_data->mode))
+ schedule_delayed_work(&trigger_data->work,
+ atomic_read(&trigger_data->interval));
+ }
+}
+
+static ssize_t device_name_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct led_classdev *led_cdev = dev_get_drvdata(dev);
+ struct led_netdev_data *trigger_data = led_cdev->trigger_data;
+ ssize_t len;
+
+ spin_lock_bh(&trigger_data->lock);
+ len = sprintf(buf, "%s\n", trigger_data->device_name);
+ spin_unlock_bh(&trigger_data->lock);
+
+ return len;
+}
+
+static ssize_t device_name_store(struct device *dev,
+ struct device_attribute *attr, const char *buf,
+ size_t size)
+{
+ struct led_classdev *led_cdev = dev_get_drvdata(dev);
+ struct led_netdev_data *trigger_data = led_cdev->trigger_data;
+
+ if (size >= IFNAMSIZ)
+ return -EINVAL;
+
+ cancel_delayed_work_sync(&trigger_data->work);
+
+ spin_lock_bh(&trigger_data->lock);
+
+ if (trigger_data->net_dev) {
+ dev_put(trigger_data->net_dev);
+ trigger_data->net_dev = NULL;
+ }
+
+ strncpy(trigger_data->device_name, buf, size);
+ if (size > 0 && trigger_data->device_name[size - 1] == '\n')
+ trigger_data->device_name[size - 1] = 0;
+
+ if (trigger_data->device_name[0] != 0)
+ trigger_data->net_dev =
+ dev_get_by_name(&init_net, trigger_data->device_name);
+
+ clear_bit(LED_MODE_LINKUP, &trigger_data->mode);
+ if (trigger_data->net_dev != NULL)
+ if (netif_carrier_ok(trigger_data->net_dev))
+ set_bit(LED_MODE_LINKUP, &trigger_data->mode);
+
+ trigger_data->last_activity = 0;
+
+ set_baseline_state(trigger_data);
+ spin_unlock_bh(&trigger_data->lock);
+
+ return size;
+}
+
+static DEVICE_ATTR_RW(device_name);
+
+#define led_mode_flags_attr(field) \
+static ssize_t field##_show(struct device *dev, \
+ struct device_attribute *attr, char *buf) \
+{ \
+ struct led_classdev *led_cdev = dev_get_drvdata(dev); \
+ struct led_netdev_data *trigger_data = led_cdev->trigger_data; \
+ \
+ return sprintf(buf, "%u\n", test_bit(LED_BLINK_##field, \
+ &trigger_data->mode)); \
+} \
+static ssize_t field##_store(struct device *dev, \
+ struct device_attribute *attr, const char *buf, size_t size) \
+{ \
+ struct led_classdev *led_cdev = dev_get_drvdata(dev); \
+ struct led_netdev_data *trigger_data = led_cdev->trigger_data; \
+ unsigned long state; \
+ int ret; \
+ \
+ ret = kstrtoul(buf, 0, &state); \
+ if (ret) \
+ return ret; \
+ \
+ cancel_delayed_work_sync(&trigger_data->work); \
+ \
+ if (state) \
+ set_bit(LED_BLINK_##field, &trigger_data->mode); \
+ else \
+ clear_bit(LED_BLINK_##field, &trigger_data->mode); \
+ \
+ set_baseline_state(trigger_data); \
+ \
+ return size; \
+} \
+static DEVICE_ATTR_RW(field);
+
+led_mode_flags_attr(link);
+led_mode_flags_attr(rx);
+led_mode_flags_attr(tx);
+
+static ssize_t interval_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct led_classdev *led_cdev = dev_get_drvdata(dev);
+ struct led_netdev_data *trigger_data = led_cdev->trigger_data;
+
+ return sprintf(buf, "%u\n",
+ jiffies_to_msecs(atomic_read(&trigger_data->interval)));
+}
+
+static ssize_t interval_store(struct device *dev,
+ struct device_attribute *attr, const char *buf,
+ size_t size)
+{
+ struct led_classdev *led_cdev = dev_get_drvdata(dev);
+ struct led_netdev_data *trigger_data = led_cdev->trigger_data;
+ unsigned long value;
+ int ret;
+
+ ret = kstrtoul(buf, 0, &value);
+ if (ret)
+ return ret;
+
+ /* impose some basic bounds on the timer interval */
+ if (value >= 5 && value <= 10000) {
+ cancel_delayed_work_sync(&trigger_data->work);
+
+ atomic_set(&trigger_data->interval, msecs_to_jiffies(value));
+ set_baseline_state(trigger_data); /* resets timer */
+ }
+
+ return size;
+}
+
+static DEVICE_ATTR_RW(interval);
+
+static int netdev_trig_notify(struct notifier_block *nb,
+ unsigned long evt, void *dv)
+{
+ struct net_device *dev =
+ netdev_notifier_info_to_dev((struct netdev_notifier_info *)dv);
+ struct led_netdev_data *trigger_data = container_of(nb,
+ struct
+ led_netdev_data,
+ notifier);
+
+ if (evt != NETDEV_UP && evt != NETDEV_DOWN && evt != NETDEV_CHANGE
+ && evt != NETDEV_REGISTER && evt != NETDEV_UNREGISTER
+ && evt != NETDEV_CHANGENAME)
+ return NOTIFY_DONE;
+
+ if (strcmp(dev->name, trigger_data->device_name))
+ return NOTIFY_DONE;
+
+ cancel_delayed_work_sync(&trigger_data->work);
+
+ spin_lock_bh(&trigger_data->lock);
+
+ clear_bit(LED_MODE_LINKUP, &trigger_data->mode);
+ switch (evt) {
+ case NETDEV_REGISTER:
+ if (trigger_data->net_dev)
+ dev_put(trigger_data->net_dev);
+ dev_hold(dev);
+ trigger_data->net_dev = dev;
+ break;
+ case NETDEV_CHANGENAME:
+ case NETDEV_UNREGISTER:
+ if (trigger_data->net_dev) {
+ dev_put(trigger_data->net_dev);
+ trigger_data->net_dev = NULL;
+ }
+ break;
+ case NETDEV_UP:
+ case NETDEV_CHANGE:
+ if (netif_carrier_ok(dev))
+ set_bit(LED_MODE_LINKUP, &trigger_data->mode);
+ break;
+ }
+
+ set_baseline_state(trigger_data);
+
+ spin_unlock_bh(&trigger_data->lock);
+
+ return NOTIFY_DONE;
+}
+
+/* here's the real work! */
+static void netdev_trig_work(struct work_struct *work)
+{
+ struct led_netdev_data *trigger_data = container_of(work,
+ struct
+ led_netdev_data,
+ work.work);
+ struct rtnl_link_stats64 *dev_stats;
+ unsigned int new_activity;
+ struct rtnl_link_stats64 temp;
+
+ if (!test_bit(LED_MODE_LINKUP, &trigger_data->mode) ||
+ !trigger_data->net_dev ||
+ (!test_bit(LED_BLINK_tx, &trigger_data->mode) &&
+ !test_bit(LED_BLINK_rx, &trigger_data->mode))) {
+ /* we don't need to do timer work, just reflect link state. */
+ led_set_brightness(trigger_data->led_cdev,
+ (test_bit
+ (LED_BLINK_link, &trigger_data->mode)
+ && test_bit(LED_MODE_LINKUP,
+ &trigger_data->mode
+ ) ? LED_FULL : LED_OFF));
+ return;
+ }
+
+ dev_stats = dev_get_stats(trigger_data->net_dev, &temp);
+ new_activity =
+ (test_bit(LED_BLINK_tx, &trigger_data->mode) ?
+ dev_stats->tx_packets : 0) +
+ (test_bit(LED_BLINK_rx, &trigger_data->mode) ?
+ dev_stats->rx_packets : 0);
+
+ if (test_bit(LED_BLINK_link, &trigger_data->mode)) {
+ /* base state is ON (link present) */
+ /* if there's no link, we don't get this far and
+ * the LED is off
+ */
+
+ /* OFF -> ON always */
+ /* ON -> OFF on activity */
+ if (trigger_data->led_cdev->brightness == LED_OFF)
+ led_set_brightness(trigger_data->led_cdev, LED_FULL);
+ else if (trigger_data->last_activity != new_activity)
+ led_set_brightness(trigger_data->led_cdev, LED_OFF);
+ } else {
+ /* base state is OFF */
+ /* ON -> OFF always */
+ /* OFF -> ON on activity */
+ if (trigger_data->led_cdev->brightness == LED_FULL)
+ led_set_brightness(trigger_data->led_cdev, LED_OFF);
+ else if (trigger_data->last_activity != new_activity)
+ led_set_brightness(trigger_data->led_cdev, LED_FULL);
+ }
+
+ trigger_data->last_activity = new_activity;
+ schedule_delayed_work(&trigger_data->work,
+ atomic_read(&trigger_data->interval));
+}
+
+static void netdev_trig_activate(struct led_classdev *led_cdev)
+{
+ struct led_netdev_data *trigger_data;
+ int rc;
+
+ trigger_data = kzalloc(sizeof(struct led_netdev_data), GFP_KERNEL);
+ if (!trigger_data)
+ return;
+
+ spin_lock_init(&trigger_data->lock);
+
+ trigger_data->notifier.notifier_call = netdev_trig_notify;
+ trigger_data->notifier.priority = 10;
+
+ INIT_DELAYED_WORK(&trigger_data->work, netdev_trig_work);
+
+ trigger_data->led_cdev = led_cdev;
+ trigger_data->net_dev = NULL;
+ trigger_data->device_name[0] = 0;
+
+ trigger_data->mode = 0;
+ atomic_set(&trigger_data->interval, msecs_to_jiffies(50));
+ trigger_data->last_activity = 0;
+
+ led_cdev->trigger_data = trigger_data;
+
+ rc = device_create_file(led_cdev->dev, &dev_attr_device_name);
+ if (rc)
+ goto err_out;
+ rc = device_create_file(led_cdev->dev, &dev_attr_link);
+ if (rc)
+ goto err_out_device_name;
+ rc = device_create_file(led_cdev->dev, &dev_attr_rx);
+ if (rc)
+ goto err_out_link;
+ rc = device_create_file(led_cdev->dev, &dev_attr_tx);
+ if (rc)
+ goto err_out_rx;
+ rc = device_create_file(led_cdev->dev, &dev_attr_interval);
+ if (rc)
+ goto err_out_tx;
+ rc = register_netdevice_notifier(&trigger_data->notifier);
+ if (rc)
+ goto err_out_interval;
+ return;
+
+err_out_interval:
+ device_remove_file(led_cdev->dev, &dev_attr_interval);
+err_out_tx:
+ device_remove_file(led_cdev->dev, &dev_attr_tx);
+err_out_rx:
+ device_remove_file(led_cdev->dev, &dev_attr_rx);
+err_out_link:
+ device_remove_file(led_cdev->dev, &dev_attr_link);
+err_out_device_name:
+ device_remove_file(led_cdev->dev, &dev_attr_device_name);
+err_out:
+ led_cdev->trigger_data = NULL;
+ kfree(trigger_data);
+}
+
+static void netdev_trig_deactivate(struct led_classdev *led_cdev)
+{
+ struct led_netdev_data *trigger_data = led_cdev->trigger_data;
+
+ if (trigger_data) {
+ unregister_netdevice_notifier(&trigger_data->notifier);
+
+ device_remove_file(led_cdev->dev, &dev_attr_device_name);
+ device_remove_file(led_cdev->dev, &dev_attr_link);
+ device_remove_file(led_cdev->dev, &dev_attr_rx);
+ device_remove_file(led_cdev->dev, &dev_attr_tx);
+ device_remove_file(led_cdev->dev, &dev_attr_interval);
+
+ cancel_delayed_work_sync(&trigger_data->work);
+
+ if (trigger_data->net_dev)
+ dev_put(trigger_data->net_dev);
+
+ kfree(trigger_data);
+ }
+}
+
+static struct led_trigger netdev_led_trigger = {
+ .name = "netdev",
+ .activate = netdev_trig_activate,
+ .deactivate = netdev_trig_deactivate,
+};
+
+static int __init netdev_trig_init(void)
+{
+ return led_trigger_register(&netdev_led_trigger);
+}
+
+static void __exit netdev_trig_exit(void)
+{
+ led_trigger_unregister(&netdev_led_trigger);
+}
+
+module_init(netdev_trig_init);
+module_exit(netdev_trig_exit);
+
+MODULE_AUTHOR("Ben Whitten <ben.whitten@gmail.com>");
+MODULE_AUTHOR("Oliver Jowett <oliver@opencloud.com>");
+MODULE_DESCRIPTION("Netdev LED trigger");
+MODULE_LICENSE("GPL");
--
2.7.4
^ permalink raw reply related
* [PATCH] Introduce a NETDEV LED trigger
From: Ben Whitten @ 2017-11-28 21:54 UTC (permalink / raw)
To: rpurdie, jacek.anaszewski, pavel
Cc: linux-leds, linux-kernel, netdev, Ben Whitten
It's quite useful to be able to trigger LEDs based on network activity,
similar mechanisms exist in routers and gateway devices.
I am looking to mainline a patch that has existed out of tree for
some time in OpenWRT/LEDE.
The patch has been updated and restructured and most of the issues raised
in the initial submission have been addressed [1].
I am also including the netdev mailing list as it was suggested previously
that if there is no activity the interval will still run and is wasteful
[1] https://lkml.org/lkml/2010/11/14/116
Ben Whitten (1):
leds: trigger: Introduce a NETDEV trigger
.../ABI/testing/sysfs-class-led-trigger-netdev | 45 +++
drivers/leds/trigger/Kconfig | 7 +
drivers/leds/trigger/Makefile | 1 +
drivers/leds/trigger/ledtrig-netdev.c | 428 +++++++++++++++++++++
4 files changed, 481 insertions(+)
create mode 100644 Documentation/ABI/testing/sysfs-class-led-trigger-netdev
create mode 100644 drivers/leds/trigger/ledtrig-netdev.c
--
2.7.4
^ permalink raw reply
* Re: pull request: Cavium Octeon III firmware
From: Steven J. Hill @ 2017-11-28 21:54 UTC (permalink / raw)
To: Ben Hutchings, linux-firmware; +Cc: netdev
In-Reply-To: <1511890206.14687.115.camel@decadent.org.uk>
On 11/28/2017 11:30 AM, Ben Hutchings wrote:
>
> The diffstat in your original pull request said that WHENCE wasn't
> updated. Maybe you corrected that after sending the pull request.
>
Ben,
The updated patch is ready for you to pull. Cheers.
Steve
git://git.linux-mips.org/pub/scm/sjhill/linux-firmware.git
^ permalink raw reply
* Re: [PATCH net-next] vmxnet3: increase default rx ring sizes
From: Shrikrishna Khare @ 2017-11-28 21:49 UTC (permalink / raw)
To: Andrew Lunn; +Cc: Shrikrishna Khare, pv-drivers, netdev, linux-kernel, jsavanyo
In-Reply-To: <20171128213327.GJ14512@lunn.ch>
On Tue, 28 Nov 2017, Andrew Lunn wrote:
> > /*
> > * Version numbers
> > */
> > -#define VMXNET3_DRIVER_VERSION_STRING "1.4.a.0-k"
> > +#define VMXNET3_DRIVER_VERSION_STRING "1.4.11.0-k"
> >
> > /* a 32-bit int, each byte encode a verion number in VMXNET3_DRIVER_VERSION */
> > -#define VMXNET3_DRIVER_VERSION_NUM 0x01040a00
> > +#define VMXNET3_DRIVER_VERSION_NUM 0x01040b00
>
> Version strings in drivers are totally pointless, but should not this
> be 0x01041100?
>
Version number is hex value written to vmxnet3 emulation, thus 0x1040b00
is correct (0x0b = 11). The VERSION_STRING should have previously been
"1.4.10.0-k", this patch rectifies that by setting correct value
"1.4.11.0-k".
I will let John Savanyo (cc'ed) comment on having the version in the
driver part.
Thanks,
Shri
^ permalink raw reply
* Re: [PATCH v5 next 1/5] modules:capabilities: add request_module_cap()
From: Kees Cook @ 2017-11-28 21:39 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: Djalal Harouni, Andy Lutomirski, Andrew Morton, James Morris,
Ben Hutchings, Solar Designer, Serge Hallyn, Jessica Yu,
Rusty Russell, LKML, linux-security-module, kernel-hardening,
Jonathan Corbet, Ingo Molnar, David S. Miller,
Network Development, Peter Zijlstra, Linus Torvalds
In-Reply-To: <20171128211659.GP729@wotan.suse.de>
On Tue, Nov 28, 2017 at 1:16 PM, Luis R. Rodriguez <mcgrof@kernel.org> wrote:
> And *all* auto-loading uses aliases? What's the difference between auto-loading
> and direct-loading?
The difference is the process privileges. Unprivilged autoloading
(e.g. int n_hdlc = N_HDLC; ioctl(fd,
TIOCSETD, &n_hdlc)), triggers a privileged call to finit_module()
under CAP_SYS_MODULE.
>> We already have a global sysctl for blocking direct-loading (modules_disabled).
>
> My point was that even if you have a CAP_NET_ADMIN check on request_module(),
> finit_module() will not check for it, so a crafty userspace could still try
> to just finit_module() directly, and completely then bypass the CAP_NET_ADMIN
> check.
You need CAP_SYS_MODULE to run finit_module().
-Kees
--
Kees Cook
Pixel Security
^ permalink raw reply
* Re: [PATCH net-next] vmxnet3: increase default rx ring sizes
From: Andrew Lunn @ 2017-11-28 21:33 UTC (permalink / raw)
To: Shrikrishna Khare; +Cc: pv-drivers, netdev, linux-kernel
In-Reply-To: <20171128211204.12234-1-skhare@vmware.com>
> /*
> * Version numbers
> */
> -#define VMXNET3_DRIVER_VERSION_STRING "1.4.a.0-k"
> +#define VMXNET3_DRIVER_VERSION_STRING "1.4.11.0-k"
>
> /* a 32-bit int, each byte encode a verion number in VMXNET3_DRIVER_VERSION */
> -#define VMXNET3_DRIVER_VERSION_NUM 0x01040a00
> +#define VMXNET3_DRIVER_VERSION_NUM 0x01040b00
Version strings in drivers are totally pointless, but should not this
be 0x01041100?
Andrew
^ permalink raw reply
* Re: [PATCH v5 next 1/5] modules:capabilities: add request_module_cap()
From: Djalal Harouni @ 2017-11-28 21:33 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: Kees Cook, Andy Lutomirski, Andrew Morton, James Morris,
Ben Hutchings, Solar Designer, Serge Hallyn, Jessica Yu,
Rusty Russell, LKML, linux-security-module, kernel-hardening,
Jonathan Corbet, Ingo Molnar, David S. Miller,
Network Development, Peter Zijlstra, Linus Torvalds
In-Reply-To: <20171128211659.GP729@wotan.suse.de>
On Tue, Nov 28, 2017 at 10:16 PM, Luis R. Rodriguez <mcgrof@kernel.org> wrote:
> On Tue, Nov 28, 2017 at 12:11:34PM -0800, Kees Cook wrote:
>> On Tue, Nov 28, 2017 at 11:14 AM, Luis R. Rodriguez <mcgrof@kernel.org> wrote:
>> > kmod is just a helper to poke userpsace to load a module, that's it.
>> >
>> > The old init_module() and newer finit_module() do the real handy work or
>> > module loading, and both currently only use may_init_module():
>> >
>> > static int may_init_module(void)
>> > {
>> > if (!capable(CAP_SYS_MODULE) || modules_disabled)
>> > return -EPERM;
>> >
>> > return 0;
>> > }
>> >
>> > This begs the question:
>> >
>> > o If userspace just tries to just use raw finit_module() do we want similar
>> > checks?
>> >
>> > Otherwise, correct me if I'm wrong this all seems pointless.
>>
>> Hm? That's direct-loading, not auto-loading. This series is only about
>> auto-loading.
>
> And *all* auto-loading uses aliases? What's the difference between auto-loading
> and direct-loading?
Not all auto-loading uses aliases, auto-loading is when kernel code
calls request_module() to loads the feature that was not present, and
direct-loading in this thread is the direct syscalls like
finit_module().
>> We already have a global sysctl for blocking direct-loading (modules_disabled).
>
> My point was that even if you have a CAP_NET_ADMIN check on request_module(),
> finit_module() will not check for it, so a crafty userspace could still try
> to just finit_module() directly, and completely then bypass the CAP_NET_ADMIN
> check.
The finit_module() uses CAP_SYS_MODULE which should allow all modules
and in this context it should be more privileged than CAP_NET_ADMIN
which is only for "netdev-%s" (to not load arbitrary modules with it).
finit_module() coming from request_module() always has the
CAP_NET_ADMIN, hence the check is done before.
> So unless I'm missing something, I see no point in adding extra checks for
> request_module() but nothing for the respective load_module().
I see, request_module() is called from kernel context which runs in
init namespace will full capabilities, the spawned userspace modprobe
will get CAP_SYS_MODULE and all other caps, then after comes modprobe
and load_module().
Btw as suggested by Linus I will update with request_module_cap() and
I can offer my help maintaining these bits too.
>
> Luis
--
tixxdz
^ permalink raw reply
* [PATCH 17/17] idr: Warn if old iterators see large IDs
From: Matthew Wilcox @ 2017-11-28 21:33 UTC (permalink / raw)
Cc: Matthew Wilcox, Chris Mi, Jiri Pirko, David S . Miller, Cong Wang,
Jamal Hadi Salim, Daniel Borkmann, Eric Biggers, Lai Jiangshan,
Tejun Heo, Rehas Sachdeva, netdev, linux-kernel
In-Reply-To: <20171128213312.28983-1-willy@infradead.org>
From: Matthew Wilcox <mawilcox@microsoft.com>
Now that the IDR can be used to store large IDs, it is possible somebody
might only partially convert their old code and use the iterators which
can only handle IDs up to INT_MAX. It's probably unwise to show them a
truncated ID, so settle for spewing warnings to dmesg, and terminating
the iteration.
Signed-off-by: Matthew Wilcox <mawilcox@microsoft.com>
---
lib/idr.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/lib/idr.c b/lib/idr.c
index 772a24513d1e..f56133a600bb 100644
--- a/lib/idr.c
+++ b/lib/idr.c
@@ -145,7 +145,11 @@ int idr_for_each(const struct idr *idr,
void __rcu **slot;
radix_tree_for_each_slot(slot, &idr->idr_rt, &iter, 0) {
- int ret = fn(iter.index, rcu_dereference_raw(*slot), data);
+ int ret;
+
+ if (WARN_ON(iter.index > INT_MAX))
+ break;
+ ret = fn(iter.index, rcu_dereference_raw(*slot), data);
if (ret)
return ret;
}
@@ -173,6 +177,9 @@ void *idr_get_next(struct idr *idr, int *nextid)
if (!slot)
return NULL;
+ if (WARN_ON_ONCE(iter.index > INT_MAX))
+ return NULL;
+
*nextid = iter.index;
return rcu_dereference_raw(*slot);
}
--
2.15.0
^ permalink raw reply related
* [PATCH 16/17] idr: Rename idr_for_each_entry_ext
From: Matthew Wilcox @ 2017-11-28 21:33 UTC (permalink / raw)
Cc: Matthew Wilcox, Chris Mi, Jiri Pirko, David S . Miller, Cong Wang,
Jamal Hadi Salim, Daniel Borkmann, Eric Biggers, Lai Jiangshan,
Tejun Heo, Rehas Sachdeva, netdev, linux-kernel
In-Reply-To: <20171128213312.28983-1-willy@infradead.org>
From: Matthew Wilcox <mawilcox@microsoft.com>
Match idr_alloc_ul with idr_get_next_ul and idr_for_each_entry_ul.
Also add kernel-doc.
Signed-off-by: Matthew Wilcox <mawilcox@microsoft.com>
---
include/linux/idr.h | 17 ++++++++++++++---
lib/idr.c | 20 +++++++++++++++-----
net/sched/act_api.c | 6 +++---
3 files changed, 32 insertions(+), 11 deletions(-)
diff --git a/include/linux/idr.h b/include/linux/idr.h
index 344380fd0887..91d27a9bcdf4 100644
--- a/include/linux/idr.h
+++ b/include/linux/idr.h
@@ -88,7 +88,7 @@ int idr_alloc_cyclic(struct idr *, void *entry, int start, int end, gfp_t);
int idr_for_each(const struct idr *,
int (*fn)(int id, void *p, void *data), void *data);
void *idr_get_next(struct idr *, int *nextid);
-void *idr_get_next_ext(struct idr *idr, unsigned long *nextid);
+void *idr_get_next_ul(struct idr *, unsigned long *nextid);
void *idr_replace(struct idr *, void *, unsigned long id);
void idr_destroy(struct idr *);
@@ -178,8 +178,19 @@ static inline void *idr_find(const struct idr *idr, unsigned long id)
*/
#define idr_for_each_entry(idr, entry, id) \
for (id = 0; ((entry) = idr_get_next(idr, &(id))) != NULL; ++id)
-#define idr_for_each_entry_ext(idr, entry, id) \
- for (id = 0; ((entry) = idr_get_next_ext(idr, &(id))) != NULL; ++id)
+
+/**
+ * idr_for_each_entry_ul() - iterate over an IDR's elements of a given type.
+ * @idr: IDR handle.
+ * @entry: The type * to use as cursor.
+ * @id: Entry ID.
+ *
+ * @entry and @id do not need to be initialized before the loop, and
+ * after normal terminatinon @entry is left with the value NULL. This
+ * is convenient for a "not found" value.
+ */
+#define idr_for_each_entry_ul(idr, entry, id) \
+ for (id = 0; ((entry) = idr_get_next_ul(idr, &(id))) != NULL; ++id)
/**
* idr_for_each_entry_continue - continue iteration over an idr's elements of a given type
diff --git a/lib/idr.c b/lib/idr.c
index 103afb97b4bd..772a24513d1e 100644
--- a/lib/idr.c
+++ b/lib/idr.c
@@ -155,9 +155,9 @@ int idr_for_each(const struct idr *idr,
EXPORT_SYMBOL(idr_for_each);
/**
- * idr_get_next - Find next populated entry
- * @idr: idr handle
- * @nextid: Pointer to lowest possible ID to return
+ * idr_get_next() - Find next populated entry.
+ * @idr: IDR handle.
+ * @nextid: Pointer to lowest possible ID to return.
*
* Returns the next populated entry in the tree with an ID greater than
* or equal to the value pointed to by @nextid. On exit, @nextid is updated
@@ -178,7 +178,17 @@ void *idr_get_next(struct idr *idr, int *nextid)
}
EXPORT_SYMBOL(idr_get_next);
-void *idr_get_next_ext(struct idr *idr, unsigned long *nextid)
+/**
+ * idr_get_next_ul() - Find next populated entry.
+ * @idr: IDR handle.
+ * @nextid: Pointer to lowest possible ID to return.
+ *
+ * Returns the next populated entry in the tree with an ID greater than
+ * or equal to the value pointed to by @nextid. On exit, @nextid is updated
+ * to the ID of the found value. To use in a loop, the value pointed to by
+ * nextid must be incremented by the user.
+ */
+void *idr_get_next_ul(struct idr *idr, unsigned long *nextid)
{
struct radix_tree_iter iter;
void __rcu **slot;
@@ -190,7 +200,7 @@ void *idr_get_next_ext(struct idr *idr, unsigned long *nextid)
*nextid = iter.index;
return rcu_dereference_raw(*slot);
}
-EXPORT_SYMBOL(idr_get_next_ext);
+EXPORT_SYMBOL(idr_get_next_ul);
/**
* idr_replace - replace pointer for given id
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index 156302c110af..4133d91b7029 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -124,7 +124,7 @@ static int tcf_dump_walker(struct tcf_idrinfo *idrinfo, struct sk_buff *skb,
s_i = cb->args[0];
- idr_for_each_entry_ext(idr, p, id) {
+ idr_for_each_entry_ul(idr, p, id) {
index++;
if (index < s_i)
continue;
@@ -181,7 +181,7 @@ static int tcf_del_walker(struct tcf_idrinfo *idrinfo, struct sk_buff *skb,
if (nla_put_string(skb, TCA_KIND, ops->kind))
goto nla_put_failure;
- idr_for_each_entry_ext(idr, p, id) {
+ idr_for_each_entry_ul(idr, p, id) {
ret = __tcf_idr_release(p, false, true);
if (ret == ACT_P_DELETED) {
module_put(ops->owner);
@@ -351,7 +351,7 @@ void tcf_idrinfo_destroy(const struct tc_action_ops *ops,
int ret;
unsigned long id = 1;
- idr_for_each_entry_ext(idr, p, id) {
+ idr_for_each_entry_ul(idr, p, id) {
ret = __tcf_idr_release(p, false, true);
if (ret == ACT_P_DELETED)
module_put(ops->owner);
--
2.15.0
^ permalink raw reply related
* [PATCH 15/17] idr: Rename idr_alloc_ext to idr_alloc_ul
From: Matthew Wilcox @ 2017-11-28 21:33 UTC (permalink / raw)
Cc: Matthew Wilcox, Chris Mi, Jiri Pirko, David S . Miller, Cong Wang,
Jamal Hadi Salim, Daniel Borkmann, Eric Biggers, Lai Jiangshan,
Tejun Heo, Rehas Sachdeva, netdev, linux-kernel
In-Reply-To: <20171128213312.28983-1-willy@infradead.org>
From: Matthew Wilcox <mawilcox@microsoft.com>
idr_alloc_ul fits better with other parts of the Linux kernel where we
need to name a function based on the types it operates on.
It uses a 'nextid' pointer argument instead of separate minimum ID and
output assigned ID, (like idr_get_next), reducing the number of arguments
by one. It also takes a 'max' argument rather than an 'end' argument
(unlike idr_alloc, but the semantics of 'end' don't work for unsigned long
arguments). And its return value is an errno, so mark it as __must_check.
Includes kernel-doc for idr_alloc_ul, which idr_alloc_ext didn't have,
and I realised we were missing a test-case where idr_alloc_cyclic wraps
around INT_MAX. Chris Mi <chrism@mellanox.com> has promised to contribute
test-cases for idr_alloc_ul.
Signed-off-by: Matthew Wilcox <mawilcox@microsoft.com>
---
include/linux/idr.h | 55 ++-------------------
include/linux/radix-tree.h | 17 +------
lib/idr.c | 99 +++++++++++++++++++++++++++++--------
lib/radix-tree.c | 3 +-
net/sched/cls_u32.c | 20 ++++----
tools/testing/radix-tree/idr-test.c | 17 +++++++
6 files changed, 111 insertions(+), 100 deletions(-)
diff --git a/include/linux/idr.h b/include/linux/idr.h
index 9b2fd6f408b2..344380fd0887 100644
--- a/include/linux/idr.h
+++ b/include/linux/idr.h
@@ -13,7 +13,6 @@
#define __IDR_H__
#include <linux/radix-tree.h>
-#include <linux/bug.h>
#include <linux/gfp.h>
#include <linux/percpu.h>
@@ -82,55 +81,9 @@ static inline void idr_set_cursor(struct idr *idr, unsigned int val)
void idr_preload(gfp_t gfp_mask);
-int idr_alloc_cmn(struct idr *idr, void *ptr, unsigned long *index,
- unsigned long start, unsigned long end, gfp_t gfp,
- bool ext);
-
-/**
- * idr_alloc - allocate an id
- * @idr: idr handle
- * @ptr: pointer to be associated with the new id
- * @start: the minimum id (inclusive)
- * @end: the maximum id (exclusive)
- * @gfp: memory allocation flags
- *
- * Allocates an unused ID in the range [start, end). Returns -ENOSPC
- * if there are no unused IDs in that range.
- *
- * Note that @end is treated as max when <= 0. This is to always allow
- * using @start + N as @end as long as N is inside integer range.
- *
- * Simultaneous modifications to the @idr are not allowed and should be
- * prevented by the user, usually with a lock. idr_alloc() may be called
- * concurrently with read-only accesses to the @idr, such as idr_find() and
- * idr_for_each_entry().
- */
-static inline int idr_alloc(struct idr *idr, void *ptr,
- int start, int end, gfp_t gfp)
-{
- unsigned long id;
- int ret;
-
- if (WARN_ON_ONCE(start < 0))
- return -EINVAL;
-
- ret = idr_alloc_cmn(idr, ptr, &id, start, end, gfp, false);
-
- if (ret)
- return ret;
-
- return id;
-}
-
-static inline int idr_alloc_ext(struct idr *idr, void *ptr,
- unsigned long *index,
- unsigned long start,
- unsigned long end,
- gfp_t gfp)
-{
- return idr_alloc_cmn(idr, ptr, index, start, end, gfp, true);
-}
-
+int idr_alloc(struct idr *, void *, int start, int end, gfp_t);
+int __must_check idr_alloc_ul(struct idr *, void *, unsigned long *nextid,
+ unsigned long max, gfp_t);
int idr_alloc_cyclic(struct idr *, void *entry, int start, int end, gfp_t);
int idr_for_each(const struct idr *,
int (*fn)(int id, void *p, void *data), void *data);
@@ -163,7 +116,7 @@ static inline int __must_check idr_alloc_u32(struct idr *idr, void *ptr,
u32 *nextid, unsigned long max, gfp_t gfp)
{
unsigned long tmp = *nextid;
- int ret = idr_alloc_ext(idr, ptr, &tmp, tmp, max + 1, gfp);
+ int ret = idr_alloc_ul(idr, ptr, &tmp, max, gfp);
*nextid = tmp;
return ret;
}
diff --git a/include/linux/radix-tree.h b/include/linux/radix-tree.h
index 23a9c89c7ad9..fc55ff31eca7 100644
--- a/include/linux/radix-tree.h
+++ b/include/linux/radix-tree.h
@@ -356,24 +356,9 @@ int radix_tree_split(struct radix_tree_root *, unsigned long index,
int radix_tree_join(struct radix_tree_root *, unsigned long index,
unsigned new_order, void *);
-void __rcu **idr_get_free_cmn(struct radix_tree_root *root,
+void __rcu **idr_get_free(struct radix_tree_root *root,
struct radix_tree_iter *iter, gfp_t gfp,
unsigned long max);
-static inline void __rcu **idr_get_free(struct radix_tree_root *root,
- struct radix_tree_iter *iter,
- gfp_t gfp,
- int end)
-{
- return idr_get_free_cmn(root, iter, gfp, end > 0 ? end - 1 : INT_MAX);
-}
-
-static inline void __rcu **idr_get_free_ext(struct radix_tree_root *root,
- struct radix_tree_iter *iter,
- gfp_t gfp,
- unsigned long end)
-{
- return idr_get_free_cmn(root, iter, gfp, end - 1);
-}
enum {
RADIX_TREE_ITER_TAG_MASK = 0x0f, /* tag index in lower nybble */
diff --git a/lib/idr.c b/lib/idr.c
index 577bfd4fe5c2..103afb97b4bd 100644
--- a/lib/idr.c
+++ b/lib/idr.c
@@ -1,4 +1,5 @@
#include <linux/bitmap.h>
+#include <linux/bug.h>
#include <linux/export.h>
#include <linux/idr.h>
#include <linux/slab.h>
@@ -7,32 +8,85 @@
DEFINE_PER_CPU(struct ida_bitmap *, ida_bitmap);
static DEFINE_SPINLOCK(simple_ida_lock);
-int idr_alloc_cmn(struct idr *idr, void *ptr, unsigned long *index,
- unsigned long start, unsigned long end, gfp_t gfp,
- bool ext)
+/**
+ * idr_alloc_ul() - Allocate a large ID.
+ * @idr: IDR handle.
+ * @ptr: Pointer to be associated with the new ID.
+ * @nextid: Pointer to minimum and new ID.
+ * @max: The maximum ID to allocate (inclusive).
+ * @gfp: Memory allocation flags.
+ *
+ * Allocates an unused ID in the range [*nextid, max] and updates the @nextid
+ * pointer with the newly assigned ID. Note that @max differs by 1 from the
+ * @end parameter to idr_alloc().
+ *
+ * The caller should provide their own locking to ensure that two concurrent
+ * modifications to the IDR are not possible. Read-only accesses to the
+ * IDR may be done under the RCU read lock or may exclude simultaneous
+ * writers.
+ *
+ * Return: 0 on success, -ENOMEM for memory allocation errors, -ENOSPC if
+ * there are no free IDs in the range.
+ */
+int idr_alloc_ul(struct idr *idr, void *ptr, unsigned long *nextid,
+ unsigned long max, gfp_t gfp)
{
struct radix_tree_iter iter;
void __rcu **slot;
if (WARN_ON_ONCE(radix_tree_is_internal_node(ptr)))
return -EINVAL;
+ if (WARN_ON_ONCE(!(idr->idr_rt.gfp_mask & ROOT_IS_IDR)))
+ idr->idr_rt.gfp_mask |= IDR_RT_MARKER;
- radix_tree_iter_init(&iter, start);
- if (ext)
- slot = idr_get_free_ext(&idr->idr_rt, &iter, gfp, end);
- else
- slot = idr_get_free(&idr->idr_rt, &iter, gfp, end);
+ radix_tree_iter_init(&iter, *nextid);
+ slot = idr_get_free(&idr->idr_rt, &iter, gfp, max);
if (IS_ERR(slot))
return PTR_ERR(slot);
radix_tree_iter_replace(&idr->idr_rt, &iter, slot, ptr);
radix_tree_iter_tag_clear(&idr->idr_rt, &iter, IDR_FREE);
- if (index)
- *index = iter.index;
+ *nextid = iter.index;
return 0;
}
-EXPORT_SYMBOL_GPL(idr_alloc_cmn);
+EXPORT_SYMBOL_GPL(idr_alloc_ul);
+
+/**
+ * idr_alloc - allocate an id
+ * @idr: idr handle
+ * @ptr: pointer to be associated with the new id
+ * @start: the minimum id (inclusive)
+ * @end: the maximum id (exclusive)
+ * @gfp: memory allocation flags
+ *
+ * Allocates an unused ID in the range [start, end). Returns -ENOSPC
+ * if there are no unused IDs in that range.
+ *
+ * Note that @end is treated as max when <= 0. This is to always allow
+ * using @start + N as @end as long as N is inside integer range.
+ *
+ * Simultaneous modifications to the @idr are not allowed and should be
+ * prevented by the user, usually with a lock. idr_alloc() may be called
+ * concurrently with read-only accesses to the @idr, such as idr_find() and
+ * idr_for_each_entry().
+ */
+int idr_alloc(struct idr *idr, void *ptr, int start, int end, gfp_t gfp)
+{
+ unsigned long id = start;
+ int ret;
+
+ if (WARN_ON_ONCE(start < 0))
+ return -EINVAL;
+
+ ret = idr_alloc_ul(idr, ptr, &id, end > 0 ? end - 1 : INT_MAX, gfp);
+
+ if (ret)
+ return ret;
+
+ return id;
+}
+EXPORT_SYMBOL_GPL(idr_alloc);
/**
* idr_alloc_cyclic - allocate new idr entry in a cyclical fashion
@@ -48,18 +102,21 @@ EXPORT_SYMBOL_GPL(idr_alloc_cmn);
*/
int idr_alloc_cyclic(struct idr *idr, void *ptr, int start, int end, gfp_t gfp)
{
- int id, curr = idr->idr_next;
-
- if (curr < start)
- curr = start;
+ unsigned long id = idr->idr_next;
+ int err, max = end > 0 ? end - 1 : INT_MAX;
- id = idr_alloc(idr, ptr, curr, end, gfp);
- if ((id == -ENOSPC) && (curr > start))
- id = idr_alloc(idr, ptr, start, curr, gfp);
+ if ((int)id < start)
+ id = start;
- if (id >= 0)
- idr->idr_next = id + 1U;
+ err = idr_alloc_ul(idr, ptr, &id, max, gfp);
+ if ((err == -ENOSPC) && (id > start)) {
+ id = start;
+ err = idr_alloc_ul(idr, ptr, &id, max, gfp);
+ }
+ if (err)
+ return err;
+ idr->idr_next = id + 1;
return id;
}
EXPORT_SYMBOL(idr_alloc_cyclic);
@@ -226,7 +283,7 @@ EXPORT_SYMBOL(idr_replace);
* bitmap, which is excessive.
*/
-#define IDA_MAX (0x80000000U / IDA_BITMAP_BITS)
+#define IDA_MAX (0x80000000U / IDA_BITMAP_BITS - 1)
/**
* ida_get_new_above - allocate new ID above or equal to a start id
diff --git a/lib/radix-tree.c b/lib/radix-tree.c
index c8d55565fafa..0a7ae3288a24 100644
--- a/lib/radix-tree.c
+++ b/lib/radix-tree.c
@@ -24,6 +24,7 @@
#include <linux/bitmap.h>
#include <linux/bitops.h>
+#include <linux/bug.h>
#include <linux/cpu.h>
#include <linux/errno.h>
#include <linux/export.h>
@@ -2135,7 +2136,7 @@ int ida_pre_get(struct ida *ida, gfp_t gfp)
}
EXPORT_SYMBOL(ida_pre_get);
-void __rcu **idr_get_free_cmn(struct radix_tree_root *root,
+void __rcu **idr_get_free(struct radix_tree_root *root,
struct radix_tree_iter *iter, gfp_t gfp,
unsigned long max)
{
diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
index 2fede6a55d04..5c36e9c0df9a 100644
--- a/net/sched/cls_u32.c
+++ b/net/sched/cls_u32.c
@@ -730,19 +730,17 @@ static int u32_delete(struct tcf_proto *tp, void *arg, bool *last)
static u32 gen_new_kid(struct tc_u_hnode *ht, u32 htid)
{
- unsigned long idr_index;
- u32 start = htid | 0x800;
- u32 max = htid | 0xFFF;
- u32 min = htid;
-
- if (idr_alloc_ext(&ht->handle_idr, NULL, &idr_index,
- start, max + 1, GFP_KERNEL)) {
- if (idr_alloc_ext(&ht->handle_idr, NULL, &idr_index,
- min + 1, max + 1, GFP_KERNEL))
- return max;
+ unsigned long index = htid | 0x800;
+ unsigned long max = htid | 0xFFF;
+
+ if (idr_alloc_ul(&ht->handle_idr, NULL, &index, max, GFP_KERNEL)) {
+ index = htid + 1;
+ if (idr_alloc_ul(&ht->handle_idr, NULL, &index, max,
+ GFP_KERNEL))
+ index = max;
}
- return (u32)idr_index;
+ return index;
}
static const struct nla_policy u32_policy[TCA_U32_MAX + 1] = {
diff --git a/tools/testing/radix-tree/idr-test.c b/tools/testing/radix-tree/idr-test.c
index 892ef8855b02..36437ade429c 100644
--- a/tools/testing/radix-tree/idr-test.c
+++ b/tools/testing/radix-tree/idr-test.c
@@ -215,6 +215,23 @@ void idr_checks(void)
assert(idr_is_empty(&idr));
+ idr_set_cursor(&idr, INT_MAX - 3UL);
+ for (i = INT_MAX - 3UL; i < INT_MAX + 3UL; i++) {
+ struct item *item;
+ unsigned int id;
+ if (i <= INT_MAX)
+ item = item_create(i, 0);
+ else
+ item = item_create(i - INT_MAX - 1, 0);
+
+ id = idr_alloc_cyclic(&idr, item, 0, 0, GFP_KERNEL);
+ assert(id == item->index);
+ }
+
+ idr_for_each(&idr, item_idr_free, &idr);
+ idr_destroy(&idr);
+ assert(idr_is_empty(&idr));
+
for (i = 1; i < 10000; i++) {
struct item *item = item_create(i, 0);
assert(idr_alloc(&idr, item, 1, 20000, GFP_KERNEL) == i);
--
2.15.0
^ permalink raw reply related
* [PATCH 14/17] cls_u32: Convert to idr_alloc_u32
From: Matthew Wilcox @ 2017-11-28 21:33 UTC (permalink / raw)
Cc: Matthew Wilcox, Chris Mi, Jiri Pirko, David S . Miller, Cong Wang,
Jamal Hadi Salim, Daniel Borkmann, Eric Biggers, Lai Jiangshan,
Tejun Heo, Rehas Sachdeva, netdev, linux-kernel
In-Reply-To: <20171128213312.28983-1-willy@infradead.org>
From: Matthew Wilcox <mawilcox@microsoft.com>
No real benefit to this classifier, but since we're allocating a u32
anyway, we should use this function.
Signed-off-by: Matthew Wilcox <mawilcox@microsoft.com>
---
net/sched/cls_u32.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
index e65b47483dc0..2fede6a55d04 100644
--- a/net/sched/cls_u32.c
+++ b/net/sched/cls_u32.c
@@ -970,8 +970,8 @@ static int u32_change(struct net *net, struct sk_buff *in_skb,
return -ENOMEM;
}
} else {
- err = idr_alloc_ext(&tp_c->handle_idr, ht, NULL,
- handle, handle + 1, GFP_KERNEL);
+ err = idr_alloc_u32(&tp_c->handle_idr, ht, &handle,
+ handle, GFP_KERNEL);
if (err) {
kfree(ht);
return err;
@@ -1020,8 +1020,7 @@ static int u32_change(struct net *net, struct sk_buff *in_skb,
if (TC_U32_HTID(handle) && TC_U32_HTID(handle^htid))
return -EINVAL;
handle = htid | TC_U32_NODE(handle);
- err = idr_alloc_ext(&ht->handle_idr, NULL, NULL,
- handle, handle + 1,
+ err = idr_alloc_u32(&ht->handle_idr, NULL, &handle, handle,
GFP_KERNEL);
if (err)
return err;
--
2.15.0
^ permalink raw reply related
* [PATCH 13/17] cls_u32: Reinstate cyclic allocation
From: Matthew Wilcox @ 2017-11-28 21:33 UTC (permalink / raw)
Cc: Matthew Wilcox, Chris Mi, Jiri Pirko, David S . Miller, Cong Wang,
Jamal Hadi Salim, Daniel Borkmann, Eric Biggers, Lai Jiangshan,
Tejun Heo, Rehas Sachdeva, netdev, linux-kernel
In-Reply-To: <20171128213312.28983-1-willy@infradead.org>
From: Matthew Wilcox <mawilcox@microsoft.com>
Commit e7614370d6f0 ("net_sched: use idr to allocate u32 filter handles)
converted htid allocation to use the IDR. The ID allocated by this
scheme changes; it used to be cyclic, but now always allocates the
lowest available. The IDR supports cyclic allocation, so just use
the right function.
Signed-off-by: Matthew Wilcox <mawilcox@microsoft.com>
---
net/sched/cls_u32.c | 14 ++++----------
1 file changed, 4 insertions(+), 10 deletions(-)
diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
index 9d48674a70e0..e65b47483dc0 100644
--- a/net/sched/cls_u32.c
+++ b/net/sched/cls_u32.c
@@ -316,19 +316,13 @@ static void *u32_get(struct tcf_proto *tp, u32 handle)
return u32_lookup_key(ht, handle);
}
+/* Protected by rtnl lock */
static u32 gen_new_htid(struct tc_u_common *tp_c, struct tc_u_hnode *ptr)
{
- unsigned long idr_index;
- int err;
-
- /* This is only used inside rtnl lock it is safe to increment
- * without read _copy_ update semantics
- */
- err = idr_alloc_ext(&tp_c->handle_idr, ptr, &idr_index,
- 1, 0x7FF, GFP_KERNEL);
- if (err)
+ int id = idr_alloc_cyclic(&tp_c->handle_idr, ptr, 1, 0x7FF, GFP_KERNEL);
+ if (id < 0)
return 0;
- return (u32)(idr_index | 0x800) << 20;
+ return (id | 0x800U) << 20;
}
static struct hlist_head *tc_u_common_hash;
--
2.15.0
^ permalink raw reply related
* [PATCH 12/17] cls_flower: Convert to idr_alloc_u32
From: Matthew Wilcox @ 2017-11-28 21:33 UTC (permalink / raw)
Cc: Matthew Wilcox, Chris Mi, Jiri Pirko, David S . Miller, Cong Wang,
Jamal Hadi Salim, Daniel Borkmann, Eric Biggers, Lai Jiangshan,
Tejun Heo, Rehas Sachdeva, netdev, linux-kernel
In-Reply-To: <20171128213312.28983-1-willy@infradead.org>
From: Matthew Wilcox <mawilcox@microsoft.com>
Use the new helper which saves a temporary variable and a few lines
of code.
Signed-off-by: Matthew Wilcox <mawilcox@microsoft.com>
---
net/sched/cls_flower.c | 26 ++++++++++----------------
1 file changed, 10 insertions(+), 16 deletions(-)
diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index ec0dc92f6104..77a7a3c883b7 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -858,7 +858,6 @@ static int fl_change(struct net *net, struct sk_buff *in_skb,
struct cls_fl_filter *fnew;
struct nlattr **tb;
struct fl_flow_mask mask = {};
- unsigned long idr_index;
int err;
if (!tca[TCA_OPTIONS])
@@ -889,21 +888,17 @@ static int fl_change(struct net *net, struct sk_buff *in_skb,
goto errout;
if (!handle) {
- err = idr_alloc_ext(&head->handle_idr, fnew, &idr_index,
- 1, 0x80000000, GFP_KERNEL);
- if (err)
- goto errout;
- fnew->handle = idr_index;
- }
-
- /* user specifies a handle and it doesn't exist */
- if (handle && !fold) {
- err = idr_alloc_ext(&head->handle_idr, fnew, &idr_index,
- handle, handle + 1, GFP_KERNEL);
- if (err)
- goto errout;
- fnew->handle = idr_index;
+ handle = 1;
+ err = idr_alloc_u32(&head->handle_idr, fnew, &handle,
+ INT_MAX, GFP_KERNEL);
+ } else if (!fold) {
+ /* user specifies a handle and it doesn't exist */
+ err = idr_alloc_u32(&head->handle_idr, fnew, &handle,
+ handle, GFP_KERNEL);
}
+ if (err)
+ goto errout;
+ fnew->handle = handle;
if (tb[TCA_FLOWER_FLAGS]) {
fnew->flags = nla_get_u32(tb[TCA_FLOWER_FLAGS]);
@@ -957,7 +952,6 @@ static int fl_change(struct net *net, struct sk_buff *in_skb,
*arg = fnew;
if (fold) {
- fnew->handle = handle;
idr_replace(&head->handle_idr, fnew, fnew->handle);
list_replace_rcu(&fold->list, &fnew->list);
tcf_unbind_filter(tp, &fold->res);
--
2.15.0
^ permalink raw reply related
* [PATCH 11/17] cls_bpf: Convert to use idr_alloc_u32
From: Matthew Wilcox @ 2017-11-28 21:33 UTC (permalink / raw)
Cc: Matthew Wilcox, Chris Mi, Jiri Pirko, David S . Miller, Cong Wang,
Jamal Hadi Salim, Daniel Borkmann, Eric Biggers, Lai Jiangshan,
Tejun Heo, Rehas Sachdeva, netdev, linux-kernel
In-Reply-To: <20171128213312.28983-1-willy@infradead.org>
From: Matthew Wilcox <mawilcox@microsoft.com>
Use the new helper. This has a modest reduction in both lines of code
and compiled code size.
Signed-off-by: Matthew Wilcox <mawilcox@microsoft.com>
---
net/sched/cls_bpf.c | 24 ++++++++++--------------
1 file changed, 10 insertions(+), 14 deletions(-)
diff --git a/net/sched/cls_bpf.c b/net/sched/cls_bpf.c
index b7c5c3150086..82050b240842 100644
--- a/net/sched/cls_bpf.c
+++ b/net/sched/cls_bpf.c
@@ -467,7 +467,6 @@ static int cls_bpf_change(struct net *net, struct sk_buff *in_skb,
struct cls_bpf_prog *oldprog = *arg;
struct nlattr *tb[TCA_BPF_MAX + 1];
struct cls_bpf_prog *prog;
- unsigned long idr_index;
int ret;
if (tca[TCA_OPTIONS] == NULL)
@@ -494,21 +493,18 @@ static int cls_bpf_change(struct net *net, struct sk_buff *in_skb,
}
if (handle == 0) {
- ret = idr_alloc_ext(&head->handle_idr, prog, &idr_index,
- 1, 0x7FFFFFFF, GFP_KERNEL);
- if (ret)
- goto errout;
- prog->handle = idr_index;
- } else {
- if (!oldprog) {
- ret = idr_alloc_ext(&head->handle_idr, prog, &idr_index,
- handle, handle + 1, GFP_KERNEL);
- if (ret)
- goto errout;
- }
- prog->handle = handle;
+ handle = 1;
+ ret = idr_alloc_u32(&head->handle_idr, prog, &handle,
+ INT_MAX, GFP_KERNEL);
+ } else if (!oldprog) {
+ ret = idr_alloc_u32(&head->handle_idr, prog, &handle,
+ handle, GFP_KERNEL);
}
+ if (ret)
+ goto errout;
+ prog->handle = handle;
+
ret = cls_bpf_set_parms(net, tp, prog, base, tb, tca[TCA_RATE], ovr);
if (ret < 0)
goto errout_idr;
--
2.15.0
^ permalink raw reply related
* [PATCH 10/17] cls_basic: Convert to use idr_alloc_u32
From: Matthew Wilcox @ 2017-11-28 21:33 UTC (permalink / raw)
Cc: Matthew Wilcox, Chris Mi, Jiri Pirko, David S . Miller, Cong Wang,
Jamal Hadi Salim, Daniel Borkmann, Eric Biggers, Lai Jiangshan,
Tejun Heo, Rehas Sachdeva, netdev, linux-kernel
In-Reply-To: <20171128213312.28983-1-willy@infradead.org>
From: Matthew Wilcox <mawilcox@microsoft.com>
Use the new helper which saves a temporary variable and a few lines of
code.
Signed-off-by: Matthew Wilcox <mawilcox@microsoft.com>
---
net/sched/cls_basic.c | 25 ++++++++++---------------
1 file changed, 10 insertions(+), 15 deletions(-)
diff --git a/net/sched/cls_basic.c b/net/sched/cls_basic.c
index 147700afcf31..529597c8c28c 100644
--- a/net/sched/cls_basic.c
+++ b/net/sched/cls_basic.c
@@ -182,7 +182,6 @@ static int basic_change(struct net *net, struct sk_buff *in_skb,
struct nlattr *tb[TCA_BASIC_MAX + 1];
struct basic_filter *fold = (struct basic_filter *) *arg;
struct basic_filter *fnew;
- unsigned long idr_index;
if (tca[TCA_OPTIONS] == NULL)
return -EINVAL;
@@ -205,21 +204,17 @@ static int basic_change(struct net *net, struct sk_buff *in_skb,
if (err < 0)
goto errout;
- if (handle) {
- fnew->handle = handle;
- if (!fold) {
- err = idr_alloc_ext(&head->handle_idr, fnew, &idr_index,
- handle, handle + 1, GFP_KERNEL);
- if (err)
- goto errout;
- }
- } else {
- err = idr_alloc_ext(&head->handle_idr, fnew, &idr_index,
- 1, 0x7FFFFFFF, GFP_KERNEL);
- if (err)
- goto errout;
- fnew->handle = idr_index;
+ if (!handle) {
+ handle = 1;
+ err = idr_alloc_u32(&head->handle_idr, fnew, &handle,
+ INT_MAX, GFP_KERNEL);
+ } else if (!fold) {
+ err = idr_alloc_u32(&head->handle_idr, fnew, &handle,
+ handle, GFP_KERNEL);
}
+ if (err)
+ goto errout;
+ fnew->handle = handle;
err = basic_set_parms(net, tp, fnew, base, tb, tca[TCA_RATE], ovr);
if (err < 0) {
--
2.15.0
^ permalink raw reply related
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