* Re: [PATCH 1/1] netlink: Deletion of an unnecessary check before the function call "__module_get"
From: David Miller @ 2014-11-19 20:28 UTC (permalink / raw)
To: elfring; +Cc: netdev, linux-kernel, kernel-janitors, cocci
In-Reply-To: <546BA747.6000703@users.sourceforge.net>
From: SF Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 18 Nov 2014 21:08:39 +0100
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Tue, 18 Nov 2014 21:03:13 +0100
>
> The __module_get() function tests whether its argument is NULL and then
> returns immediately. Thus the test around the call is not needed.
>
> This issue was detected by using the Coccinelle software.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
Applied, thanks.
^ permalink raw reply
* Re: pull-request: can 2014-11-18
From: David Miller @ 2014-11-19 20:29 UTC (permalink / raw)
To: mkl; +Cc: netdev, linux-can, kernel
In-Reply-To: <1416343067-9810-1-git-send-email-mkl@pengutronix.de>
From: Marc Kleine-Budde <mkl@pengutronix.de>
Date: Tue, 18 Nov 2014 21:37:30 +0100
> this is a pull request of 17 patches for net/master for the v3.18 release
> cycle.
Pulled, thanks Marc.
^ permalink raw reply
* Re: [RFC] situation with csum_and_copy_... API
From: David Miller @ 2014-11-19 20:31 UTC (permalink / raw)
To: viro; +Cc: torvalds, netdev, linux-kernel
In-Reply-To: <20141118212307.GU7996@ZenIV.linux.org.uk>
From: Al Viro <viro@ZenIV.linux.org.uk>
Date: Tue, 18 Nov 2014 21:23:07 +0000
> On Tue, Nov 18, 2014 at 12:49:13PM -0800, Linus Torvalds wrote:
>> "access_ok()" isn't that expensive, and removing them as unnecessary
>> is fraught with errors. We've had several cases of "oops, we used
>> __get_user() in a loop, because it generates much better code, but
>> we'd forgotten to do access_ok(), so now people can read kernel data".
>
> OK... If netdev folks can live with that for now, I've no problem with
> dropping 3/5. However, I really think we need a variant of csum-and-copy
> that would _not_ bother with access_ok() longer term. That can wait, though...
I think because of the way Al verifies things at the top level, and
how we structure access to these msg->msg_iov so strictly, these cases
of access_ok() really can safely go.
But that is just my opinion, and yes I do acknowledge that we've had
serious holes in this area in the past.
^ permalink raw reply
* Re: [patch net-next v3 8/9] net: move vlan pop/push functions into common code
From: Pravin Shelar @ 2014-11-19 20:32 UTC (permalink / raw)
To: Jiri Benc
Cc: Jiri Pirko, netdev, David Miller, Jamal Hadi Salim, Tom Herbert,
Eric Dumazet, Willem de Bruijn, Daniel Borkmann, mst, fw,
Paul.Durrant, Thomas Graf, Cong Wang
In-Reply-To: <20141119140357.5f9d0594@griffin>
On Wed, Nov 19, 2014 at 5:03 AM, Jiri Benc <jbenc@redhat.com> wrote:
> On Wed, 19 Nov 2014 00:06:19 -0800, Pravin Shelar wrote:
>> skb_reset_mac_len() sets length according to ethernet and network
>> offsets, but mpls expects mac-length to be offset to mpls header (ref.
>> skb_mpls_header()). Therefore rather than reset we need to subtract
>> VLAN_HLEN from mac_len. Same goes for vlan-push(), we can add
>> VLAN_HLEN.
>
> Subtracting VLAN_HLEN from mac_len would break the common vlan cases,
> where skb->protocol is ETH_P_8021Q and skb->mac_len is 14 on input to
> __skb_vlan_pop. After __skb_vlan_pop pops the vlan header, it changes
> skb->protocol to the protocol of the actual frame and has to leave
> skb->mac_len at 14.
>
MPLS API depends on mac-length to point to start of MPLS. OVS takes
care of this by setting mac-length of every packet it process
accordingly. If that is not in common path then we can not move this
API to common path, Since that will break OVS MPLS action.
> You'll need something more sophisticated to support MPLS here, I'm
> afraid.
>
> Jiri
>
> --
> Jiri Benc
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [patch net-next v4 5/9] vlan: introduce *vlan_hwaccel_push_inside helpers
From: Pravin Shelar @ 2014-11-19 20:32 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev, David Miller, Jamal Hadi Salim, Tom Herbert, Eric Dumazet,
Willem de Bruijn, Daniel Borkmann, mst, fw, Paul.Durrant,
Thomas Graf, Cong Wang
In-Reply-To: <1416402303-25341-6-git-send-email-jiri@resnulli.us>
On Wed, Nov 19, 2014 at 5:04 AM, Jiri Pirko <jiri@resnulli.us> wrote:
> Use them to push skb->vlan_tci into the payload and avoid code
> duplication.
>
> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
Acked-by: Pravin B Shelar <pshelar@nicira.com>
> ---
>
> v3->v4:
> - fixed error path in ovs datapath
>
> drivers/net/vxlan.c | 22 ++++++----------------
> include/linux/if_vlan.h | 34 ++++++++++++++++++++++++++++++++++
> net/core/dev.c | 8 ++------
> net/core/netpoll.c | 4 +---
> net/ipv4/geneve.c | 11 +++--------
> net/openvswitch/datapath.c | 4 +---
> net/openvswitch/vport-gre.c | 12 ++++--------
> 7 files changed, 51 insertions(+), 44 deletions(-)
>
> diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
> index bb8fbab..64d45fa 100644
> --- a/drivers/net/vxlan.c
> +++ b/drivers/net/vxlan.c
> @@ -1599,14 +1599,9 @@ static int vxlan6_xmit_skb(struct vxlan_sock *vs,
> if (unlikely(err))
> return err;
>
> - if (vlan_tx_tag_present(skb)) {
> - skb = vlan_insert_tag_set_proto(skb, skb->vlan_proto,
> - vlan_tx_tag_get(skb));
> - if (WARN_ON(!skb))
> - return -ENOMEM;
> -
> - skb->vlan_tci = 0;
> - }
> + skb = vlan_hwaccel_push_inside(skb);
> + if (WARN_ON(!skb))
> + return -ENOMEM;
>
> vxh = (struct vxlanhdr *) __skb_push(skb, sizeof(*vxh));
> vxh->vx_flags = htonl(VXLAN_FLAGS);
> @@ -1643,14 +1638,9 @@ int vxlan_xmit_skb(struct vxlan_sock *vs,
> if (unlikely(err))
> return err;
>
> - if (vlan_tx_tag_present(skb)) {
> - skb = vlan_insert_tag_set_proto(skb, skb->vlan_proto,
> - vlan_tx_tag_get(skb));
> - if (WARN_ON(!skb))
> - return -ENOMEM;
> -
> - skb->vlan_tci = 0;
> - }
> + skb = vlan_hwaccel_push_inside(skb);
> + if (WARN_ON(!skb))
> + return -ENOMEM;
>
> vxh = (struct vxlanhdr *) __skb_push(skb, sizeof(*vxh));
> vxh->vx_flags = htonl(VXLAN_FLAGS);
> diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
> index 46e4a15..291e670 100644
> --- a/include/linux/if_vlan.h
> +++ b/include/linux/if_vlan.h
> @@ -341,6 +341,40 @@ static inline struct sk_buff *vlan_insert_tag_set_proto(struct sk_buff *skb,
> return skb;
> }
>
> +/*
> + * __vlan_hwaccel_push_inside - pushes vlan tag to the payload
> + * @skb: skbuff to tag
> + *
> + * Pushes the VLAN tag from @skb->vlan_tci inside to the payload.
> + *
> + * Following the skb_unshare() example, in case of error, the calling function
> + * doesn't have to worry about freeing the original skb.
> + */
> +static inline struct sk_buff *__vlan_hwaccel_push_inside(struct sk_buff *skb)
> +{
> + skb = vlan_insert_tag_set_proto(skb, skb->vlan_proto,
> + vlan_tx_tag_get(skb));
> + if (likely(skb))
> + skb->vlan_tci = 0;
> + return skb;
> +}
> +/*
> + * vlan_hwaccel_push_inside - pushes vlan tag to the payload
> + * @skb: skbuff to tag
> + *
> + * Checks is tag is present in @skb->vlan_tci and if it is, it pushes the
> + * VLAN tag from @skb->vlan_tci inside to the payload.
> + *
> + * Following the skb_unshare() example, in case of error, the calling function
> + * doesn't have to worry about freeing the original skb.
> + */
> +static inline struct sk_buff *vlan_hwaccel_push_inside(struct sk_buff *skb)
> +{
> + if (vlan_tx_tag_present(skb))
> + skb = __vlan_hwaccel_push_inside(skb);
> + return skb;
> +}
> +
> /**
> * __vlan_hwaccel_put_tag - hardware accelerated VLAN inserting
> * @skb: skbuff to tag
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 3611e60..ac48362 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -2644,12 +2644,8 @@ static struct sk_buff *validate_xmit_vlan(struct sk_buff *skb,
> netdev_features_t features)
> {
> if (vlan_tx_tag_present(skb) &&
> - !vlan_hw_offload_capable(features, skb->vlan_proto)) {
> - skb = vlan_insert_tag_set_proto(skb, skb->vlan_proto,
> - vlan_tx_tag_get(skb));
> - if (skb)
> - skb->vlan_tci = 0;
> - }
> + !vlan_hw_offload_capable(features, skb->vlan_proto))
> + skb = __vlan_hwaccel_push_inside(skb);
> return skb;
> }
>
> diff --git a/net/core/netpoll.c b/net/core/netpoll.c
> index 65d3723..e0ad5d1 100644
> --- a/net/core/netpoll.c
> +++ b/net/core/netpoll.c
> @@ -79,8 +79,7 @@ static int netpoll_start_xmit(struct sk_buff *skb, struct net_device *dev,
>
> if (vlan_tx_tag_present(skb) &&
> !vlan_hw_offload_capable(features, skb->vlan_proto)) {
> - skb = vlan_insert_tag_set_proto(skb, skb->vlan_proto,
> - vlan_tx_tag_get(skb));
> + skb = __vlan_hwaccel_push_inside(skb);
> if (unlikely(!skb)) {
> /* This is actually a packet drop, but we
> * don't want the code that calls this
> @@ -88,7 +87,6 @@ static int netpoll_start_xmit(struct sk_buff *skb, struct net_device *dev,
> */
> goto out;
> }
> - skb->vlan_tci = 0;
> }
>
> status = netdev_start_xmit(skb, dev, txq, false);
> diff --git a/net/ipv4/geneve.c b/net/ipv4/geneve.c
> index fd430a6..a457232 100644
> --- a/net/ipv4/geneve.c
> +++ b/net/ipv4/geneve.c
> @@ -131,14 +131,9 @@ int geneve_xmit_skb(struct geneve_sock *gs, struct rtable *rt,
> if (unlikely(err))
> return err;
>
> - if (vlan_tx_tag_present(skb)) {
> - skb = vlan_insert_tag_set_proto(skb, skb->vlan_proto,
> - vlan_tx_tag_get(skb));
> - if (unlikely(!skb)
> - return -ENOMEM;
> -
> - skb->vlan_tci = 0;
> - }
> + skb = vlan_hwaccel_push_inside(skb);
> + if (unlikely(!skb))
> + return -ENOMEM;
>
> gnvh = (struct genevehdr *)__skb_push(skb, sizeof(*gnvh) + opt_len);
> geneve_build_header(gnvh, tun_flags, vni, opt_len, opt);
> diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
> index c63e60e..f37ca3e 100644
> --- a/net/openvswitch/datapath.c
> +++ b/net/openvswitch/datapath.c
> @@ -425,12 +425,10 @@ static int queue_userspace_packet(struct datapath *dp, struct sk_buff *skb,
> if (!nskb)
> return -ENOMEM;
>
> - nskb = vlan_insert_tag_set_proto(nskb, nskb->vlan_proto,
> - vlan_tx_tag_get(nskb));
> + nskb = __vlan_hwaccel_push_inside(nskb);
> if (!nskb)
> return -ENOMEM;
>
> - nskb->vlan_tci = 0;
> skb = nskb;
> }
>
> diff --git a/net/openvswitch/vport-gre.c b/net/openvswitch/vport-gre.c
> index 777cd8c..6b69df5 100644
> --- a/net/openvswitch/vport-gre.c
> +++ b/net/openvswitch/vport-gre.c
> @@ -175,14 +175,10 @@ static int gre_tnl_send(struct vport *vport, struct sk_buff *skb)
> goto err_free_rt;
> }
>
> - if (vlan_tx_tag_present(skb)) {
> - skb = vlan_insert_tag_set_proto(skb, skb->vlan_proto,
> - vlan_tx_tag_get(skb));
> - if (unlikely(!skb) {
> - err = -ENOMEM;
> - goto err_free_rt;
> - }
> - skb->vlan_tci = 0;
> + skb = vlan_hwaccel_push_inside(skb);
> + if (unlikely(!skb)) {
> + err = -ENOMEM;
> + goto err_free_rt;
> }
>
> /* Push Tunnel header. */
> --
> 1.9.3
>
^ permalink raw reply
* Re: [ovs-dev] [PATCH net] openvswitch: Fix mask generation for IPv6 labels.
From: Pravin Shelar @ 2014-11-19 20:33 UTC (permalink / raw)
To: Joe Stringer; +Cc: dev@openvswitch.org, netdev, LKML
In-Reply-To: <201411191151.40064.joestringer@nicira.com>
On Wed, Nov 19, 2014 at 11:51 AM, Joe Stringer <joestringer@nicira.com> wrote:
> On Wednesday, November 19, 2014 11:08:35 Pravin Shelar wrote:
>> On Wed, Nov 19, 2014 at 9:48 AM, Joe Stringer <joestringer@nicira.com>
> wrote:
>> > On Wednesday, November 19, 2014 00:11:01 Pravin Shelar wrote:
>> >> On Tue, Nov 18, 2014 at 11:25 PM, Joe Stringer <joestringer@nicira.com>
>> >
>> > wrote:
>> >> > On 18 November 2014 22:09, Pravin Shelar <pshelar@nicira.com> wrote:
>> >> >> On Tue, Nov 18, 2014 at 10:54 AM, Joe Stringer
>> >> >> <joestringer@nicira.com>
>> >> >>
>> >> >> wrote:
>> >> >> > When userspace doesn't provide a mask, OVS datapath generates a
>> >> >> > fully unwildcarded mask for the flow. This is done by taking a
>> >> >> > copy of the flow key, then iterating across its attributes,
>> >> >> > setting all values to 0xff. This works for most attributes, as the
>> >> >> > length of the netlink attribute typically matches the length of
>> >> >> > the value. However, IPv6 labels only use the lower 20 bits of the
>> >> >> > field. This patch makes a special case to handle this.
>> >> >> >
>> >> >> > This fixes the following error seen when installing IPv6 flows
>> >> >> > without a mask:
>> >> >> >
>> >> >> > openvswitch: netlink: Invalid IPv6 flow label value
>> >> >> > (value=ffffffff, max=fffff)
>> >> >>
>> >> >> We should allow exact match mask here rather than generating
>> >> >> wildcarded mask. So that ovs can catch invalid ipv6.label.
>> >> >
>> >> > I don't quite follow, I thought this was exact-match? (The existing
>> >> > function sets all bits to 1)
>> >>
>> >> With 0xffffffff value we can exact match on all ipv6.lable bits.
>> >
>> > The label field is only 20 bits. The other bits in the same word of the
>> > IPv6 header are for version (fixed) and traffic class (handled
>> > separately). We don't do anything with the other bits.
>>
>> This is just to make sure that we do not use those field for any thing
>> else. Masking those extra bits can hide incorrect ipv6 key extraction.
>
> Oh, I see. I meant something more like:
>
> ipv6_key->ipv6_label &= htonl(0xFFF00000);
> ipv6_key->ipv6_label |= htonl(0x000FFFFF);
>
> (Which would propagate the invalid bits from the flow key, but actually produce
> an exact match).
yes, it can wildcard unused bits.
^ permalink raw reply
* Re: [PATCH RESEND] fix #51791 - bug? mac 00:00:00:00:00:00 with natsemi DP83815 after driver load
From: David Miller @ 2014-11-19 20:35 UTC (permalink / raw)
To: devzero; +Cc: netdev
In-Reply-To: <trinity-a7b6eb2e-598a-4ad0-9df5-7567cb544671-1416347483688@3capp-webde-bs54>
From: "Roland Kletzing" <devzero@web.de>
Date: Tue, 18 Nov 2014 22:51:23 +0100
> This one should fix Bugzilla #51791
I'm sorry for being a pain, but I want to teach you how to properly
submit clean patches so that I don't need to go back and forth with
you in the future about these issues.
Your subject line needs adjusting.
After the [PATCH ...], there should be a properly subsystem prefix
followed by a colon character, which in this case could be "natsemi: "
Then, your subject line should be succinct and contain all of the
context necessary to understand exactly what the bug is about.
This means that the bug ID number doesn't belong there. It's
completely ambiguous as to what bug repository that ID is for,
and such information belongs in the commit message body.
So when you mention bug IDs in the commit message body, say where
it's from. If it's the main kernel bugzilla, say so.
Thanks.
^ permalink raw reply
* Re: [RFC] situation with csum_and_copy_... API
From: Linus Torvalds @ 2014-11-19 20:40 UTC (permalink / raw)
To: David Miller; +Cc: Al Viro, Network Development, Linux Kernel Mailing List
In-Reply-To: <20141119.153136.867017618826698045.davem@davemloft.net>
On Wed, Nov 19, 2014 at 12:31 PM, David Miller <davem@davemloft.net> wrote:
>
> But that is just my opinion, and yes I do acknowledge that we've had
> serious holes in this area in the past.
The serious holes have generally been exactly in the "upper layers
already check" camp, and then it turns out that some odd ioctl or
other thing ends up doing something odd and interesting.
If Al has actual performance profiles showing that the access_ok() is
a real problem, then fine. As a low-level optimization, I agree with
it. But not as a "let's just drop them, and make the security rules be
non-local and subtle, and require people to know the details of the
whole call-chain".
Seeing a "__get_user()" and just being able to glance up in the same
function and seeing the "access_ok()" is just a good safety net. And
means that people don't have to waste time thinking about or looking
for where the hell the security net really is.
Linus
^ permalink raw reply
* Re: [PATCH v3] fix locking regression in ipx_sendmsg and ipx_recvmsg
From: David Miller @ 2014-11-19 20:44 UTC (permalink / raw)
To: jbohac; +Cc: arnd, acme, netdev
In-Reply-To: <20141119103814.GB19092@midget.suse.cz>
From: Jiri Bohac <jbohac@suse.cz>
Date: Wed, 19 Nov 2014 11:38:14 +0100
> This fixes an old regression introduced by commit
> b0d0d915 (ipx: remove the BKL).
>
> When a recvmsg syscall blocks waiting for new data, no data can be sent on the
> same socket with sendmsg because ipx_recvmsg() sleeps with the socket locked.
>
> This breaks mars-nwe (NetWare emulator):
> - the ncpserv process reads the request using recvmsg
> - ncpserv forks and spawns nwconn
> - ncpserv calls a (blocking) recvmsg and waits for new requests
> - nwconn deadlocks in sendmsg on the same socket
>
> Commit b0d0d915 has simply replaced BKL locking with
> lock_sock/release_sock. Unlike now, BKL got unlocked while
> sleeping, so a blocking recvmsg did not block a concurrent
> sendmsg.
>
> Only keep the socket locked while actually working with the socket data and
> release it prior to calling skb_recv_datagram().
>
>
> Signed-off-by: Jiri Bohac <jbohac@suse.cz>
Please fix your Subject line to have a proper subsystem prefix, in this
case "ipx: " is sufficient.
In fact, I think your previous versions has the subject line setup
correctly wrt. this, why did you break it? :-)
> @@ -1764,6 +1764,7 @@ static int ipx_recvmsg(struct kiocb *iocb, struct socket *sock,
> struct ipxhdr *ipx = NULL;
> struct sk_buff *skb;
> int copied, rc;
> + int locked = 1;
>
> lock_sock(sk);
> /* put the autobinding in */
Please use 'bool' and true/false.
^ permalink raw reply
* Re: [PATCH net-net 0/4] Increase the limit of tuntap queues
From: Michael S. Tsirkin @ 2014-11-19 20:44 UTC (permalink / raw)
To: David Miller
Cc: pagupta, linux-kernel, netdev, jasowang, dgibson, vfalico,
edumazet, vyasevic, hkchu, wuzhy, xemul, therbert, bhutchings,
xii, stephen, jiri, sergei.shtylyov
In-Reply-To: <20141119.151628.768548269128919029.davem@davemloft.net>
On Wed, Nov 19, 2014 at 03:16:28PM -0500, David Miller wrote:
> From: Pankaj Gupta <pagupta@redhat.com>
> Date: Tue, 18 Nov 2014 21:52:54 +0530
>
> > - Accept maximum number of queues as sysctl param so that any user space
> > application like libvirt can use this value to limit number of queues. Also
> > Administrators can specify maximum number of queues by updating this sysctl
> > entry.
>
> This is the only part I don't like.
>
> Just let whoever has privileges to configure the tun device shoot
> themselves in the foot if they want to by configuring "too many"
> queues.
>
> If the virtual entity runs itself out of resources by doing something
> stupid, it's purely their problem.
Well it will run host out of kernel, no?
--
MST
^ permalink raw reply
* Re: [ovs-dev] [PATCH net] openvswitch: Fix mask generation for IPv6 labels.
From: Joe Stringer @ 2014-11-19 20:49 UTC (permalink / raw)
To: Pravin Shelar; +Cc: dev@openvswitch.org, netdev, LKML
In-Reply-To: <CALnjE+otirroKNXE7qbUHfoTifipp8LJ6g8yNsLz0dBjm3PUsQ@mail.gmail.com>
On Wednesday, November 19, 2014 12:33:10 Pravin Shelar wrote:
> On Wed, Nov 19, 2014 at 11:51 AM, Joe Stringer <joestringer@nicira.com>
wrote:
> > On Wednesday, November 19, 2014 11:08:35 Pravin Shelar wrote:
> >> On Wed, Nov 19, 2014 at 9:48 AM, Joe Stringer <joestringer@nicira.com>
> >
> > wrote:
> >> > On Wednesday, November 19, 2014 00:11:01 Pravin Shelar wrote:
> >> >> On Tue, Nov 18, 2014 at 11:25 PM, Joe Stringer
> >> >> <joestringer@nicira.com>
> >> >
> >> > wrote:
> >> >> > On 18 November 2014 22:09, Pravin Shelar <pshelar@nicira.com> wrote:
> >> >> >> On Tue, Nov 18, 2014 at 10:54 AM, Joe Stringer
> >> >> >> <joestringer@nicira.com>
> >> >> >>
> >> >> >> wrote:
> >> >> >> > When userspace doesn't provide a mask, OVS datapath generates a
> >> >> >> > fully unwildcarded mask for the flow. This is done by taking a
> >> >> >> > copy of the flow key, then iterating across its attributes,
> >> >> >> > setting all values to 0xff. This works for most attributes, as
> >> >> >> > the length of the netlink attribute typically matches the
> >> >> >> > length of the value. However, IPv6 labels only use the lower 20
> >> >> >> > bits of the field. This patch makes a special case to handle
> >> >> >> > this.
> >> >> >> >
> >> >> >> > This fixes the following error seen when installing IPv6 flows
> >> >> >> > without a mask:
> >> >> >> >
> >> >> >> > openvswitch: netlink: Invalid IPv6 flow label value
> >> >> >> > (value=ffffffff, max=fffff)
> >> >> >>
> >> >> >> We should allow exact match mask here rather than generating
> >> >> >> wildcarded mask. So that ovs can catch invalid ipv6.label.
> >> >> >
> >> >> > I don't quite follow, I thought this was exact-match? (The existing
> >> >> > function sets all bits to 1)
> >> >>
> >> >> With 0xffffffff value we can exact match on all ipv6.lable bits.
> >> >
> >> > The label field is only 20 bits. The other bits in the same word of
> >> > the IPv6 header are for version (fixed) and traffic class (handled
> >> > separately). We don't do anything with the other bits.
> >>
> >> This is just to make sure that we do not use those field for any thing
> >> else. Masking those extra bits can hide incorrect ipv6 key extraction.
> >
> > Oh, I see. I meant something more like:
> >
> > ipv6_key->ipv6_label &= htonl(0xFFF00000);
> > ipv6_key->ipv6_label |= htonl(0x000FFFFF);
> >
> > (Which would propagate the invalid bits from the flow key, but actually
> > produce an exact match).
>
> yes, it can wildcard unused bits.
I'll send a v2.
^ permalink raw reply
* Re: BCM4313 & brcmsmac & 3.12: only semi-working?
From: Michael Tokarev @ 2014-11-19 21:00 UTC (permalink / raw)
To: Arend van Spriel
Cc: Maximilian Engelhardt, Rafał Miłecki, Seth Forshee,
brcm80211 development, linux-wireless@vger.kernel.org,
Network Development
In-Reply-To: <546CF66A.3000207@msgid.tls.msk.ru>
19.11.2014 22:58, Michael Tokarev wrote:
> 19.11.2014 20:54, Arend van Spriel wrote:
[]
>> I submitted two patches upstream and additionally I have attached two other that are still under review. Could you try these patches and sent me the content of the two debugfs files 'macstat' and 'hardware' after a stall has occurred.
>
> You didn't tell which kernel it is based on. So I tried it on 3.16,
Ok, I misunderstood you apparently, -- I only tried 2 patches,
while I should try all 4. So here it goes.
The hardware info again:
> chipnum 0x4313
> chiprev 0x1
> chippackage 0x8
> corerev 0x18
> boardid 0x1795
> boardvendor 0x103c
> boardrev P107
> boardflags 0x402201
> boardflags2 0x884
> ucoderev 0x262032c
> radiorev 0x1
> phytype 0x8
> phyrev 0x1
> anarev 0xa
> nvramrev 8
Macstat:
txallfrm: 287
txrtsfrm: 118
txctsfrm: 25
txackfrm: 60
txdnlfrm: 0
txbcnfrm: 0
txfunfl[8]: 0 0 0 0 0 0 0 0
txtplunfl: 0
txphyerr: 0
pktengrxducast: 0
pktengrxdmcast: 0
rxfrmtoolong: 330
rxfrmtooshrt: 16
rxinvmachdr: 722
rxbadfcs: 4306
rxbadplcp: 7257
rxcrsglitch: 61757
rxstrt: 6667
rxdfrmucastmbss: 41
rxmfrmucastmbss: 25
rxcfrmucast: 116
rxrtsucast: 0
rxctsucast: 59
rxackucast: 19
rxdfrmocast: 70
rxmfrmocast: 84
rxcfrmocast: 211
rxrtsocast: 3
rxctsocast: 20
rxdfrmmcast: 9
rxmfrmmcast: 1486
rxcfrmmcast: 0
rxbeaconmbss: 377
rxdfrmucastobss: 0
rxbeaconobss: 1086
rxrsptmout: 94
bcntxcancl: 0
rxf0ovfl: 0
rxf1ovfl: 0
rxf2ovfl: 0
txsfovfl: 0
pmqovfl: 0
rxcgprqfrm: 0
rxcgprsqovfl: 0
txcgprsfail: 0
txcgprssuc: 0
prs_timeout: 0
rxnack: 0
frmscons: 0
txnack: 0
txglitch_nack: 38
txburst: 4
bphy_rxcrsglitch: 2
phywatchdog: 0
bphy_badplcp: 0
As far as I can see, the stats are never updated during stall,
no numbers are changing, at least while the download is waiting
for the next packet. Sometimes wpa_supplicant does something
little, so some stats gets updated, eg, this is how it looks like
after about 2..3 minutes:
txallfrm: 420
txrtsfrm: 201
txctsfrm: 25
txackfrm: 69
txdnlfrm: 0
txbcnfrm: 0
txfunfl[8]: 0 0 0 0 0 0 0 0
txtplunfl: 0
txphyerr: 0
pktengrxducast: 0
pktengrxdmcast: 0
rxfrmtoolong: 1908
rxfrmtooshrt: 73
rxinvmachdr: 4115
rxbadfcs: 15064
rxbadplcp: 42368
rxcrsglitch: 36620
rxstrt: 26393
rxdfrmucastmbss: 48
rxmfrmucastmbss: 27
rxcfrmucast: 158
rxrtsucast: 0
rxctsucast: 92
rxackucast: 25
rxdfrmocast: 113
rxmfrmocast: 390
rxcfrmocast: 962
rxrtsocast: 38
rxctsocast: 59
rxdfrmmcast: 48
rxmfrmmcast: 7681
rxcfrmmcast: 0
rxbeaconmbss: 1505
rxdfrmucastobss: 0
rxbeaconobss: 6059
rxrsptmout: 171
bcntxcancl: 0
rxf0ovfl: 0
rxf1ovfl: 0
rxf2ovfl: 0
txsfovfl: 0
pmqovfl: 0
rxcgprqfrm: 0
rxcgprsqovfl: 0
txcgprsfail: 0
txcgprssuc: 0
prs_timeout: 0
rxnack: 0
frmscons: 0
txnack: 0
txglitch_nack: 41
txburst: 4
bphy_rxcrsglitch: 5
phywatchdog: 0
bphy_badplcp: 0
This is with 3.18-tobe kernel (current Linus git).
Dunno if this is helpful or not...
Thanks,
/mjt
^ permalink raw reply
* Re: [PATCH net-next 3/4] igb: enable internal PPS for the i210.
From: Keller, Jacob E @ 2014-11-19 21:06 UTC (permalink / raw)
To: richardcochran@gmail.com
Cc: netdev@vger.kernel.org, davem@davemloft.net, Allan, Bruce W,
Ronciak, John, Kirsher, Jeffrey T, Vick, Matthew
In-Reply-To: <20141119202604.GA22213@localhost.localdomain>
On Wed, 2014-11-19 at 21:26 +0100, Richard Cochran wrote:
> On Wed, Nov 19, 2014 at 07:32:33PM +0000, Keller, Jacob E wrote:
> > Good catch :)
>
> (Well, my X session suddenly disappeared, and a kernel oops appeared in
> the console... hard to overlook ;^)
>
> > Did you see my concern about the reset path needing to fully restore the
> > state since it is called after a hardware MAC reset which has cleared
> > all these registers?
>
> Yes, and that bit I copied from the first series a year ago. I don't
> remember why, but IIRC that was necessary to let the SDP stuff work at
> all. Maybe the reset function was called under different circumstances
> back then. I'll take another look.
>
> I find it a bit weird that the auxiliary functions don't work when the
> interface or the link is down.
>
> Thanks,
> Richard
I think you need something here, but it should be clearing that register
after a MAC reset, so it needs to be re-initialized. I'm not sure if
that reset path was used in the same place in the past.
Well, I think igb and ixgbe destroy the ptp device when the interface
goes down, and restore it when it comes up. Probably we should instead
handle some things in reset path and allow the ptp device to remain.
It's partly due to the clock speed changing based on link speed.
Regards,
Jake
^ permalink raw reply
* Re: [RFC] situation with csum_and_copy_... API
From: Al Viro @ 2014-11-19 21:17 UTC (permalink / raw)
To: Linus Torvalds
Cc: David Miller, Network Development, Linux Kernel Mailing List
In-Reply-To: <CA+55aFyQR2gOzEDROcWFcQzLvTjOxyJRjFJXJ03JB5knd-Gsgg@mail.gmail.com>
On Wed, Nov 19, 2014 at 12:40:53PM -0800, Linus Torvalds wrote:
> On Wed, Nov 19, 2014 at 12:31 PM, David Miller <davem@davemloft.net> wrote:
> >
> > But that is just my opinion, and yes I do acknowledge that we've had
> > serious holes in this area in the past.
>
> The serious holes have generally been exactly in the "upper layers
> already check" camp, and then it turns out that some odd ioctl or
> other thing ends up doing something odd and interesting.
>
> If Al has actual performance profiles showing that the access_ok() is
> a real problem, then fine. As a low-level optimization, I agree with
> it. But not as a "let's just drop them, and make the security rules be
> non-local and subtle, and require people to know the details of the
> whole call-chain".
>
> Seeing a "__get_user()" and just being able to glance up in the same
> function and seeing the "access_ok()" is just a good safety net. And
> means that people don't have to waste time thinking about or looking
> for where the hell the security net really is.
Umm... It's not quite that bad - the thing is, for iov_iter-net series
we'll need copy_and_csum_{to,from}_iter() anyway and for iovec-backed
iov_iter instances we already ask the iovec to be validated wrt access_ok().
And this validation (already done by rw_copy_check_uvector()) is going to
be next to iov_iter_init() setting the iov_iter up.
Moreover, I'm planning to take iov_iter_init() into rw_copy_check_uvector().
That way setting ->iov is done from the same function that has just checked
all ranges. If you look at the callers, you'll see that almost all of them
are directly followed by iov_iter_init() and folding it in is a fairly
obvious cleanup.
The thing is, with ..._iter() variants added we are left with no other
in-tree callers of csum_and_copy_{to,from}_user(). I agree that dropping
those access_ok() is too early at this point - the analysis of paths
by which a range can reach them is scary right now. Moreover, it mostly
parallels the changes later in the series - ones that propagate a pointer
to iov_iter put into msdghdr in place of ->msg_iov/->msg_iovlen down to
the new primitives. _After_ those steps the analysis (see the horrors in
commit message of 3/5) becomes trivial.
So whether we end up removing those access_ok() or not, this is not the
time to do so. It still might make sense in the end, but not right now.
Frankly, my preference would be to provide __csum_and_copy_...() that do
not bother with access_ok() (and do so consistently between the architectures),
and do not bother with zeroing or trying to do an accurate csum in case of
error. With uniform implementation of csum_and_copy_...() that does
access_ok(), tries to call __csum_... variant and, in case of failure,
does __copy_..._user(), zeroes the tail in the "from" one and calculates
the csum by source of destination - whichever's kernel-side. I.e. do what
ppc64 is doing. That way we get obviously safe csum_and_copy_.._user(),
and consistent __ counterparts directly used by ..._iter() primitives.
Quite a bit of complexity becomes possible to remove from asm code,
while we are at it - zeroing isn't the worst of it, contortions needed to
calculate the csum accurately in error case are often nastier. So much
that e.g. arm doesn't even bother trying.
Again, this is a separate work - I agree with you regarding the overhead
being a non-issue for existing callers, and if somebody tries e.g. to send
64K from 32K-element vector of 2-byte ranges they'll have a _lot_ of other
overhead that will drown that of those access_ok(). In normal cases the
price of copying the data itself is going to swamp that of access_ok(),
of course.
IOW, consider the access_ok() changes withdrawn for now. It's too early
in the series for them and the only reason to pull them that high in
ordering had been the fear of overhead. Which is very unlikely to be
an issue. They won't come back (if they come back at all) until the
proof of correctness becomes absolutely trivial.
^ permalink raw reply
* Re: [RFC] situation with csum_and_copy_... API
From: David Miller @ 2014-11-19 21:17 UTC (permalink / raw)
To: torvalds; +Cc: viro, netdev, linux-kernel
In-Reply-To: <CA+55aFyQR2gOzEDROcWFcQzLvTjOxyJRjFJXJ03JB5knd-Gsgg@mail.gmail.com>
From: Linus Torvalds <torvalds@linux-foundation.org>
Date: Wed, 19 Nov 2014 12:40:53 -0800
> On Wed, Nov 19, 2014 at 12:31 PM, David Miller <davem@davemloft.net> wrote:
>>
>> But that is just my opinion, and yes I do acknowledge that we've had
>> serious holes in this area in the past.
>
> The serious holes have generally been exactly in the "upper layers
> already check" camp, and then it turns out that some odd ioctl or
> other thing ends up doing something odd and interesting.
>
> If Al has actual performance profiles showing that the access_ok() is
> a real problem, then fine. As a low-level optimization, I agree with
> it. But not as a "let's just drop them, and make the security rules be
> non-local and subtle, and require people to know the details of the
> whole call-chain".
>
> Seeing a "__get_user()" and just being able to glance up in the same
> function and seeing the "access_ok()" is just a good safety net. And
> means that people don't have to waste time thinking about or looking
> for where the hell the security net really is.
Fair enough.
^ permalink raw reply
* [PATCHv2 net] openvswitch: Fix mask generation for IPv6 labels.
From: Joe Stringer @ 2014-11-19 21:20 UTC (permalink / raw)
To: netdev-u79uwXL29TY76Z2rM5mHXA
Cc: dev-yBygre7rU0TnMu66kgdUjQ, linux-kernel-u79uwXL29TY76Z2rM5mHXA
When userspace doesn't provide a mask, OVS datapath generates a fully
unwildcarded mask for the flow. This is done by taking a copy of the
flow key, then iterating across its attributes, setting all values to
0xff. This works for most attributes, as the length of the netlink
attribute typically matches the length of the value. However, IPv6
labels only use the lower 20 bits of the field. This patch makes a
special case to handle this.
This fixes the following error seen when installing IPv6 flows without a mask:
openvswitch: netlink: Invalid IPv6 flow label value (value=ffffffff, max=fffff)
Signed-off-by: Joe Stringer <joestringer@nicira.com>
---
v2: OR lower 20 bits (upper 12 bits remain from earlier memdup)
---
net/openvswitch/flow_netlink.c | 22 ++++++++++++----------
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/net/openvswitch/flow_netlink.c b/net/openvswitch/flow_netlink.c
index fa4ec2e..e530025 100644
--- a/net/openvswitch/flow_netlink.c
+++ b/net/openvswitch/flow_netlink.c
@@ -825,7 +825,7 @@ static int ovs_key_from_nlattrs(struct sw_flow_match *match, u64 attrs,
return 0;
}
-static void nlattr_set(struct nlattr *attr, u8 val, bool is_attr_mask_key)
+static void mask_set_nlattr(struct nlattr *attr)
{
struct nlattr *nla;
int rem;
@@ -835,16 +835,18 @@ static void nlattr_set(struct nlattr *attr, u8 val, bool is_attr_mask_key)
/* We assume that ovs_key_lens[type] == -1 means that type is a
* nested attribute
*/
- if (is_attr_mask_key && ovs_key_lens[nla_type(nla)] == -1)
- nlattr_set(nla, val, false);
+ if (ovs_key_lens[nla_type(nla)] == -1)
+ nla_for_each_nested(nla, attr, rem)
+ memset(nla_data(nla), 0xff, nla_len(nla));
else
- memset(nla_data(nla), val, nla_len(nla));
- }
-}
+ memset(nla_data(nla), 0xff, nla_len(nla));
-static void mask_set_nlattr(struct nlattr *attr, u8 val)
-{
- nlattr_set(attr, val, true);
+ if (nla_type(nla) == OVS_KEY_ATTR_IPV6) {
+ struct ovs_key_ipv6 *ipv6_key = nla_data(nla);
+
+ ipv6_key->ipv6_label |= htonl(0x000FFFFF);
+ }
+ }
}
/**
@@ -926,7 +928,7 @@ int ovs_nla_get_match(struct sw_flow_match *match,
if (!newmask)
return -ENOMEM;
- mask_set_nlattr(newmask, 0xff);
+ mask_set_nlattr(newmask);
/* The userspace does not send tunnel attributes that are 0,
* but we should not wildcard them nonetheless.
--
1.7.10.4
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
^ permalink raw reply related
* Re: Question about Patch Submissions
From: rapier @ 2014-11-19 21:29 UTC (permalink / raw)
To: Alexei Starovoitov; +Cc: Cong Wang, netdev
In-Reply-To: <CAADnVQ+1_Ct3hhmZrCqpxcT=-XS4OXrpRbrHHHyOKQOuoDc06g@mail.gmail.com>
On 11/18/14, 8:38 PM, Alexei Starovoitov wrote:
> On Tue, Nov 18, 2014 at 12:39 PM, rapier <rapier@psc.edu> wrote:
>> break this down for presentation. We can make sure each section of the patch
>> applies cleanly but the entirety of the patch set would have to be applied
>> for it to compile.
>
> all patches need to compile cleanly on all architectures,
> otherwise bisect will be broken.
The issue is that this is large patch set. The diff comes in at around
2200 lines and adds new functionality to the kernel by implementing RFC
4898. What I'm really trying to figure out is how to split up the diff
files so I'm not dropping all 2200 lines into a single email message.
I'm assuming that's not really how people want to see submissions but if
I am incorrect please let me know.
There are two major components that we should be able to submit as
distinct patches so that should reduce the line count per submission.
Chris Rapier
^ permalink raw reply
* Re: [RFC] situation with csum_and_copy_... API
From: Al Viro @ 2014-11-19 21:30 UTC (permalink / raw)
To: David Miller; +Cc: torvalds, netdev, linux-kernel
In-Reply-To: <20141119.161744.1661940121298888832.davem@davemloft.net>
On Wed, Nov 19, 2014 at 04:17:44PM -0500, David Miller wrote:
> > Seeing a "__get_user()" and just being able to glance up in the same
> > function and seeing the "access_ok()" is just a good safety net. And
> > means that people don't have to waste time thinking about or looking
> > for where the hell the security net really is.
>
> Fair enough.
OK, with 3/5 dropped 4/5 get a trivial conflict (removal of function in 4/5
vs. change in it in 3/5). With that dealt with, the sucker is in
git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs.git for-davem
Shortlog:
Al Viro (4):
separate kernel- and userland-side msghdr
{compat_,}verify_iovec(): switch to generic copying of iovecs
fold verify_iovec() into copy_msghdr_from_user()
bury skb_copy_to_page()
Diffstat:
arch/arm/kernel/sys_oabi-compat.c | 4 +--
include/linux/socket.h | 17 +++++++---
include/linux/syscalls.h | 6 ++--
include/net/compat.h | 5 ++-
include/net/sock.h | 23 -------------
net/compat.c | 83 +++++++++++++++-------------------------------
net/core/iovec.c | 47 --------------------------
net/socket.c | 140 +++++++++++++++++++++++++++++++++++++-----------------------------------------
8 files changed, 114 insertions(+), 211 deletions(-)
I'll post more for review after I finally get some sleep - up for bloody 27
hours by now ;-/
^ permalink raw reply
* RE: [PATCH] ixgbe: Correctly disable vlan filter in promiscuous mode
From: Tantilov, Emil S @ 2014-11-19 21:50 UTC (permalink / raw)
To: Vlad Yasevich, netdev@vger.kernel.org
Cc: Kirsher, Jeffrey T, Keller, Jacob E, Skidmore, Donald C
In-Reply-To: <546CF258.9050205@gmail.com>
>-----Original Message-----
>From: Vlad Yasevich [mailto:vyasevich@gmail.com]
>Sent: Wednesday, November 19, 2014 11:41 AM
>To: Tantilov, Emil S; netdev@vger.kernel.org
>Cc: Kirsher, Jeffrey T; Keller, Jacob E; Skidmore, Donald C
>Subject: Re: [PATCH] ixgbe: Correctly disable vlan filter in
>promiscuous mode
>
>On 11/18/2014 03:24 PM, Tantilov, Emil S wrote:
>>> -----Original Message-----
>>> From: netdev-owner@vger.kernel.org [mailto:netdev-
>>> owner@vger.kernel.org] On Behalf Of Vladislav Yasevich
>>> Sent: Tuesday, November 18, 2014 11:28 AM
>>> To: netdev@vger.kernel.org
>>> Cc: Vladislav Yasevich; Kirsher, Jeffrey T; Keller, Jacob E
>>> Subject: [PATCH] ixgbe: Correctly disable vlan filter in promiscuous mode
>>>
>>> IXGBE adapater seems to require that vlan filtering be enabled if VMDQ
>>> or SRIOV are enabled. When those functions are disabled,
>>> vlan filtering may be disabled in promiscuous mode.
>>>
>>> Prior to commit a9b8943ee129e11045862d6d6e25c5b63c95403c
>>> ixgbe: remove vlan_filter_disable and enable functions
>>>
>>> the logic was correct. However, after the commit the logic
>>> got reversed and vlan filtered in now turned on when VMDQ/SRIOV
>>> is disabled.
>>>
>>> This patch changes the condition to enable hw vlan filtered
>>> when VMDQ or SRIOV is enabled.
>>>
>>> Fixes: a9b8943ee129e11045862d6d6e25c5b63c95403c (ixgbe:
>>> remove
>>> vlan_filter_disable and enable functions)
>>> CC: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
>>> CC: Jacob Keller <jacob.e.keller@intel.com>
>>> Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
>>> ---
>>> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 4 ++--
>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>>> b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>>> index d2df4e3..3f81c7a 100644
>>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>>> @@ -3936,8 +3936,8 @@ void ixgbe_set_rx_mode(struct
>>> net_device *netdev)
>>> * if SR-IOV and VMDQ are disabled - otherwise ensure
>>> * that hardware VLAN filters remain enabled.
>>> */
>>> - if (!(adapter->flags & (IXGBE_FLAG_VMDQ_ENABLED |
>>> - IXGBE_FLAG_SRIOV_ENABLED)))
>>> + if (adapter->flags & (IXGBE_FLAG_VMDQ_ENABLED |
>>> + IXGBE_FLAG_SRIOV_ENABLED))
>>> vlnctrl |= (IXGBE_VLNCTRL_VFE |
>>> IXGBE_VLNCTRL_CFIEN);
>>> } else {
>>> if (netdev->flags & IFF_ALLMULTI) {
>>
>> The current logic is correct and it's like this on purpose
>> as it should be obvious by the comment preceding this check.
>
>Actually the comment right now does not match what the code
>is doing.
>
>The comment states:
> /* Only disable hardware filter vlans in promiscuous mode
> * if SR-IOV and VMDQ are disabled - otherwise ensure
> * that hardware VLAN filters remain enabled.
> */
>
>However, the code currently will _enable_ vlan filtering if VMDQ/SRIOV
>is _disabled_ in promiscuous mode.
Actually you're right. Sorry - I misread the patch initially.
This is indeed a bug in the code.
Acked-by: Emil Tantilov <emil.s.tantilov@intel.com>
Jeff should pick it up.
Thanks,
Emil
^ permalink raw reply
* Re: [PATCHv2 net] openvswitch: Fix mask generation for IPv6 labels.
From: Joe Stringer @ 2014-11-19 21:52 UTC (permalink / raw)
To: netdev; +Cc: linux-kernel, pshelar, dev
In-Reply-To: <1416432001-35678-1-git-send-email-joestringer@nicira.com>
On Wednesday, November 19, 2014 13:20:01 Joe Stringer wrote:
> When userspace doesn't provide a mask, OVS datapath generates a fully
> unwildcarded mask for the flow. This is done by taking a copy of the
> flow key, then iterating across its attributes, setting all values to
> 0xff. This works for most attributes, as the length of the netlink
> attribute typically matches the length of the value. However, IPv6
> labels only use the lower 20 bits of the field. This patch makes a
> special case to handle this.
>
> This fixes the following error seen when installing IPv6 flows without a
> mask:
>
> openvswitch: netlink: Invalid IPv6 flow label value (value=ffffffff,
> max=fffff)
Based on offlist discussion with Pravin, I'll send a patch for an alternative
approach.
^ permalink raw reply
* Re: [RFC] situation with csum_and_copy_... API
From: David Miller @ 2014-11-19 21:53 UTC (permalink / raw)
To: viro; +Cc: torvalds, netdev, linux-kernel
In-Reply-To: <20141119213006.GE7996@ZenIV.linux.org.uk>
From: Al Viro <viro@ZenIV.linux.org.uk>
Date: Wed, 19 Nov 2014 21:30:07 +0000
> On Wed, Nov 19, 2014 at 04:17:44PM -0500, David Miller wrote:
>> > Seeing a "__get_user()" and just being able to glance up in the same
>> > function and seeing the "access_ok()" is just a good safety net. And
>> > means that people don't have to waste time thinking about or looking
>> > for where the hell the security net really is.
>>
>> Fair enough.
>
> OK, with 3/5 dropped 4/5 get a trivial conflict (removal of function in 4/5
> vs. change in it in 3/5). With that dealt with, the sucker is in
> git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs.git for-davem
>
> Shortlog:
> Al Viro (4):
> separate kernel- and userland-side msghdr
> {compat_,}verify_iovec(): switch to generic copying of iovecs
> fold verify_iovec() into copy_msghdr_from_user()
> bury skb_copy_to_page()
Pulled, thanks Al.
^ permalink raw reply
* Re: [PATCH 1/1] mISDN: Deletion of unnecessary checks before the function call "vfree"
From: David Miller @ 2014-11-19 21:54 UTC (permalink / raw)
To: elfring; +Cc: isdn, netdev, linux-kernel, kernel-janitors, julia.lawall
In-Reply-To: <546CF5A6.3030305@users.sourceforge.net>
From: SF Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 19 Nov 2014 20:55:18 +0100
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Wed, 19 Nov 2014 20:48:26 +0100
>
> The vfree() function performs also input parameter validation. Thus the test
> around the call is not needed.
>
> This issue was detected by using the Coccinelle software.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
Applied, thanks.
^ permalink raw reply
* [PATCHv3 net] openvswitch: Don't validate IPv6 label masks.
From: Joe Stringer @ 2014-11-19 21:54 UTC (permalink / raw)
To: netdev-u79uwXL29TY76Z2rM5mHXA
Cc: dev-yBygre7rU0TnMu66kgdUjQ, linux-kernel-u79uwXL29TY76Z2rM5mHXA
When userspace doesn't provide a mask, OVS datapath generates a fully
unwildcarded mask for the flow by copying the flow and setting all bits
in all fields. For IPv6 label, this creates a mask that matches on the
upper 12 bits, causing the following error:
openvswitch: netlink: Invalid IPv6 flow label value (value=ffffffff, max=fffff)
This patch ignores the label validation check for masks, avoiding this
error.
Signed-off-by: Joe Stringer <joestringer@nicira.com>
---
v3: Alternative approach.
Was "openvswitch: Fix mask generation for IPv6 labels."
v2: OR lower 20 bits (upper 12 bits remain from earlier memdup)
---
net/openvswitch/flow_netlink.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/openvswitch/flow_netlink.c b/net/openvswitch/flow_netlink.c
index fa4ec2e..089b195 100644
--- a/net/openvswitch/flow_netlink.c
+++ b/net/openvswitch/flow_netlink.c
@@ -690,7 +690,7 @@ static int ovs_key_from_nlattrs(struct sw_flow_match *match, u64 attrs,
return -EINVAL;
}
- if (ipv6_key->ipv6_label & htonl(0xFFF00000)) {
+ if (!is_mask && ipv6_key->ipv6_label & htonl(0xFFF00000)) {
OVS_NLERR("IPv6 flow label %x is out of range (max=%x).\n",
ntohl(ipv6_key->ipv6_label), (1 << 20) - 1);
return -EINVAL;
--
1.7.10.4
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
^ permalink raw reply related
* Re: [PATCH v2] ipv6: delete protocol and unregister rtnetlink when cleanup
From: David Miller @ 2014-11-19 21:55 UTC (permalink / raw)
To: duanj.fnst; +Cc: cwang, netdev, eric.dumazet
In-Reply-To: <546BF3EB.1080603@cn.fujitsu.com>
From: Duan Jiong <duanj.fnst@cn.fujitsu.com>
Date: Wed, 19 Nov 2014 09:35:39 +0800
> pim6_protocol was added when initiation, but it not deleted.
> Similarly, unregister RTNL_FAMILY_IP6MR rtnetlink.
>
> Signed-off-by: Duan Jiong <duanj.fnst@cn.fujitsu.com>
> ---
> v2: add missing rtnl_unregister()
Applied, thanks.
^ permalink raw reply
* Re: [PATCH net-next] bpf: fix arraymap NULL deref and missing overflow and zero size checks
From: David Miller @ 2014-11-19 21:55 UTC (permalink / raw)
To: ast; +Cc: fengguang.wu, hannes, dborkman, netdev, linux-kernel
In-Reply-To: <1416360736-9531-1-git-send-email-ast@plumgrid.com>
From: Alexei Starovoitov <ast@plumgrid.com>
Date: Tue, 18 Nov 2014 17:32:16 -0800
> - fix NULL pointer dereference:
> kernel/bpf/arraymap.c:41 array_map_alloc() error: potential null dereference 'array'. (kzalloc returns null)
> kernel/bpf/arraymap.c:41 array_map_alloc() error: we previously assumed 'array' could be null (see line 40)
>
> - integer overflow check was missing in arraymap
> (hashmap checks for overflow via kmalloc_array())
>
> - arraymap can round_up(value_size, 8) to zero. check was missing.
>
> - hashmap was missing zero size check as well, since roundup_pow_of_two() can
> truncate into zero
>
> - found a typo in the arraymap comment and unnecessary empty line
>
> Fix all of these issues and make both overflow checks explicit U32 in size.
>
> Reported-by: kbuild test robot <fengguang.wu@intel.com>
> Signed-off-by: Alexei Starovoitov <ast@plumgrid.com>
> ---
> This silly NULL deref bug and missing overflow check was an oversight when
> I refactored the code from two allocations (kmalloc for struct bpf_array and
> kcalloc for array of elements) in the first implementation of arraymap
> into one allocation which is this code.
Applied, thanks.
^ 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